rework ServiceInfoFactory priorities

src/SDK/Common/Configuration/KnownValues.php

@@ -197,6 +197,7 @@ interface KnownValues
     public const VALUE_DETECTORS_SDK = 'sdk';
     public const VALUE_DETECTORS_SDK_PROVIDED = 'sdk_provided';
     public const VALUE_DETECTORS_SERVICE = 'service';
+    public const VALUE_DETECTORS_SERVICE_INSTANCE = 'service_instance';
     public const VALUE_DETECTORS_COMPOSER = 'composer';
     public const OTEL_PHP_DETECTORS = [
         self::VALUE_ALL,

src/SDK/Resource/Detectors/Service.php

@@ -10,7 +10,6 @@
 use OpenTelemetry\SDK\Resource\ResourceDetectorInterface;
 use OpenTelemetry\SDK\Resource\ResourceInfo;
 use OpenTelemetry\SemConv\ResourceAttributes;
-use Ramsey\Uuid\Uuid;
 
 /**
  * @see https://github.com/open-telemetry/semantic-conventions/tree/main/docs/resource#service-experimental
@@ -19,18 +18,13 @@ final class Service implements ResourceDetectorInterface
 {
     public function getResource(): ResourceInfo
     {
-        static $serviceInstanceId;
-        $serviceInstanceId ??= Uuid::uuid4()->toString();
         $serviceName = Configuration::has(Variables::OTEL_SERVICE_NAME)
             ? Configuration::getString(Variables::OTEL_SERVICE_NAME)
             : null;
 
         $attributes = [
-            ResourceAttributes::SERVICE_INSTANCE_ID => $serviceInstanceId,
+            ResourceAttributes::SERVICE_NAME => $serviceName,
         ];
-        if ($serviceName !== null) {
-            $attributes[ResourceAttributes::SERVICE_NAME] = $serviceName;
-        }
 
         return ResourceInfo::create(Attributes::create($attributes), ResourceAttributes::SCHEMA_URL);
     }

src/SDK/Resource/Detectors/ServiceInstance.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\SDK\Resource\Detectors;
+
+use OpenTelemetry\SDK\Common\Attribute\Attributes;
+use OpenTelemetry\SDK\Resource\ResourceDetectorInterface;
+use OpenTelemetry\SDK\Resource\ResourceInfo;
+use OpenTelemetry\SemConv\ResourceAttributes;
+use Ramsey\Uuid\Uuid;
+
+/**
+ * @see https://github.com/open-telemetry/semantic-conventions/tree/main/docs/resource#service-experimental
+ *
+ * Spec deviation: Service Instance ID is not generated by the Service detector, as UUID is not useful in shared-nothing
+ * PHP setups (FPM, Apache), and cannot be replaced by a more useful value due to Service being the last detector.
+ */
+final class ServiceInstance implements ResourceDetectorInterface
+{
+    public function getResource(): ResourceInfo
+    {
+        static $serviceInstanceId;
+        $serviceInstanceId ??= Uuid::uuid4()->toString();
+
+        $attributes = [
+            ResourceAttributes::SERVICE_INSTANCE_ID => $serviceInstanceId,
+        ];
+
+        return ResourceInfo::create(Attributes::create($attributes), ResourceAttributes::SCHEMA_URL);
+    }
+}

src/SDK/Resource/ResourceInfoFactory.php

@@ -29,50 +29,51 @@ public static function defaultResource(): ResourceInfo
             return (new Detectors\Composite([
                 new Detectors\Host(),
                 new Detectors\Process(),
-                ...Registry::resourceDetectors(),
                 new Detectors\Environment(),
                 new Detectors\Sdk(),
                 new Detectors\Service(),
+                ...Registry::resourceDetectors(),
             ]))->getResource();
         }
 
+        /**
+         * Process env-provided detectors:
+         * - host, process, environment, composer first if requested
+         * - sdk, service always
+         * - any other detectors registered in the registry if requested
+         */
         $resourceDetectors = [];
-
-        foreach ($detectors as $detector) {
-            switch ($detector) {
-                case Values::VALUE_DETECTORS_ENVIRONMENT:
-                    $resourceDetectors[] = new Detectors\Environment();
-
-                    break;
-                case Values::VALUE_DETECTORS_HOST:
-                    $resourceDetectors[] = new Detectors\Host();
-
-                    break;
-                case Values::VALUE_DETECTORS_PROCESS:
-                    $resourceDetectors[] = new Detectors\Process();
-
-                    break;
-
-                case Values::VALUE_DETECTORS_COMPOSER:
-                    $resourceDetectors[] = new Detectors\Composer();
-
-                    break;
-                case Values::VALUE_DETECTORS_SDK_PROVIDED: //deprecated
-                case Values::VALUE_DETECTORS_OS: //deprecated
-                case Values::VALUE_DETECTORS_PROCESS_RUNTIME: //deprecated
-                case Values::VALUE_NONE:
-
-                    break;
-                default:
-                    try {
-                        $resourceDetectors[] = Registry::resourceDetector($detector);
-                    } catch (RuntimeException $e) {
-                        self::logWarning($e->getMessage());
-                    }
+        foreach ([
+            Values::VALUE_DETECTORS_HOST => Detectors\Host::class,
+            Values::VALUE_DETECTORS_PROCESS => Detectors\Process::class,
+            Values::VALUE_DETECTORS_ENVIRONMENT => Detectors\Environment::class,
+            Values::VALUE_DETECTORS_COMPOSER => Detectors\Composer::class,
+            Values::VALUE_DETECTORS_SERVICE_INSTANCE => Detectors\ServiceInstance::class,
+        ] as $detector => $class) {
+            if (in_array($detector, $detectors)) {
+                $resourceDetectors[] = new $class();
+                $detectors = array_diff($detectors, [$detector]);
             }
         }
         $resourceDetectors [] = new Detectors\Sdk();
         $resourceDetectors [] = new Detectors\Service();
+        // Don't try to load mandatory + deprecated detectors
+        $detectors = array_diff($detectors, [
+            Values::VALUE_DETECTORS_SDK,
+            Values::VALUE_DETECTORS_SERVICE,
+            Values::VALUE_DETECTORS_SDK_PROVIDED, //deprecated
+            Values::VALUE_DETECTORS_OS, //deprecated
+            Values::VALUE_DETECTORS_PROCESS_RUNTIME, //deprecated
+            VALUES::VALUE_NONE,
+        ]);
+
+        foreach ($detectors as $detector) {
+            try {
+                $resourceDetectors[] = Registry::resourceDetector($detector);
+            } catch (RuntimeException $e) {
+                self::logWarning($e->getMessage());
+            }
+        }
 
         return (new Detectors\Composite($resourceDetectors))->getResource();
     }

tests/Integration/Config/ConfigurationTest.php

@@ -124,7 +124,6 @@ public function test_resource_include_exclude(): void
             'process.executable.path',
             'process.owner',
             'process.runtime.name',
-            'service.instance.id',
             'service.name',
             'telemetry.distro.name',
             'telemetry.distro.version',
@@ -145,7 +144,6 @@ public function test_resource_defaults(): void
     {
         $expectedKeys = [
             'service.name',
-            'service.instance.id',
             'telemetry.distro.name',
             'telemetry.distro.version',
             'telemetry.sdk.language',

tests/Integration/SDK/Resource/ResourceInfoFactoryTest.php

@@ -171,4 +171,16 @@ public function test_composer_default_resources(): void
         $this->assertEquals('open-telemetry/opentelemetry', $resource->getAttributes()->get(ResourceAttributes::SERVICE_NAME));
         $this->assertEquals(InstalledVersions::getRootPackage()['pretty_version'], $resource->getAttributes()->get(ResourceAttributes::SERVICE_VERSION));
     }
+
+    public function test_env_doesnt_clobber_service(): void
+    {
+        //service.name should be provided by mandatory Service detector, and should not be clobbered by environment detector
+        $this->setEnvironmentVariable('OTEL_SERVICE_NAME', 'test-service');
+        $this->setEnvironmentVariable('OTEL_RESOURCE_ATTRIBUTES', 'service.name=ignore-me');
+        $this->setEnvironmentVariable('OTEL_PHP_DETECTORS', 'env');
+
+        $resource = ResourceInfoFactory::defaultResource();
+
+        $this->assertEquals('test-service', $resource->getAttributes()->get(ResourceAttributes::SERVICE_NAME));
+    }
 }

tests/Unit/SDK/Resource/Detectors/ServiceInstanceIdTest.php

@@ -0,0 +1,44 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Unit\SDK\Resource\Detectors;
+
+use OpenTelemetry\SDK\Resource\Detectors;
+use OpenTelemetry\SemConv\ResourceAttributes;
+use OpenTelemetry\Tests\TestState;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\TestCase;
+
+#[CoversClass(Detectors\ServiceInstance::class)]
+class ServiceInstanceIdTest extends TestCase
+{
+    use TestState;
+
+    const UUID_REGEX = '/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i';
+
+    private Detectors\ServiceInstance $detector;
+
+    public function setUp(): void
+    {
+        $this->detector = new Detectors\ServiceInstance();
+    }
+
+    public function test_service_get_resource_with_default_service_instance_id(): void
+    {
+        $resource = $this->detector->getResource();
+
+        $this->assertNotEmpty($resource->getAttributes());
+
+        $id = $resource->getAttributes()->get(ResourceAttributes::SERVICE_INSTANCE_ID);
+        $this->assertMatchesRegularExpression(self::UUID_REGEX, $id);
+    }
+
+    public function test_service_get_resource_multiple_calls_same_service_instance_id(): void
+    {
+        $resource1 = $this->detector->getResource();
+        $resource2 = $this->detector->getResource();
+
+        $this->assertSame($resource1->getAttributes()->get(ResourceAttributes::SERVICE_INSTANCE_ID), $resource2->getAttributes()->get(ResourceAttributes::SERVICE_INSTANCE_ID));
+    }
+}

tests/Unit/SDK/Resource/Detectors/ServiceTest.php

@@ -15,33 +15,13 @@ class ServiceTest extends TestCase
 {
     use TestState;
 
-    const UUID_REGEX = '/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i';
-
     private Detectors\Service $detector;
 
     public function setUp(): void
     {
         $this->detector = new Detectors\Service();
     }
 
-    public function test_service_get_resource_with_default_service_instance_id(): void
-    {
-        $resource = $this->detector->getResource();
-
-        $this->assertNotEmpty($resource->getAttributes());
-
-        $id = $resource->getAttributes()->get(ResourceAttributes::SERVICE_INSTANCE_ID);
-        $this->assertMatchesRegularExpression(self::UUID_REGEX, $id);
-    }
-
-    public function test_service_get_resource_multiple_calls_same_service_instance_id(): void
-    {
-        $resource1 = $this->detector->getResource();
-        $resource2 = $this->detector->getResource();
-
-        $this->assertSame($resource1->getAttributes()->get(ResourceAttributes::SERVICE_INSTANCE_ID), $resource2->getAttributes()->get(ResourceAttributes::SERVICE_INSTANCE_ID));
-    }
-
     public function test_sdk_get_resource_with_service_name(): void
     {
         $this->setEnvironmentVariable('OTEL_SERVICE_NAME', 'test-service');

tests/Unit/SDK/Resource/ResourceInfoFactoryTest.php

@@ -107,10 +107,10 @@ public function test_resource_with_invalid_environment_variable(): void
     #[Group('compliance')]
     public function test_resource_from_environment_service_name_takes_precedence_over_resource_attribute(): void
     {
-        $this->setEnvironmentVariable('OTEL_RESOURCE_ATTRIBUTES', 'service.name=bar');
-        $this->setEnvironmentVariable('OTEL_SERVICE_NAME', 'foo');
+        $this->setEnvironmentVariable('OTEL_RESOURCE_ATTRIBUTES', 'service.name=from-resource-attributes');
+        $this->setEnvironmentVariable('OTEL_SERVICE_NAME', 'from-service-name');
         $resource = ResourceInfoFactory::defaultResource();
-        $this->assertEquals('foo', $resource->getAttributes()->get('service.name'));
+        $this->assertEquals('from-service-name', $resource->getAttributes()->get('service.name'));
     }
 
     #[Group('compliance')]
@@ -188,11 +188,20 @@ public function test_environment_get_resource_service_name_precedence_over_resou
     {
         $this->setEnvironmentVariable('OTEL_RESOURCE_ATTRIBUTES', 'service.name=from-env');
         $this->setEnvironmentVariable('OTEL_SERVICE_NAME', 'from-service-name');
-        $this->setEnvironmentVariable('OTEL_PHP_DETECTORS', 'environment');
+        $this->setEnvironmentVariable('OTEL_PHP_DETECTORS', 'env');
 
         $resource = ResourceInfoFactory::defaultResource();
 
         $this->assertNotEmpty($resource->getAttributes());
         $this->assertSame('from-service-name', $resource->getAttributes()->get(ResourceAttributes::SERVICE_NAME));
     }
+
+    public function test_service_instance_detector(): void
+    {
+        $this->setEnvironmentVariable('OTEL_PHP_DETECTORS', 'service_instance');
+
+        $resource = ResourceInfoFactory::defaultResource();
+
+        $this->assertNotEmpty($resource->getAttributes()->get(ResourceAttributes::SERVICE_INSTANCE_ID));
+    }
 }