Fix overwritting of the service.instance.id which has been populated from the user provided values from the resource detectors

Author: rads-1996

opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py

@@ -638,6 +638,12 @@ def get_aggregated_resources(
             detected_resource: Resource = _EMPTY_RESOURCE
             try:
                 detected_resource = future.result(timeout=timeout)
+
+                if (
+                    isinstance(detector, ServiceInstanceIdResourceDetector)
+                    and SERVICE_INSTANCE_ID in detectors_merged_resource.attributes
+                ):
+                    continue
             except concurrent.futures.TimeoutError as ex:
                 if detector.raise_on_error:
                     raise ex
@@ -651,8 +657,7 @@ def get_aggregated_resources(
                 if detector.raise_on_error:
                     raise ex
                 logger.warning("Exception %s in detector %s, ignoring", ex, detector)
-            finally:
-                detectors_merged_resource = detectors_merged_resource.merge(detected_resource)
+            detectors_merged_resource = detectors_merged_resource.merge(detected_resource)
     finally:
         executor.shutdown(wait=False, cancel_futures=True)
 

opentelemetry-sdk/tests/resources/test_resources.py

@@ -1036,6 +1036,56 @@ def tearDown(self) -> None:
     def test_is_process_dependent(self):
         self.assertTrue(ServiceInstanceIdResourceDetector().is_process_dependent())
 
+    def test_aggregation_preserves_instance_id_from_previous_detector(self):
+        resource_detector = Mock(spec=ResourceDetector)
+        resource_detector.detect.return_value = Resource({SERVICE_INSTANCE_ID: "service-instance-id"})
+        subsequent_detector = Mock(spec=ResourceDetector)
+        subsequent_detector.detect.return_value = Resource({"key": "value"})
+
+        resource = get_aggregated_resources(
+            [
+                resource_detector,
+                ServiceInstanceIdResourceDetector(),
+                subsequent_detector,
+            ],
+            initial_resource=Resource.get_empty(),
+        )
+
+        self.assertEqual(
+            resource.attributes,
+            {SERVICE_INSTANCE_ID: "service-instance-id", "key": "value"},
+        )
+
+    def test_aggregation_preserves_instance_id_from_initial_resource(self):
+        resource = get_aggregated_resources(
+            [ServiceInstanceIdResourceDetector()],
+            initial_resource=Resource({SERVICE_INSTANCE_ID: "user-provided-instance-id"}),
+        )
+
+        self.assertEqual(resource.attributes[SERVICE_INSTANCE_ID], "user-provided-instance-id")
+
+    @patch.dict(
+        environ,
+        {OTEL_RESOURCE_ATTRIBUTES: "service.instance.id=environment-instance-id"},
+        clear=True,
+    )
+    def test_aggregation_preserves_instance_id_from_environment(self):
+        resource = get_aggregated_resources(
+            [OTELResourceDetector(), ServiceInstanceIdResourceDetector()],
+            initial_resource=Resource.get_empty(),
+        )
+
+        self.assertEqual(resource.attributes[SERVICE_INSTANCE_ID], "environment-instance-id")
+
+    def test_aggregation_generates_instance_id_when_missing(self):
+        resource = get_aggregated_resources(
+            [ServiceInstanceIdResourceDetector()],
+            initial_resource=Resource({"key": "value"}),
+        )
+
+        self.assertEqual(uuid.UUID(resource.attributes[SERVICE_INSTANCE_ID]).version, 4)
+        self.assertEqual(resource.attributes["key"], "value")
+
     def test_detect_value_is_valid_uuid4(self):
         _resources_module._service_instance_id = None
         _resources_module._service_instance_id_pid = None