rework ServiceInfoFactory priorities (#1644)

* rework ServiceInfoFactory priorities
- default/all: load detectors from registry last
- env-provided detectors: load in same order as default/all, then from registry in requested order

* split service.instance.id into own detector
to avoid always generating service.instance.id (which is useless for fpm/apache), split it
into its own detector, ServiceInstance, and don't use it by default. If it is enabled, ensure
it runs before registry-provided detectors.

* Update src/SDK/Resource/ResourceInfoFactory.php

Co-authored-by: Chris Lightfoot-Wild <[email protected]>

* override

---------

Co-authored-by: Chris Lightfoot-Wild <[email protected]>

Author: brettmc

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,

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
@@ -20,18 +19,13 @@ final class Service implements ResourceDetectorInterface
     #[\Override]
     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);
     }

Resource/Detectors/ServiceInstance.php

@@ -0,0 +1,33 @@
+<?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
+{
+    #[\Override]
+    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);
+    }
+}

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();
     }