From 3928b131528125923dacb4a4f0bd03d4de1141cd Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Sat, 13 Jan 2024 16:52:09 +1100 Subject: [PATCH] optimize resource creation (#1210) * reduce resource creation from sdk autoloader - instead of creating new resources from various factories, create them once from sdk autoloader and pass them to the factories (most of which already supported this). - check env before php.ini for config, since this is the more popular approach and saves some cycles - cache header from OtlpUtil, to save a couple of calls to Sdk detector * adding benchmark for resource creation --- src/Contrib/Otlp/OtlpUtil.php | 15 +++++++++----- .../Resolver/CompositeResolver.php | 2 +- src/SDK/Logs/LoggerProviderFactory.php | 5 +++-- src/SDK/Metrics/MeterProviderFactory.php | 5 +++-- src/SDK/Resource/ResourceInfo.php | 3 ++- src/SDK/SdkAutoloader.php | 7 +++++-- tests/Benchmark/ResourceCreationBench.php | 20 +++++++++++++++++++ 7 files changed, 44 insertions(+), 13 deletions(-) create mode 100644 tests/Benchmark/ResourceCreationBench.php diff --git a/src/Contrib/Otlp/OtlpUtil.php b/src/Contrib/Otlp/OtlpUtil.php index 6901c1324..a60ed30fa 100644 --- a/src/Contrib/Otlp/OtlpUtil.php +++ b/src/Contrib/Otlp/OtlpUtil.php @@ -35,11 +35,16 @@ public static function method(string $signal): string */ public static function getUserAgentHeader(): array { - $resource = (new Sdk())->getResource(); + static $header; + if ($header === null) { + $resource = (new Sdk())->getResource(); + + $header = ['User-Agent' => sprintf( + 'OTel OTLP Exporter PHP/%s', + $resource->getAttributes()->get(ResourceAttributes::TELEMETRY_SDK_VERSION) ?: 'unknown' + )]; + } - return ['User-Agent' => sprintf( - 'OTel OTLP Exporter PHP/%s', - $resource->getAttributes()->get(ResourceAttributes::TELEMETRY_SDK_VERSION) ?: 'unknown' - )]; + return $header; } } diff --git a/src/SDK/Common/Configuration/Resolver/CompositeResolver.php b/src/SDK/Common/Configuration/Resolver/CompositeResolver.php index 05128198b..1160129be 100644 --- a/src/SDK/Common/Configuration/Resolver/CompositeResolver.php +++ b/src/SDK/Common/Configuration/Resolver/CompositeResolver.php @@ -18,8 +18,8 @@ public static function instance(): self { static $instance; $instance ??= new self([ - new PhpIniResolver(), new EnvironmentResolver(), + new PhpIniResolver(), ]); return $instance; diff --git a/src/SDK/Logs/LoggerProviderFactory.php b/src/SDK/Logs/LoggerProviderFactory.php index 3d0e965fd..1827c212d 100644 --- a/src/SDK/Logs/LoggerProviderFactory.php +++ b/src/SDK/Logs/LoggerProviderFactory.php @@ -6,11 +6,12 @@ use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeFactory; use OpenTelemetry\SDK\Metrics\MeterProviderInterface; +use OpenTelemetry\SDK\Resource\ResourceInfo; use OpenTelemetry\SDK\Sdk; class LoggerProviderFactory { - public function create(?MeterProviderInterface $meterProvider = null): LoggerProviderInterface + public function create(?MeterProviderInterface $meterProvider = null, ?ResourceInfo $resource = null): LoggerProviderInterface { if (Sdk::isDisabled()) { return NoopLoggerProvider::getInstance(); @@ -19,6 +20,6 @@ public function create(?MeterProviderInterface $meterProvider = null): LoggerPro $processor = (new LogRecordProcessorFactory())->create($exporter, $meterProvider); $instrumentationScopeFactory = new InstrumentationScopeFactory((new LogRecordLimitsBuilder())->build()->getAttributeFactory()); - return new LoggerProvider($processor, $instrumentationScopeFactory); + return new LoggerProvider($processor, $instrumentationScopeFactory, $resource); } } diff --git a/src/SDK/Metrics/MeterProviderFactory.php b/src/SDK/Metrics/MeterProviderFactory.php index 5f7f9988d..0f62cec5f 100644 --- a/src/SDK/Metrics/MeterProviderFactory.php +++ b/src/SDK/Metrics/MeterProviderFactory.php @@ -16,6 +16,7 @@ use OpenTelemetry\SDK\Metrics\MetricExporter\NoopMetricExporter; use OpenTelemetry\SDK\Metrics\MetricReader\ExportingReader; use OpenTelemetry\SDK\Registry; +use OpenTelemetry\SDK\Resource\ResourceInfo; use OpenTelemetry\SDK\Resource\ResourceInfoFactory; use OpenTelemetry\SDK\Sdk; @@ -28,7 +29,7 @@ class MeterProviderFactory * - "The exporter MUST configure the default aggregation on the basis of instrument kind using the * OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION variable as described below if it is implemented." */ - public function create(): MeterProviderInterface + public function create(?ResourceInfo $resource = null): MeterProviderInterface { if (Sdk::isDisabled()) { return new NoopMeterProvider(); @@ -50,7 +51,7 @@ public function create(): MeterProviderInterface // @todo "The exporter MUST be paired with a periodic exporting MetricReader" $reader = new ExportingReader($exporter); - $resource = ResourceInfoFactory::defaultResource(); + $resource ??= ResourceInfoFactory::defaultResource(); $exemplarFilter = $this->createExemplarFilter(Configuration::getEnum(Variables::OTEL_METRICS_EXEMPLAR_FILTER)); return MeterProvider::builder() diff --git a/src/SDK/Resource/ResourceInfo.php b/src/SDK/Resource/ResourceInfo.php index 4210a6142..a5dc1eb18 100644 --- a/src/SDK/Resource/ResourceInfo.php +++ b/src/SDK/Resource/ResourceInfo.php @@ -50,7 +50,7 @@ public function serialize(): string $copyOfAttributesAsArray = array_slice($this->attributes->toArray(), 0); //This may be overly cautious (in trying to avoid mutating the source array) ksort($copyOfAttributesAsArray); //sort the associative array by keys since the serializer will consider equal arrays different otherwise - //The exact return value doesn't matter, as long as it can distingusih between instances that represent the same/different resources + //The exact return value doesn't matter, as long as it can distinguish between instances that represent the same/different resources return serialize([ 'schemaUrl' => $this->schemaUrl, 'attributes' => $copyOfAttributesAsArray, @@ -62,6 +62,7 @@ public function serialize(): string * resource, the value of the updating resource MUST be picked (even if the updated value is empty) * * @see https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/resource/sdk.md#merge + * @todo can we optimize this to avoid re-validating the attributes on merge? */ public function merge(ResourceInfo $updating): ResourceInfo { diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index c08195e19..246f578cd 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -13,6 +13,7 @@ use OpenTelemetry\SDK\Logs\LoggerProviderFactory; use OpenTelemetry\SDK\Metrics\MeterProviderFactory; use OpenTelemetry\SDK\Propagation\PropagatorFactory; +use OpenTelemetry\SDK\Resource\ResourceInfoFactory; use OpenTelemetry\SDK\Trace\ExporterFactory; use OpenTelemetry\SDK\Trace\SamplerFactory; use OpenTelemetry\SDK\Trace\SpanProcessorFactory; @@ -41,15 +42,17 @@ public static function autoload(): bool } $emitMetrics = Configuration::getBoolean(Variables::OTEL_PHP_INTERNAL_METRICS_ENABLED); + $resource = ResourceInfoFactory::defaultResource(); $exporter = (new ExporterFactory())->create(); - $meterProvider = (new MeterProviderFactory())->create(); + $meterProvider = (new MeterProviderFactory())->create($resource); $spanProcessor = (new SpanProcessorFactory())->create($exporter, $emitMetrics ? $meterProvider : null); $tracerProvider = (new TracerProviderBuilder()) ->addSpanProcessor($spanProcessor) + ->setResource($resource) ->setSampler((new SamplerFactory())->create()) ->build(); - $loggerProvider = (new LoggerProviderFactory())->create($emitMetrics ? $meterProvider : null); + $loggerProvider = (new LoggerProviderFactory())->create($emitMetrics ? $meterProvider : null, $resource); ShutdownHandler::register([$tracerProvider, 'shutdown']); ShutdownHandler::register([$meterProvider, 'shutdown']); diff --git a/tests/Benchmark/ResourceCreationBench.php b/tests/Benchmark/ResourceCreationBench.php new file mode 100644 index 000000000..281a43f9e --- /dev/null +++ b/tests/Benchmark/ResourceCreationBench.php @@ -0,0 +1,20 @@ +