From be76d192d2a0f26bf630079b88bfb0f8f5364918 Mon Sep 17 00:00:00 2001 From: Tobias Bachert Date: Wed, 24 Apr 2024 21:50:25 +0200 Subject: [PATCH 01/43] [WIP] Add instrumentation configuration --- .../configure_instrumentation.php | 99 +++++++++++++++++++ .../instrumentation/otel-instrumentation.yaml | 3 + examples/instrumentation/otel-sdk.yaml | 7 ++ examples/src/Example.php | 9 ++ examples/src/ExampleConfig.php | 12 +++ examples/src/ExampleConfigProvider.php | 40 ++++++++ examples/src/ExampleInstrumentation.php | 47 +++++++++ .../ConfigurationRegistry.php | 22 +++++ .../ExtensionHookManager.php | 63 ++++++++++++ .../AutoInstrumentation/HookManager.php | 19 ++++ .../AutoInstrumentation/Instrumentation.php | 10 ++ .../InstrumentationConfiguration.php | 6 ++ .../AutoInstrumentation/NoopHookManager.php | 20 ++++ 13 files changed, 357 insertions(+) create mode 100644 examples/instrumentation/configure_instrumentation.php create mode 100644 examples/instrumentation/otel-instrumentation.yaml create mode 100644 examples/instrumentation/otel-sdk.yaml create mode 100644 examples/src/Example.php create mode 100644 examples/src/ExampleConfig.php create mode 100644 examples/src/ExampleConfigProvider.php create mode 100644 examples/src/ExampleInstrumentation.php create mode 100644 src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php create mode 100644 src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php create mode 100644 src/API/Instrumentation/AutoInstrumentation/HookManager.php create mode 100644 src/API/Instrumentation/AutoInstrumentation/Instrumentation.php create mode 100644 src/API/Instrumentation/AutoInstrumentation/InstrumentationConfiguration.php create mode 100644 src/API/Instrumentation/AutoInstrumentation/NoopHookManager.php diff --git a/examples/instrumentation/configure_instrumentation.php b/examples/instrumentation/configure_instrumentation.php new file mode 100644 index 000000000..6a5ae9d0f --- /dev/null +++ b/examples/instrumentation/configure_instrumentation.php @@ -0,0 +1,99 @@ +create(new Context())->setAutoShutdown(true)->build(); +$configuration = parseInstrumentationConfig(__DIR__ . '/otel-instrumentation.yaml')->create(new Context()); + +$hookManager = hookManager(); +$context = new Context($sdk->getTracerProvider()); +$storage = \OpenTelemetry\Context\Context::storage(); + +foreach (ServiceLoader::load(Instrumentation::class) as $instrumentation) { + $instrumentation->register($hookManager, $context, $configuration, $storage); +} + +$scope = $storage->attach($hookManager->enable($storage->current())); + +try { + echo (new Example())->test(), PHP_EOL; +} finally { + $scope->detach(); +} + + +function hookManager(): HookManager { + foreach (ServiceLoader::load(HookManager::class) as $hookManager) { + return $hookManager; + } + + return new NoopHookManager(); +} + +function parseInstrumentationConfig(string $file): ComponentPlugin { + // TODO Include in SDK config? + return (new ConfigurationFactory( + ServiceLoader::load(ComponentProvider::class), + new class implements ComponentProvider { + + /** + * @param array{ + * config: list>, + * } $properties + */ + public function createPlugin(array $properties, Context $context): ConfigurationRegistry { + $configurationRegistry = new ConfigurationRegistry(); + foreach ($properties['config'] as $configuration) { + $configurationRegistry->add($configuration->create($context)); + } + + return $configurationRegistry; + } + + public function getConfig(ComponentProviderRegistry $registry): ArrayNodeDefinition { + $root = new ArrayNodeDefinition('instrumentation'); + $root + ->children() + // TODO add disabled_instrumentations arrayNode to allow disabling specific instrumentation classes? + ->append($registry->componentList('config', InstrumentationConfiguration::class)) + ->end() + ; + + return $root; + } + }, + new EnvSourceReader([ + new ServerEnvSource(), + new PhpIniEnvSource(), + ]), + ))->parseFile($file); +} diff --git a/examples/instrumentation/otel-instrumentation.yaml b/examples/instrumentation/otel-instrumentation.yaml new file mode 100644 index 000000000..908e10659 --- /dev/null +++ b/examples/instrumentation/otel-instrumentation.yaml @@ -0,0 +1,3 @@ +config: + - example_instrumentation: + span_name: ${EXAMPLE_INSTRUMENTATION_SPAN_NAME:-example span} diff --git a/examples/instrumentation/otel-sdk.yaml b/examples/instrumentation/otel-sdk.yaml new file mode 100644 index 000000000..bccdf3470 --- /dev/null +++ b/examples/instrumentation/otel-sdk.yaml @@ -0,0 +1,7 @@ +file_format: '0.1' + +tracer_provider: + processors: + - simple: + exporter: + console: {} diff --git a/examples/src/Example.php b/examples/src/Example.php new file mode 100644 index 000000000..4534723b3 --- /dev/null +++ b/examples/src/Example.php @@ -0,0 +1,9 @@ + + */ +final class ExampleConfigProvider implements ComponentProvider { + + /** + * @param array{ + * span_name: string, + * enabled: bool, + * } $properties + */ + public function createPlugin(array $properties, Context $context): InstrumentationConfiguration { + return new ExampleConfig( + spanName: $properties['span_name'], + enabled: $properties['enabled'], + ); + } + + public function getConfig(ComponentProviderRegistry $registry): ArrayNodeDefinition { + $root = new ArrayNodeDefinition('example_instrumentation'); + $root + ->children() + ->scalarNode('span_name')->isRequired()->validate()->always(Validation::ensureString())->end()->end() + ->end() + ->canBeDisabled() + ; + + return $root; + } +} diff --git a/examples/src/ExampleInstrumentation.php b/examples/src/ExampleInstrumentation.php new file mode 100644 index 000000000..6edf6667b --- /dev/null +++ b/examples/src/ExampleInstrumentation.php @@ -0,0 +1,47 @@ +get(ExampleConfig::class) ?? throw new Exception('example instrumentation must be configured'); + if (!$config->enabled) { + return; + } + + $tracer = $context->tracerProvider->getTracer('example-instrumentation'); + + $hookManager->hook( + Example::class, + 'test', + static function() use ($tracer, $config, $storage): void { + $context = $storage->current(); + + $span = $tracer + ->spanBuilder($config->spanName) + ->setParent($context) + ->startSpan(); + + $storage->attach($span->storeInContext($context)); + }, + static function() use ($storage): void { + if (!$scope = $storage->scope()) { + return; + } + + $scope->detach(); + + $span = Span::fromContext($scope->context()); + $span->end(); + } + ); + } +} diff --git a/src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php b/src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php new file mode 100644 index 000000000..d5c70f7fe --- /dev/null +++ b/src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php @@ -0,0 +1,22 @@ +configurations[$configuration::class] = $configuration; + + return $this; + } + + /** + * @template C of InstrumentationConfiguration + * @param class-string $id + * @return C|null + */ + public function get(string $id): ?InstrumentationConfiguration { + return $this->configurations[$id] ?? null; + } +} diff --git a/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php b/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php new file mode 100644 index 000000000..aee2c65d0 --- /dev/null +++ b/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php @@ -0,0 +1,63 @@ +contextKey = Context::createKey(self::class); + } + + public function hook(?string $class, string $function, ?Closure $preHook = null, ?Closure $postHook = null): void { + assert(extension_loaded('opentelemetry')); + + /** @noinspection PhpFullyQualifiedNameUsageInspection */ + \OpenTelemetry\Instrumentation\hook($class, $function, $this->bindHookScope($preHook), $this->bindHookScope($postHook)); + } + + public function enable(Context $context): Context { + return $context->with($this->contextKey, true); + } + + public function disable(Context $context): Context { + return $context->with($this->contextKey, null); + } + + private function bindHookScope(?Closure $closure): ?Closure { + if (!$closure) { + return null; + } + + $contextKey = $this->contextKey; + $reflection = new ReflectionFunction($closure); + + // TODO Add an option flag to ext-opentelemetry `hook` that configures whether return values should be used? + if (!$reflection->getReturnType() || (string) $reflection->getReturnType() === 'void') { + return static function(mixed ...$args) use ($closure, $contextKey): void { + if (!Context::getCurrent()->get($contextKey)) { + return; + } + + $closure(...$args); + }; + } + + return static function(mixed ...$args) use ($closure, $contextKey): mixed { + if (!Context::getCurrent()->get($contextKey)) { + return null; + } + + return $closure(...$args); + }; + } +} diff --git a/src/API/Instrumentation/AutoInstrumentation/HookManager.php b/src/API/Instrumentation/AutoInstrumentation/HookManager.php new file mode 100644 index 000000000..3685d211a --- /dev/null +++ b/src/API/Instrumentation/AutoInstrumentation/HookManager.php @@ -0,0 +1,19 @@ + Date: Mon, 6 May 2024 21:09:25 +1000 Subject: [PATCH 02/43] add autoloading for auto-instrumentations using SPI --- composer.json | 10 +- .../configure_instrumentation.php | 83 ++--------- .../configure_instrumentation_global.php | 20 +++ examples/src/Example.php | 11 +- examples/src/ExampleConfig.php | 11 +- examples/src/ExampleConfigProvider.php | 18 ++- examples/src/ExampleInstrumentation.php | 21 ++- src/API/Globals.php | 17 ++- .../ConfigurationRegistry.php | 14 +- .../ExtensionHookManager.php | 45 +++--- .../AutoInstrumentation/HookManager.php | 8 +- .../AutoInstrumentation/Instrumentation.php | 8 +- .../InstrumentationConfiguration.php | 8 +- .../AutoInstrumentation/NoopHookManager.php | 17 ++- src/API/Instrumentation/Configurator.php | 15 ++ src/API/Instrumentation/ContextKeys.php | 8 ++ .../Instrumentation/ConfigurationRegistry.php | 50 +++++++ .../OpenTelemetryInstrumentation.php | 50 +++++++ src/Config/SDK/Instrumentation.php | 63 +++++++++ src/SDK/Common/Configuration/Variables.php | 2 + src/SDK/Sdk.php | 7 + src/SDK/SdkAutoloader.php | 133 +++++++++++++----- src/SDK/SdkBuilder.php | 13 ++ tests/Unit/SDK/SdkTest.php | 5 +- 24 files changed, 480 insertions(+), 157 deletions(-) create mode 100644 examples/instrumentation/configure_instrumentation_global.php create mode 100644 src/Config/SDK/ComponentProvider/Instrumentation/ConfigurationRegistry.php create mode 100644 src/Config/SDK/ComponentProvider/OpenTelemetryInstrumentation.php create mode 100644 src/Config/SDK/Instrumentation.php diff --git a/composer.json b/composer.json index 6f53c3354..0e8745588 100644 --- a/composer.json +++ b/composer.json @@ -145,7 +145,15 @@ "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Logs\\LogRecordExporterConsole", "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Logs\\LogRecordExporterOtlp", "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Logs\\LogRecordProcessorBatch", - "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Logs\\LogRecordProcessorSimple" + "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Logs\\LogRecordProcessorSimple", + + "OpenTelemetry\\Example\\ExampleConfigProvider" + ], + "OpenTelemetry\\API\\Instrumentation\\AutoInstrumentation\\HookManager": [ + "OpenTelemetry\\API\\Instrumentation\\AutoInstrumentation\\ExtensionHookManager" + ], + "OpenTelemetry\\API\\Instrumentation\\AutoInstrumentation\\Instrumentation": [ + "OpenTelemetry\\Example\\ExampleInstrumentation" ] } } diff --git a/examples/instrumentation/configure_instrumentation.php b/examples/instrumentation/configure_instrumentation.php index 6a5ae9d0f..5f434f83a 100644 --- a/examples/instrumentation/configure_instrumentation.php +++ b/examples/instrumentation/configure_instrumentation.php @@ -1,99 +1,38 @@ -create(new Context())->setAutoShutdown(true)->setHookManager(new ExtensionHookManager())->build(); +$configuration = \OpenTelemetry\Config\SDK\Instrumentation::parseFile(__DIR__ . '/otel-instrumentation.yaml')->create(); -$sdk = Configuration::parseFile(__DIR__ . '/otel-sdk.yaml')->create(new Context())->setAutoShutdown(true)->build(); -$configuration = parseInstrumentationConfig(__DIR__ . '/otel-instrumentation.yaml')->create(new Context()); - -$hookManager = hookManager(); $context = new Context($sdk->getTracerProvider()); $storage = \OpenTelemetry\Context\Context::storage(); foreach (ServiceLoader::load(Instrumentation::class) as $instrumentation) { - $instrumentation->register($hookManager, $context, $configuration, $storage); + $instrumentation->register($sdk->getHookManager(), $context, $configuration, $storage); } -$scope = $storage->attach($hookManager->enable($storage->current())); +$scope = $storage->attach($sdk->getHookManager()->enable($storage->current())); try { echo (new Example())->test(), PHP_EOL; } finally { $scope->detach(); } - - -function hookManager(): HookManager { - foreach (ServiceLoader::load(HookManager::class) as $hookManager) { - return $hookManager; - } - - return new NoopHookManager(); -} - -function parseInstrumentationConfig(string $file): ComponentPlugin { - // TODO Include in SDK config? - return (new ConfigurationFactory( - ServiceLoader::load(ComponentProvider::class), - new class implements ComponentProvider { - - /** - * @param array{ - * config: list>, - * } $properties - */ - public function createPlugin(array $properties, Context $context): ConfigurationRegistry { - $configurationRegistry = new ConfigurationRegistry(); - foreach ($properties['config'] as $configuration) { - $configurationRegistry->add($configuration->create($context)); - } - - return $configurationRegistry; - } - - public function getConfig(ComponentProviderRegistry $registry): ArrayNodeDefinition { - $root = new ArrayNodeDefinition('instrumentation'); - $root - ->children() - // TODO add disabled_instrumentations arrayNode to allow disabling specific instrumentation classes? - ->append($registry->componentList('config', InstrumentationConfiguration::class)) - ->end() - ; - - return $root; - } - }, - new EnvSourceReader([ - new ServerEnvSource(), - new PhpIniEnvSource(), - ]), - ))->parseFile($file); -} diff --git a/examples/instrumentation/configure_instrumentation_global.php b/examples/instrumentation/configure_instrumentation_global.php new file mode 100644 index 000000000..496d25b6e --- /dev/null +++ b/examples/instrumentation/configure_instrumentation_global.php @@ -0,0 +1,20 @@ +test(), PHP_EOL; diff --git a/examples/src/Example.php b/examples/src/Example.php index 4534723b3..c21077762 100644 --- a/examples/src/Example.php +++ b/examples/src/Example.php @@ -1,9 +1,14 @@ - */ -final class ExampleConfigProvider implements ComponentProvider { +final class ExampleConfigProvider implements ComponentProvider +{ /** + * @psalm-suppress MoreSpecificImplementedParamType * @param array{ * span_name: string, * enabled: bool, * } $properties */ - public function createPlugin(array $properties, Context $context): InstrumentationConfiguration { + public function createPlugin(array $properties, Context $context): InstrumentationConfiguration + { return new ExampleConfig( spanName: $properties['span_name'], enabled: $properties['enabled'], ); } - public function getConfig(ComponentProviderRegistry $registry): ArrayNodeDefinition { + /** + * @psalm-suppress UndefinedInterfaceMethod + */ + public function getConfig(ComponentProviderRegistry $registry): ArrayNodeDefinition + { $root = new ArrayNodeDefinition('example_instrumentation'); $root ->children() diff --git a/examples/src/ExampleInstrumentation.php b/examples/src/ExampleInstrumentation.php index 6edf6667b..7f0816ba4 100644 --- a/examples/src/ExampleInstrumentation.php +++ b/examples/src/ExampleInstrumentation.php @@ -1,29 +1,38 @@ -get(ExampleConfig::class) ?? throw new Exception('example instrumentation must be configured'); if (!$config->enabled) { return; } - $tracer = $context->tracerProvider->getTracer('example-instrumentation'); + //$tracer = $context->tracerProvider->getTracer('example-instrumentation'); $hookManager->hook( Example::class, 'test', - static function() use ($tracer, $config, $storage): void { + static function () use (/*$tracer,*/ $config, $storage): void { + static $instrumentation; + $instrumentation ??= new CachedInstrumentation('example-instrumentation'); $context = $storage->current(); + $tracer = $instrumentation->tracer(); $span = $tracer ->spanBuilder($config->spanName) @@ -32,7 +41,7 @@ static function() use ($tracer, $config, $storage): void { $storage->attach($span->storeInContext($context)); }, - static function() use ($storage): void { + static function () use ($storage): void { if (!$scope = $storage->scope()) { return; } diff --git a/src/API/Globals.php b/src/API/Globals.php index 7d296f4fc..489acb2fb 100644 --- a/src/API/Globals.php +++ b/src/API/Globals.php @@ -7,6 +7,7 @@ use function assert; use Closure; use const E_USER_WARNING; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\API\Instrumentation\ContextKeys; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; @@ -34,6 +35,7 @@ public function __construct( private readonly LoggerProviderInterface $loggerProvider, private readonly EventLoggerProviderInterface $eventLoggerProvider, private readonly TextMapPropagatorInterface $propagator, + private readonly HookManager $hookManager, ) { } @@ -62,6 +64,11 @@ public static function eventLoggerProvider(): EventLoggerProviderInterface return Context::getCurrent()->get(ContextKeys::eventLoggerProvider()) ?? self::globals()->eventLoggerProvider; } + public static function hookManager(): HookManager + { + return Context::getCurrent()->get(ContextKeys::hookManager()) ?? self::globals()->hookManager; + } + /** * @param Closure(Configurator): Configurator $initializer * @@ -103,10 +110,16 @@ private static function globals(): self $propagator = $context->get(ContextKeys::propagator()); $loggerProvider = $context->get(ContextKeys::loggerProvider()); $eventLoggerProvider = $context->get(ContextKeys::eventLoggerProvider()); + $hookManager = $context->get(ContextKeys::hookManager()); - assert(isset($tracerProvider, $meterProvider, $loggerProvider, $eventLoggerProvider, $propagator)); + assert(isset($tracerProvider, $meterProvider, $loggerProvider, $eventLoggerProvider, $propagator, $hookManager)); - return self::$globals = new self($tracerProvider, $meterProvider, $loggerProvider, $eventLoggerProvider, $propagator); + return self::$globals = new self($tracerProvider, $meterProvider, $loggerProvider, $eventLoggerProvider, $propagator, $hookManager); + } + + public static function init(): void + { + self::globals(); } /** diff --git a/src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php b/src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php index d5c70f7fe..06413c186 100644 --- a/src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php +++ b/src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php @@ -1,11 +1,16 @@ -configurations[$configuration::class] = $configuration; return $this; @@ -16,7 +21,8 @@ public function add(InstrumentationConfiguration $configuration): self { * @param class-string $id * @return C|null */ - public function get(string $id): ?InstrumentationConfiguration { + public function get(string $id): ?InstrumentationConfiguration + { return $this->configurations[$id] ?? null; } } diff --git a/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php b/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php index aee2c65d0..d1b674aae 100644 --- a/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php +++ b/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php @@ -1,39 +1,50 @@ -contextKey = Context::createKey(self::class); } - public function hook(?string $class, string $function, ?Closure $preHook = null, ?Closure $postHook = null): void { + /** + * @phan-suppress PhanUndeclaredFunction + */ + public function hook(?string $class, string $function, ?Closure $preHook = null, ?Closure $postHook = null): void + { assert(extension_loaded('opentelemetry')); /** @noinspection PhpFullyQualifiedNameUsageInspection */ \OpenTelemetry\Instrumentation\hook($class, $function, $this->bindHookScope($preHook), $this->bindHookScope($postHook)); } - public function enable(Context $context): Context { + public function enable(Context $context): Context + { return $context->with($this->contextKey, true); } - public function disable(Context $context): Context { + public function disable(Context $context): Context + { return $context->with($this->contextKey, null); } - private function bindHookScope(?Closure $closure): ?Closure { + private function bindHookScope(?Closure $closure): ?Closure + { if (!$closure) { return null; } @@ -43,19 +54,19 @@ private function bindHookScope(?Closure $closure): ?Closure { // TODO Add an option flag to ext-opentelemetry `hook` that configures whether return values should be used? if (!$reflection->getReturnType() || (string) $reflection->getReturnType() === 'void') { - return static function(mixed ...$args) use ($closure, $contextKey): void { - if (!Context::getCurrent()->get($contextKey)) { + return static function (mixed ...$args) use ($closure, $contextKey): void { + /*if (!Context::getCurrent()->get($contextKey)) { return; - } + }*/ $closure(...$args); }; } - return static function(mixed ...$args) use ($closure, $contextKey): mixed { - if (!Context::getCurrent()->get($contextKey)) { + return static function (mixed ...$args) use ($closure, $contextKey): mixed { + /*if (!Context::getCurrent()->get($contextKey)) { return null; - } + }*/ return $closure(...$args); }; diff --git a/src/API/Instrumentation/AutoInstrumentation/HookManager.php b/src/API/Instrumentation/AutoInstrumentation/HookManager.php index 3685d211a..db57dbadf 100644 --- a/src/API/Instrumentation/AutoInstrumentation/HookManager.php +++ b/src/API/Instrumentation/AutoInstrumentation/HookManager.php @@ -1,11 +1,15 @@ -withPropagator(new NoopTextMapPropagator()) ->withLoggerProvider(NoopLoggerProvider::getInstance()) ->withEventLoggerProvider(new NoopEventLoggerProvider()) + ->withHookManager(new NoopHookManager()) ; } @@ -82,6 +86,9 @@ public function storeInContext(?ContextInterface $context = null): ContextInterf if ($this->eventLoggerProvider !== null) { $context = $context->with(ContextKeys::eventLoggerProvider(), $this->eventLoggerProvider); } + if ($this->hookManager !== null) { + $context = $context->with(ContextKeys::hookManager(), $this->hookManager); + } return $context; } @@ -125,4 +132,12 @@ public function withEventLoggerProvider(?EventLoggerProviderInterface $eventLogg return $self; } + + public function withHookManager(?HookManager $hookManager): Configurator + { + $self = clone $this; + $self->hookManager = $hookManager; + + return $self; + } } diff --git a/src/API/Instrumentation/ContextKeys.php b/src/API/Instrumentation/ContextKeys.php index 9f53e5b86..5840f055b 100644 --- a/src/API/Instrumentation/ContextKeys.php +++ b/src/API/Instrumentation/ContextKeys.php @@ -4,6 +4,7 @@ namespace OpenTelemetry\API\Instrumentation; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; use OpenTelemetry\API\Logs\LoggerProviderInterface; use OpenTelemetry\API\Metrics\MeterProviderInterface; @@ -66,4 +67,11 @@ public static function eventLoggerProvider(): ContextKeyInterface return $instance ??= Context::createKey(EventLoggerProviderInterface::class); } + + public static function hookManager(): ContextKeyInterface + { + static $instance; + + return $instance ??= Context::createKey(HookManager::class); + } } diff --git a/src/Config/SDK/ComponentProvider/Instrumentation/ConfigurationRegistry.php b/src/Config/SDK/ComponentProvider/Instrumentation/ConfigurationRegistry.php new file mode 100644 index 000000000..e58f2ec58 --- /dev/null +++ b/src/Config/SDK/ComponentProvider/Instrumentation/ConfigurationRegistry.php @@ -0,0 +1,50 @@ + + */ +class ConfigurationRegistry implements ComponentProvider +{ + /** + * @param array{ + * config: list>, + * } $properties + */ + public function createPlugin(array $properties, Context $context): ConfigurationRegistryComponent + { + $configurationRegistry = new ConfigurationRegistryComponent(); + foreach ($properties['config'] as $configuration) { + $configurationRegistry->add($configuration->create($context)); + } + + return $configurationRegistry; + } + + public function getConfig(ComponentProviderRegistry $registry): ArrayNodeDefinition + { + $root = new ArrayNodeDefinition('instrumentation'); + $root + ->children() + // TODO add disabled_instrumentations arrayNode to allow disabling specific instrumentation classes? + ->append($registry->componentList('config', InstrumentationConfiguration::class)) + ->end() + ; + + return $root; + } +} diff --git a/src/Config/SDK/ComponentProvider/OpenTelemetryInstrumentation.php b/src/Config/SDK/ComponentProvider/OpenTelemetryInstrumentation.php new file mode 100644 index 000000000..4eb413ddf --- /dev/null +++ b/src/Config/SDK/ComponentProvider/OpenTelemetryInstrumentation.php @@ -0,0 +1,50 @@ + + */ +class OpenTelemetryInstrumentation implements ComponentProvider +{ + /** + * @param array{ + * config: list>, + * } $properties + */ + public function createPlugin(array $properties, Context $context): ConfigurationRegistry + { + $configurationRegistry = new ConfigurationRegistry(); + foreach ($properties['config'] as $configuration) { + $configurationRegistry->add($configuration->create($context)); + } + + return $configurationRegistry; + } + + public function getConfig(ComponentProviderRegistry $registry): ArrayNodeDefinition + { + $root = new ArrayNodeDefinition('instrumentation'); + $root + ->children() + // TODO add disabled_instrumentations arrayNode to allow disabling specific instrumentation classes? + ->append($registry->componentList('config', InstrumentationConfiguration::class)) + ->end() + ; + + return $root; + } +} diff --git a/src/Config/SDK/Instrumentation.php b/src/Config/SDK/Instrumentation.php new file mode 100644 index 000000000..8f4238c54 --- /dev/null +++ b/src/Config/SDK/Instrumentation.php @@ -0,0 +1,63 @@ + $plugin + */ + private function __construct( + private readonly ComponentPlugin $plugin, + ) { + } + + public function create(Context $context = new Context()): ConfigurationRegistry + { + $plugin = $this->plugin; + + return $plugin->create($context); + } + + /** + * @param string|list $file + */ + public static function parseFile( + string|array $file, + ?string $cacheFile = null, + bool $debug = true, + ): Instrumentation { + return new self(self::factory()->parseFile($file, $cacheFile, $debug)); + } + + /** + * @return ConfigurationFactory + */ + private static function factory(): ConfigurationFactory + { + static $factory; + + return $factory ??= new ConfigurationFactory( + ServiceLoader::load(ComponentProvider::class), + new OpenTelemetryInstrumentation(), + new EnvSourceReader([ + new ServerEnvSource(), + new PhpIniEnvSource(), + ]), + ); + } +} diff --git a/src/SDK/Common/Configuration/Variables.php b/src/SDK/Common/Configuration/Variables.php index 8ccb28ac1..76ae3c8e8 100644 --- a/src/SDK/Common/Configuration/Variables.php +++ b/src/SDK/Common/Configuration/Variables.php @@ -140,4 +140,6 @@ interface Variables public const OTEL_PHP_INTERNAL_METRICS_ENABLED = 'OTEL_PHP_INTERNAL_METRICS_ENABLED'; //whether the SDK should emit its own metrics public const OTEL_PHP_DISABLED_INSTRUMENTATIONS = 'OTEL_PHP_DISABLED_INSTRUMENTATIONS'; public const OTEL_PHP_EXCLUDED_URLS = 'OTEL_PHP_EXCLUDED_URLS'; + public const OTEL_PHP_SDK_CONFIG_FILE = 'OTEL_PHP_SDK_CONFIG_FILE'; + public const OTEL_PHP_INSTRUMENTATION_CONFIG_FILE = 'OTEL_PHP_INSTRUMENTATION_CONFIG_FILE'; } diff --git a/src/SDK/Sdk.php b/src/SDK/Sdk.php index 78f541f64..03af0324a 100644 --- a/src/SDK/Sdk.php +++ b/src/SDK/Sdk.php @@ -4,6 +4,7 @@ namespace OpenTelemetry\SDK; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; use OpenTelemetry\API\Metrics\MeterProviderInterface; use OpenTelemetry\API\Trace\TracerProviderInterface; @@ -22,6 +23,7 @@ public function __construct( private readonly LoggerProviderInterface $loggerProvider, private readonly EventLoggerProviderInterface $eventLoggerProvider, private readonly TextMapPropagatorInterface $propagator, + private readonly HookManager $hookManager, ) { } @@ -69,4 +71,9 @@ public function getPropagator(): TextMapPropagatorInterface { return $this->propagator; } + + public function getHookManager(): HookManager + { + return $this->hookManager; + } } diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index f204d8540..9d71738be 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -5,8 +5,16 @@ namespace OpenTelemetry\SDK; use InvalidArgumentException; +use Nevay\SPI\ServiceLoader; use OpenTelemetry\API\Globals; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ExtensionHookManager; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\NoopHookManager; use OpenTelemetry\API\Instrumentation\Configurator; +use OpenTelemetry\Config\SDK\Configuration as SdkConfiguration; +use OpenTelemetry\Config\SDK\Configuration\Context as ConfigContext; +use OpenTelemetry\Config\SDK\Instrumentation; +use OpenTelemetry\Context\Context; use OpenTelemetry\SDK\Common\Configuration\Configuration; use OpenTelemetry\SDK\Common\Configuration\Variables; use OpenTelemetry\SDK\Common\Util\ShutdownHandler; @@ -30,43 +38,102 @@ public static function autoload(): bool if (!self::isEnabled() || self::isExcludedUrl()) { return false; } - Globals::registerInitializer(function (Configurator $configurator) { - $propagator = (new PropagatorFactory())->create(); - if (Sdk::isDisabled()) { - //@see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-environment-variables.md#general-sdk-configuration - return $configurator->withPropagator($propagator); - } - $emitMetrics = Configuration::getBoolean(Variables::OTEL_PHP_INTERNAL_METRICS_ENABLED); - - $resource = ResourceInfoFactory::defaultResource(); - $exporter = (new ExporterFactory())->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, $resource); - $eventLoggerProvider = (new EventLoggerProviderFactory())->create($loggerProvider); - - ShutdownHandler::register($tracerProvider->shutdown(...)); - ShutdownHandler::register($meterProvider->shutdown(...)); - ShutdownHandler::register($loggerProvider->shutdown(...)); - - return $configurator - ->withTracerProvider($tracerProvider) - ->withMeterProvider($meterProvider) - ->withLoggerProvider($loggerProvider) - ->withEventLoggerProvider($eventLoggerProvider) - ->withPropagator($propagator) - ; - }); + if (Configuration::has(Variables::OTEL_PHP_SDK_CONFIG_FILE)) { + Globals::registerInitializer(fn ($configurator) => self::configFileBasedSdkInitializer($configurator)); + self::configureInstrumentation(); + } else { + Globals::registerInitializer(fn ($configurator) => self::environmentBasedInitializer($configurator)); + } return true; } + public static function environmentBasedInitializer(Configurator $configurator): Configurator + { + $propagator = (new PropagatorFactory())->create(); + if (Sdk::isDisabled()) { + //@see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-environment-variables.md#general-sdk-configuration + return $configurator->withPropagator($propagator); + } + $hookManager = new ExtensionHookManager(); //to should be enabled/disable by env? + $emitMetrics = Configuration::getBoolean(Variables::OTEL_PHP_INTERNAL_METRICS_ENABLED); + + $resource = ResourceInfoFactory::defaultResource(); + $exporter = (new ExporterFactory())->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, $resource); + $eventLoggerProvider = (new EventLoggerProviderFactory())->create($loggerProvider); + + ShutdownHandler::register($tracerProvider->shutdown(...)); + ShutdownHandler::register($meterProvider->shutdown(...)); + ShutdownHandler::register($loggerProvider->shutdown(...)); + + return $configurator + ->withTracerProvider($tracerProvider) + ->withMeterProvider($meterProvider) + ->withLoggerProvider($loggerProvider) + ->withEventLoggerProvider($eventLoggerProvider) + ->withPropagator($propagator) + ->withHookManager($hookManager) + ; + } + + public static function configFileBasedSdkInitializer(Configurator $configurator): Configurator + { + $file = Configuration::getString(Variables::OTEL_PHP_SDK_CONFIG_FILE); + $config = SdkConfiguration::parseFile($file); + $sdk = $config + ->create() + ->setAutoShutdown(true) + ->build(); + + return $configurator + ->withTracerProvider($sdk->getTracerProvider()) + ->withMeterProvider($sdk->getMeterProvider()) + ->withLoggerProvider($sdk->getLoggerProvider()) + ->withEventLoggerProvider($sdk->getEventLoggerProvider()) + ->withPropagator($sdk->getPropagator()) + ->withHookManager($sdk->getHookManager()) + ; + } + + /** + * @phan-suppress PhanUndeclaredClassMethod + */ + private static function configureInstrumentation(): void + { + if (!Configuration::has(Variables::OTEL_PHP_INSTRUMENTATION_CONFIG_FILE)) { + return; + } + $file = Configuration::getString(Variables::OTEL_PHP_INSTRUMENTATION_CONFIG_FILE); + $configuration = Instrumentation::parseFile($file)->create(); + $context = new ConfigContext(); + $storage = Context::storage(); + $hookManager = self::getHookManager(); + foreach (ServiceLoader::load(\OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation::class) as $instrumentation) { + $instrumentation->register($hookManager, $context, $configuration, $storage); + } + } + + /** + * @phan-suppress PhanUndeclaredClassMethod + */ + private static function getHookManager(): HookManager + { + foreach (ServiceLoader::load(HookManager::class) as $hookManager) { + return $hookManager; + } + + return new NoopHookManager(); + } + /** * Test whether a request URI is set, and if it matches the excluded urls configuration option * diff --git a/src/SDK/SdkBuilder.php b/src/SDK/SdkBuilder.php index 403713e0b..2a6f8c21b 100644 --- a/src/SDK/SdkBuilder.php +++ b/src/SDK/SdkBuilder.php @@ -4,6 +4,8 @@ namespace OpenTelemetry\SDK; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\NoopHookManager; use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; use OpenTelemetry\API\Logs\NoopEventLoggerProvider; @@ -26,6 +28,7 @@ class SdkBuilder private ?LoggerProviderInterface $loggerProvider = null; private ?EventLoggerProviderInterface $eventLoggerProvider = null; private ?TextMapPropagatorInterface $propagator = null; + private ?HookManager $hookManager = null; private bool $autoShutdown = false; /** @@ -73,12 +76,20 @@ public function setPropagator(TextMapPropagatorInterface $propagator): self return $this; } + public function setHookManager(HookManager $hookManager): self + { + $this->hookManager = $hookManager; + + return $this; + } + public function build(): Sdk { $tracerProvider = $this->tracerProvider ?? new NoopTracerProvider(); $meterProvider = $this->meterProvider ?? new NoopMeterProvider(); $loggerProvider = $this->loggerProvider ?? new NoopLoggerProvider(); $eventLoggerProvider = $this->eventLoggerProvider ?? new NoopEventLoggerProvider(); + $hookManager = $this->hookManager ?? new NoopHookManager(); if ($this->autoShutdown) { // rector rule disabled in config, because ShutdownHandler::register() does not keep a strong reference to $this ShutdownHandler::register($tracerProvider->shutdown(...)); @@ -92,6 +103,7 @@ public function build(): Sdk $loggerProvider, $eventLoggerProvider, $this->propagator ?? NoopTextMapPropagator::getInstance(), + $hookManager, ); } @@ -104,6 +116,7 @@ public function buildAndRegisterGlobal(): ScopeInterface ->withMeterProvider($sdk->getMeterProvider()) ->withLoggerProvider($sdk->getLoggerProvider()) ->withEventLoggerProvider($sdk->getEventLoggerProvider()) + ->withHookManager($sdk->getHookManager()) ->storeInContext(); return Context::storage()->attach($context); diff --git a/tests/Unit/SDK/SdkTest.php b/tests/Unit/SDK/SdkTest.php index be35ab3bb..8ca62df98 100644 --- a/tests/Unit/SDK/SdkTest.php +++ b/tests/Unit/SDK/SdkTest.php @@ -5,6 +5,7 @@ namespace OpenTelemetry\Tests\Unit\SDK; use AssertWell\PHPUnitGlobalState\EnvironmentVariables; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface; use OpenTelemetry\SDK\Common\Configuration\Variables; @@ -84,11 +85,13 @@ public function test_getters(): void $tracerProvider = $this->createMock(TracerProviderInterface::class); $loggerProvider = $this->createMock(LoggerProviderInterface::class); $eventLoggerProvider = $this->createMock(EventLoggerProviderInterface::class); - $sdk = new Sdk($tracerProvider, $meterProvider, $loggerProvider, $eventLoggerProvider, $propagator); + $hookManager = $this->createMock(HookManager::class); + $sdk = new Sdk($tracerProvider, $meterProvider, $loggerProvider, $eventLoggerProvider, $propagator, $hookManager); $this->assertSame($propagator, $sdk->getPropagator()); $this->assertSame($meterProvider, $sdk->getMeterProvider()); $this->assertSame($tracerProvider, $sdk->getTracerProvider()); $this->assertSame($loggerProvider, $sdk->getLoggerProvider()); $this->assertSame($eventLoggerProvider, $sdk->getEventLoggerProvider()); + $this->assertSame($hookManager, $sdk->getHookManager()); } } From 31f93c08f2b035146c3c1aa0b8db19ced8364579 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Mon, 6 May 2024 21:19:39 +1000 Subject: [PATCH 03/43] allow autoloading and non-autoloading to work --- examples/src/ExampleInstrumentation.php | 11 ++++------- .../AutoInstrumentation/Instrumentation.php | 7 +++++-- src/SDK/SdkAutoloader.php | 4 +--- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/examples/src/ExampleInstrumentation.php b/examples/src/ExampleInstrumentation.php index 7f0816ba4..971bf568f 100644 --- a/examples/src/ExampleInstrumentation.php +++ b/examples/src/ExampleInstrumentation.php @@ -5,10 +5,10 @@ namespace OpenTelemetry\Example; use Exception; +use OpenTelemetry\API\Globals; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ConfigurationRegistry; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation; -use OpenTelemetry\API\Instrumentation\CachedInstrumentation; use OpenTelemetry\API\Trace\Span; use OpenTelemetry\Config\SDK\Configuration\Context; use OpenTelemetry\Context\ContextStorageInterface; @@ -16,23 +16,20 @@ final class ExampleInstrumentation implements Instrumentation { - public function register(HookManager $hookManager, Context $context, ConfigurationRegistry $configuration, ContextStorageInterface $storage): void + public function register(HookManager $hookManager, ?Context $context, ConfigurationRegistry $configuration, ContextStorageInterface $storage): void { $config = $configuration->get(ExampleConfig::class) ?? throw new Exception('example instrumentation must be configured'); if (!$config->enabled) { return; } - //$tracer = $context->tracerProvider->getTracer('example-instrumentation'); + $tracer = $context ? $context->tracerProvider->getTracer('example-instrumentation') : Globals::tracerProvider()->getTracer('example-instrumentation'); $hookManager->hook( Example::class, 'test', - static function () use (/*$tracer,*/ $config, $storage): void { - static $instrumentation; - $instrumentation ??= new CachedInstrumentation('example-instrumentation'); + static function () use ($tracer, $config, $storage): void { $context = $storage->current(); - $tracer = $instrumentation->tracer(); $span = $tracer ->spanBuilder($config->spanName) diff --git a/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php b/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php index 2d9463c04..4c0d3d4f5 100644 --- a/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php +++ b/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php @@ -9,6 +9,9 @@ interface Instrumentation { - - public function register(HookManager $hookManager, Context $context, ConfigurationRegistry $configuration, ContextStorageInterface $storage): void; + /** + * @todo context is nullable in order to support autoloading (and retrieving lazy-loaded tracers), but auto and non-auto + * should work the same. + */ + public function register(HookManager $hookManager, ?Context $context, ConfigurationRegistry $configuration, ContextStorageInterface $storage): void; } diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index 9d71738be..7a391ebdb 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -12,7 +12,6 @@ use OpenTelemetry\API\Instrumentation\AutoInstrumentation\NoopHookManager; use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\Config\SDK\Configuration as SdkConfiguration; -use OpenTelemetry\Config\SDK\Configuration\Context as ConfigContext; use OpenTelemetry\Config\SDK\Instrumentation; use OpenTelemetry\Context\Context; use OpenTelemetry\SDK\Common\Configuration\Configuration; @@ -114,11 +113,10 @@ private static function configureInstrumentation(): void } $file = Configuration::getString(Variables::OTEL_PHP_INSTRUMENTATION_CONFIG_FILE); $configuration = Instrumentation::parseFile($file)->create(); - $context = new ConfigContext(); $storage = Context::storage(); $hookManager = self::getHookManager(); foreach (ServiceLoader::load(\OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation::class) as $instrumentation) { - $instrumentation->register($hookManager, $context, $configuration, $storage); + $instrumentation->register($hookManager, null, $configuration, $storage); } } From ef343e114fc7b489f4a285d02a47ba3d28ea8bb1 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 7 May 2024 09:24:55 +1000 Subject: [PATCH 04/43] fix attribute --- .../AutoInstrumentation/ExtensionHookManager.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php b/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php index d1b674aae..396f54243 100644 --- a/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php +++ b/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php @@ -7,11 +7,12 @@ use function assert; use Closure; use function extension_loaded; +use Nevay\SPI\ServiceProviderDependency\ExtensionDependency; use OpenTelemetry\Context\Context; use OpenTelemetry\Context\ContextKeyInterface; use ReflectionFunction; -// #[Nevay\SPI\ServiceProviderDependency\ExtensionDependency('opentelemetry', '^1.0')] +#[ExtensionDependency('opentelemetry', '^1.0')] final class ExtensionHookManager implements HookManager { From ba765d99eda1550fdb80394e76f27317759af91f Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 7 May 2024 09:45:17 +1000 Subject: [PATCH 05/43] experimental config file --- src/SDK/Common/Configuration/Defaults.php | 2 ++ src/SDK/Common/Configuration/Variables.php | 2 +- src/SDK/SdkAutoloader.php | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/SDK/Common/Configuration/Defaults.php b/src/SDK/Common/Configuration/Defaults.php index fcfea6e4e..a58d927b5 100644 --- a/src/SDK/Common/Configuration/Defaults.php +++ b/src/SDK/Common/Configuration/Defaults.php @@ -119,4 +119,6 @@ interface Defaults public const OTEL_PHP_DISABLED_INSTRUMENTATIONS = []; public const OTEL_PHP_LOGS_PROCESSOR = 'batch'; public const OTEL_PHP_LOG_DESTINATION = 'default'; + public const OTEL_EXPERIMENTAL_CONFIG_FILE = 'sdk-config.yaml'; + public const OTEL_PHP_INSTRUMENTATION_CONFIG_FILE = 'instrumentation-config.yaml'; } diff --git a/src/SDK/Common/Configuration/Variables.php b/src/SDK/Common/Configuration/Variables.php index 76ae3c8e8..2f57c9e27 100644 --- a/src/SDK/Common/Configuration/Variables.php +++ b/src/SDK/Common/Configuration/Variables.php @@ -140,6 +140,6 @@ interface Variables public const OTEL_PHP_INTERNAL_METRICS_ENABLED = 'OTEL_PHP_INTERNAL_METRICS_ENABLED'; //whether the SDK should emit its own metrics public const OTEL_PHP_DISABLED_INSTRUMENTATIONS = 'OTEL_PHP_DISABLED_INSTRUMENTATIONS'; public const OTEL_PHP_EXCLUDED_URLS = 'OTEL_PHP_EXCLUDED_URLS'; - public const OTEL_PHP_SDK_CONFIG_FILE = 'OTEL_PHP_SDK_CONFIG_FILE'; + public const OTEL_EXPERIMENTAL_CONFIG_FILE = 'OTEL_EXPERIMENTAL_CONFIG_FILE'; public const OTEL_PHP_INSTRUMENTATION_CONFIG_FILE = 'OTEL_PHP_INSTRUMENTATION_CONFIG_FILE'; } diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index 7a391ebdb..89ebc3403 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -37,7 +37,7 @@ public static function autoload(): bool if (!self::isEnabled() || self::isExcludedUrl()) { return false; } - if (Configuration::has(Variables::OTEL_PHP_SDK_CONFIG_FILE)) { + if (Configuration::has(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE)) { Globals::registerInitializer(fn ($configurator) => self::configFileBasedSdkInitializer($configurator)); self::configureInstrumentation(); } else { @@ -86,7 +86,7 @@ public static function environmentBasedInitializer(Configurator $configurator): public static function configFileBasedSdkInitializer(Configurator $configurator): Configurator { - $file = Configuration::getString(Variables::OTEL_PHP_SDK_CONFIG_FILE); + $file = Configuration::getString(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE); $config = SdkConfiguration::parseFile($file); $sdk = $config ->create() From b771c57306c6449460f6c47f50d6ec0bf57c5c95 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 7 May 2024 13:44:57 +1000 Subject: [PATCH 06/43] fixing invalid dependencies - deptrac was rightly complaining that API (indirectly) depended on SDK through config/sdk. For now, remove usage of Config\SDK\Configuration\Context - update deptrac config to allow some new dependencies --- deptrac.yaml | 29 +++++++++++++++++++ .../configure_instrumentation.php | 9 +++--- examples/src/ExampleInstrumentation.php | 5 ++-- .../ExtensionHookManager.php | 2 ++ .../AutoInstrumentation/Instrumentation.php | 3 +- 5 files changed, 39 insertions(+), 9 deletions(-) diff --git a/deptrac.yaml b/deptrac.yaml index eb2f363e7..501f46755 100644 --- a/deptrac.yaml +++ b/deptrac.yaml @@ -25,6 +25,10 @@ deptrac: collectors: - type: directory value: src/SDK/.* + - name: ConfigSDK + collectors: + - type: directory + value: src/Config/SDK/.* - name: Context collectors: - type: directory @@ -85,21 +89,46 @@ deptrac: value: ^GuzzleHttp\\* - type: className value: ^Buzz\\* + - name: SPI + collectors: + - type: className + value: ^Nevay\\SPI\\* + - name: SymfonyConfig + collectors: + - type: className + value: ^Symfony\\Component\\Config\\* + - type: className + value: ^Symfony\\Component\\Yaml\\* + - type: className + value: ^Symfony\\Component\\VarExporter\\* ruleset: Context: - FFI SemConv: ~ + ConfigSDK: + - SymfonyConfig + - API + - SDK + - SPI + - PsrLog + - Composer + - Context + - Contrib + - Extension API: - Context - PsrLog + - SPI SDK: - +API + - ConfigSDK - SemConv - PsrHttp - HttpPlug - Composer - HttpClients + - SPI Contrib: - +SDK - +OtelProto diff --git a/examples/instrumentation/configure_instrumentation.php b/examples/instrumentation/configure_instrumentation.php index 5f434f83a..cc19ad3c0 100644 --- a/examples/instrumentation/configure_instrumentation.php +++ b/examples/instrumentation/configure_instrumentation.php @@ -5,6 +5,7 @@ namespace _; use Nevay\SPI\ServiceLoader; +use OpenTelemetry\API\Globals; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ExtensionHookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation; use OpenTelemetry\Config\SDK\Configuration; @@ -19,17 +20,17 @@ require __DIR__ . '/../../vendor/autoload.php'; -$sdk = Configuration::parseFile(__DIR__ . '/otel-sdk.yaml')->create(new Context())->setAutoShutdown(true)->setHookManager(new ExtensionHookManager())->build(); +Configuration::parseFile(__DIR__ . '/otel-sdk.yaml')->create(new Context())->setAutoShutdown(true)->setHookManager(new ExtensionHookManager())->buildAndRegisterGlobal(); $configuration = \OpenTelemetry\Config\SDK\Instrumentation::parseFile(__DIR__ . '/otel-instrumentation.yaml')->create(); -$context = new Context($sdk->getTracerProvider()); +$hookManager = Globals::hookManager(); $storage = \OpenTelemetry\Context\Context::storage(); foreach (ServiceLoader::load(Instrumentation::class) as $instrumentation) { - $instrumentation->register($sdk->getHookManager(), $context, $configuration, $storage); + $instrumentation->register($hookManager, $configuration, $storage); } -$scope = $storage->attach($sdk->getHookManager()->enable($storage->current())); +$scope = $storage->attach($hookManager->enable($storage->current())); try { echo (new Example())->test(), PHP_EOL; diff --git a/examples/src/ExampleInstrumentation.php b/examples/src/ExampleInstrumentation.php index 971bf568f..386e1ca77 100644 --- a/examples/src/ExampleInstrumentation.php +++ b/examples/src/ExampleInstrumentation.php @@ -10,20 +10,19 @@ use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation; use OpenTelemetry\API\Trace\Span; -use OpenTelemetry\Config\SDK\Configuration\Context; use OpenTelemetry\Context\ContextStorageInterface; final class ExampleInstrumentation implements Instrumentation { - public function register(HookManager $hookManager, ?Context $context, ConfigurationRegistry $configuration, ContextStorageInterface $storage): void + public function register(HookManager $hookManager, ConfigurationRegistry $configuration, ContextStorageInterface $storage): void { $config = $configuration->get(ExampleConfig::class) ?? throw new Exception('example instrumentation must be configured'); if (!$config->enabled) { return; } - $tracer = $context ? $context->tracerProvider->getTracer('example-instrumentation') : Globals::tracerProvider()->getTracer('example-instrumentation'); + $tracer = Globals::tracerProvider()->getTracer('example-instrumentation'); $hookManager->hook( Example::class, diff --git a/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php b/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php index 396f54243..ca657f5c6 100644 --- a/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php +++ b/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php @@ -12,6 +12,8 @@ use OpenTelemetry\Context\ContextKeyInterface; use ReflectionFunction; +/** @phan-file-suppress PhanUndeclaredClassAttribute */ + #[ExtensionDependency('opentelemetry', '^1.0')] final class ExtensionHookManager implements HookManager { diff --git a/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php b/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php index 4c0d3d4f5..bcee688f8 100644 --- a/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php +++ b/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php @@ -4,7 +4,6 @@ namespace OpenTelemetry\API\Instrumentation\AutoInstrumentation; -use OpenTelemetry\Config\SDK\Configuration\Context; use OpenTelemetry\Context\ContextStorageInterface; interface Instrumentation @@ -13,5 +12,5 @@ interface Instrumentation * @todo context is nullable in order to support autoloading (and retrieving lazy-loaded tracers), but auto and non-auto * should work the same. */ - public function register(HookManager $hookManager, ?Context $context, ConfigurationRegistry $configuration, ContextStorageInterface $storage): void; + public function register(HookManager $hookManager, ConfigurationRegistry $configuration, ContextStorageInterface $storage): void; } From ba994d5133e0c2ec72bb5ab3e034ee3ba63774cb Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 7 May 2024 15:08:53 +1000 Subject: [PATCH 07/43] dont register hook manager globally or in sdk --- .../instrumentation/configure_instrumentation.php | 5 ++--- .../configure_instrumentation_global.php | 2 +- src/API/Globals.php | 12 ++---------- .../AutoInstrumentation/Instrumentation.php | 4 ---- src/API/Instrumentation/Configurator.php | 15 --------------- src/API/Instrumentation/ContextKeys.php | 8 -------- src/SDK/Sdk.php | 7 ------- src/SDK/SdkAutoloader.php | 6 +----- src/SDK/SdkBuilder.php | 13 ------------- tests/Unit/SDK/SdkTest.php | 5 +---- 10 files changed, 7 insertions(+), 70 deletions(-) diff --git a/examples/instrumentation/configure_instrumentation.php b/examples/instrumentation/configure_instrumentation.php index cc19ad3c0..238622580 100644 --- a/examples/instrumentation/configure_instrumentation.php +++ b/examples/instrumentation/configure_instrumentation.php @@ -5,7 +5,6 @@ namespace _; use Nevay\SPI\ServiceLoader; -use OpenTelemetry\API\Globals; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ExtensionHookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation; use OpenTelemetry\Config\SDK\Configuration; @@ -20,10 +19,10 @@ require __DIR__ . '/../../vendor/autoload.php'; -Configuration::parseFile(__DIR__ . '/otel-sdk.yaml')->create(new Context())->setAutoShutdown(true)->setHookManager(new ExtensionHookManager())->buildAndRegisterGlobal(); +Configuration::parseFile(__DIR__ . '/otel-sdk.yaml')->create(new Context())->setAutoShutdown(true)->buildAndRegisterGlobal(); $configuration = \OpenTelemetry\Config\SDK\Instrumentation::parseFile(__DIR__ . '/otel-instrumentation.yaml')->create(); -$hookManager = Globals::hookManager(); +$hookManager = new ExtensionHookManager(); $storage = \OpenTelemetry\Context\Context::storage(); foreach (ServiceLoader::load(Instrumentation::class) as $instrumentation) { diff --git a/examples/instrumentation/configure_instrumentation_global.php b/examples/instrumentation/configure_instrumentation_global.php index 496d25b6e..c1a446789 100644 --- a/examples/instrumentation/configure_instrumentation_global.php +++ b/examples/instrumentation/configure_instrumentation_global.php @@ -12,7 +12,7 @@ */ // EXAMPLE_INSTRUMENTATION_SPAN_NAME=test1234 php examples/instrumentation/configure_instrumentation.php putenv('OTEL_PHP_AUTOLOAD_ENABLED=true'); -putenv(sprintf('OTEL_PHP_SDK_CONFIG_FILE=%s/%s', __DIR__, 'otel-sdk.yaml')); +putenv(sprintf('OTEL_EXPERIMENTAL_CONFIG_FILE=%s/%s', __DIR__, 'otel-sdk.yaml')); putenv(sprintf('OTEL_PHP_INSTRUMENTATION_CONFIG_FILE=%s/%s', __DIR__, 'otel-instrumentation.yaml')); require __DIR__ . '/../../vendor/autoload.php'; diff --git a/src/API/Globals.php b/src/API/Globals.php index 489acb2fb..c5cfa8ce0 100644 --- a/src/API/Globals.php +++ b/src/API/Globals.php @@ -7,7 +7,6 @@ use function assert; use Closure; use const E_USER_WARNING; -use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\API\Instrumentation\ContextKeys; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; @@ -35,7 +34,6 @@ public function __construct( private readonly LoggerProviderInterface $loggerProvider, private readonly EventLoggerProviderInterface $eventLoggerProvider, private readonly TextMapPropagatorInterface $propagator, - private readonly HookManager $hookManager, ) { } @@ -64,11 +62,6 @@ public static function eventLoggerProvider(): EventLoggerProviderInterface return Context::getCurrent()->get(ContextKeys::eventLoggerProvider()) ?? self::globals()->eventLoggerProvider; } - public static function hookManager(): HookManager - { - return Context::getCurrent()->get(ContextKeys::hookManager()) ?? self::globals()->hookManager; - } - /** * @param Closure(Configurator): Configurator $initializer * @@ -110,11 +103,10 @@ private static function globals(): self $propagator = $context->get(ContextKeys::propagator()); $loggerProvider = $context->get(ContextKeys::loggerProvider()); $eventLoggerProvider = $context->get(ContextKeys::eventLoggerProvider()); - $hookManager = $context->get(ContextKeys::hookManager()); - assert(isset($tracerProvider, $meterProvider, $loggerProvider, $eventLoggerProvider, $propagator, $hookManager)); + assert(isset($tracerProvider, $meterProvider, $loggerProvider, $eventLoggerProvider, $propagator)); - return self::$globals = new self($tracerProvider, $meterProvider, $loggerProvider, $eventLoggerProvider, $propagator, $hookManager); + return self::$globals = new self($tracerProvider, $meterProvider, $loggerProvider, $eventLoggerProvider, $propagator); } public static function init(): void diff --git a/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php b/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php index bcee688f8..8eb684b6a 100644 --- a/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php +++ b/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php @@ -8,9 +8,5 @@ interface Instrumentation { - /** - * @todo context is nullable in order to support autoloading (and retrieving lazy-loaded tracers), but auto and non-auto - * should work the same. - */ public function register(HookManager $hookManager, ConfigurationRegistry $configuration, ContextStorageInterface $storage): void; } diff --git a/src/API/Instrumentation/Configurator.php b/src/API/Instrumentation/Configurator.php index 63e2e2033..d8912ec2d 100644 --- a/src/API/Instrumentation/Configurator.php +++ b/src/API/Instrumentation/Configurator.php @@ -4,8 +4,6 @@ namespace OpenTelemetry\API\Instrumentation; -use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; -use OpenTelemetry\API\Instrumentation\AutoInstrumentation\NoopHookManager; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; use OpenTelemetry\API\Logs\LoggerProviderInterface; use OpenTelemetry\API\Logs\NoopEventLoggerProvider; @@ -33,7 +31,6 @@ final class Configurator implements ImplicitContextKeyedInterface private ?TextMapPropagatorInterface $propagator = null; private ?LoggerProviderInterface $loggerProvider = null; private ?EventLoggerProviderInterface $eventLoggerProvider = null; - private ?HookManager $hookManager = null; private function __construct() { @@ -58,7 +55,6 @@ public static function createNoop(): Configurator ->withPropagator(new NoopTextMapPropagator()) ->withLoggerProvider(NoopLoggerProvider::getInstance()) ->withEventLoggerProvider(new NoopEventLoggerProvider()) - ->withHookManager(new NoopHookManager()) ; } @@ -86,9 +82,6 @@ public function storeInContext(?ContextInterface $context = null): ContextInterf if ($this->eventLoggerProvider !== null) { $context = $context->with(ContextKeys::eventLoggerProvider(), $this->eventLoggerProvider); } - if ($this->hookManager !== null) { - $context = $context->with(ContextKeys::hookManager(), $this->hookManager); - } return $context; } @@ -132,12 +125,4 @@ public function withEventLoggerProvider(?EventLoggerProviderInterface $eventLogg return $self; } - - public function withHookManager(?HookManager $hookManager): Configurator - { - $self = clone $this; - $self->hookManager = $hookManager; - - return $self; - } } diff --git a/src/API/Instrumentation/ContextKeys.php b/src/API/Instrumentation/ContextKeys.php index 5840f055b..9f53e5b86 100644 --- a/src/API/Instrumentation/ContextKeys.php +++ b/src/API/Instrumentation/ContextKeys.php @@ -4,7 +4,6 @@ namespace OpenTelemetry\API\Instrumentation; -use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; use OpenTelemetry\API\Logs\LoggerProviderInterface; use OpenTelemetry\API\Metrics\MeterProviderInterface; @@ -67,11 +66,4 @@ public static function eventLoggerProvider(): ContextKeyInterface return $instance ??= Context::createKey(EventLoggerProviderInterface::class); } - - public static function hookManager(): ContextKeyInterface - { - static $instance; - - return $instance ??= Context::createKey(HookManager::class); - } } diff --git a/src/SDK/Sdk.php b/src/SDK/Sdk.php index 03af0324a..78f541f64 100644 --- a/src/SDK/Sdk.php +++ b/src/SDK/Sdk.php @@ -4,7 +4,6 @@ namespace OpenTelemetry\SDK; -use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; use OpenTelemetry\API\Metrics\MeterProviderInterface; use OpenTelemetry\API\Trace\TracerProviderInterface; @@ -23,7 +22,6 @@ public function __construct( private readonly LoggerProviderInterface $loggerProvider, private readonly EventLoggerProviderInterface $eventLoggerProvider, private readonly TextMapPropagatorInterface $propagator, - private readonly HookManager $hookManager, ) { } @@ -71,9 +69,4 @@ public function getPropagator(): TextMapPropagatorInterface { return $this->propagator; } - - public function getHookManager(): HookManager - { - return $this->hookManager; - } } diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index 89ebc3403..d37de6f6b 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -7,7 +7,6 @@ use InvalidArgumentException; use Nevay\SPI\ServiceLoader; use OpenTelemetry\API\Globals; -use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ExtensionHookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\NoopHookManager; use OpenTelemetry\API\Instrumentation\Configurator; @@ -54,7 +53,6 @@ public static function environmentBasedInitializer(Configurator $configurator): //@see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-environment-variables.md#general-sdk-configuration return $configurator->withPropagator($propagator); } - $hookManager = new ExtensionHookManager(); //to should be enabled/disable by env? $emitMetrics = Configuration::getBoolean(Variables::OTEL_PHP_INTERNAL_METRICS_ENABLED); $resource = ResourceInfoFactory::defaultResource(); @@ -80,7 +78,6 @@ public static function environmentBasedInitializer(Configurator $configurator): ->withLoggerProvider($loggerProvider) ->withEventLoggerProvider($eventLoggerProvider) ->withPropagator($propagator) - ->withHookManager($hookManager) ; } @@ -99,7 +96,6 @@ public static function configFileBasedSdkInitializer(Configurator $configurator) ->withLoggerProvider($sdk->getLoggerProvider()) ->withEventLoggerProvider($sdk->getEventLoggerProvider()) ->withPropagator($sdk->getPropagator()) - ->withHookManager($sdk->getHookManager()) ; } @@ -116,7 +112,7 @@ private static function configureInstrumentation(): void $storage = Context::storage(); $hookManager = self::getHookManager(); foreach (ServiceLoader::load(\OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation::class) as $instrumentation) { - $instrumentation->register($hookManager, null, $configuration, $storage); + $instrumentation->register($hookManager, $configuration, $storage); } } diff --git a/src/SDK/SdkBuilder.php b/src/SDK/SdkBuilder.php index 2a6f8c21b..403713e0b 100644 --- a/src/SDK/SdkBuilder.php +++ b/src/SDK/SdkBuilder.php @@ -4,8 +4,6 @@ namespace OpenTelemetry\SDK; -use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; -use OpenTelemetry\API\Instrumentation\AutoInstrumentation\NoopHookManager; use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; use OpenTelemetry\API\Logs\NoopEventLoggerProvider; @@ -28,7 +26,6 @@ class SdkBuilder private ?LoggerProviderInterface $loggerProvider = null; private ?EventLoggerProviderInterface $eventLoggerProvider = null; private ?TextMapPropagatorInterface $propagator = null; - private ?HookManager $hookManager = null; private bool $autoShutdown = false; /** @@ -76,20 +73,12 @@ public function setPropagator(TextMapPropagatorInterface $propagator): self return $this; } - public function setHookManager(HookManager $hookManager): self - { - $this->hookManager = $hookManager; - - return $this; - } - public function build(): Sdk { $tracerProvider = $this->tracerProvider ?? new NoopTracerProvider(); $meterProvider = $this->meterProvider ?? new NoopMeterProvider(); $loggerProvider = $this->loggerProvider ?? new NoopLoggerProvider(); $eventLoggerProvider = $this->eventLoggerProvider ?? new NoopEventLoggerProvider(); - $hookManager = $this->hookManager ?? new NoopHookManager(); if ($this->autoShutdown) { // rector rule disabled in config, because ShutdownHandler::register() does not keep a strong reference to $this ShutdownHandler::register($tracerProvider->shutdown(...)); @@ -103,7 +92,6 @@ public function build(): Sdk $loggerProvider, $eventLoggerProvider, $this->propagator ?? NoopTextMapPropagator::getInstance(), - $hookManager, ); } @@ -116,7 +104,6 @@ public function buildAndRegisterGlobal(): ScopeInterface ->withMeterProvider($sdk->getMeterProvider()) ->withLoggerProvider($sdk->getLoggerProvider()) ->withEventLoggerProvider($sdk->getEventLoggerProvider()) - ->withHookManager($sdk->getHookManager()) ->storeInContext(); return Context::storage()->attach($context); diff --git a/tests/Unit/SDK/SdkTest.php b/tests/Unit/SDK/SdkTest.php index 8ca62df98..be35ab3bb 100644 --- a/tests/Unit/SDK/SdkTest.php +++ b/tests/Unit/SDK/SdkTest.php @@ -5,7 +5,6 @@ namespace OpenTelemetry\Tests\Unit\SDK; use AssertWell\PHPUnitGlobalState\EnvironmentVariables; -use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface; use OpenTelemetry\SDK\Common\Configuration\Variables; @@ -85,13 +84,11 @@ public function test_getters(): void $tracerProvider = $this->createMock(TracerProviderInterface::class); $loggerProvider = $this->createMock(LoggerProviderInterface::class); $eventLoggerProvider = $this->createMock(EventLoggerProviderInterface::class); - $hookManager = $this->createMock(HookManager::class); - $sdk = new Sdk($tracerProvider, $meterProvider, $loggerProvider, $eventLoggerProvider, $propagator, $hookManager); + $sdk = new Sdk($tracerProvider, $meterProvider, $loggerProvider, $eventLoggerProvider, $propagator); $this->assertSame($propagator, $sdk->getPropagator()); $this->assertSame($meterProvider, $sdk->getMeterProvider()); $this->assertSame($tracerProvider, $sdk->getTracerProvider()); $this->assertSame($loggerProvider, $sdk->getLoggerProvider()); $this->assertSame($eventLoggerProvider, $sdk->getEventLoggerProvider()); - $this->assertSame($hookManager, $sdk->getHookManager()); } } From 35feeff9b20ec1186b17d86678b38280fb422cdb Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 7 May 2024 15:18:49 +1000 Subject: [PATCH 08/43] remove unused function, psalm ignore missing extension function --- psalm.xml.dist | 5 +++++ src/API/Globals.php | 5 ----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/psalm.xml.dist b/psalm.xml.dist index 0fe505df9..ed0d7ffc9 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -27,6 +27,11 @@ + + + + + diff --git a/src/API/Globals.php b/src/API/Globals.php index c5cfa8ce0..7d296f4fc 100644 --- a/src/API/Globals.php +++ b/src/API/Globals.php @@ -109,11 +109,6 @@ private static function globals(): self return self::$globals = new self($tracerProvider, $meterProvider, $loggerProvider, $eventLoggerProvider, $propagator); } - public static function init(): void - { - self::globals(); - } - /** * @internal */ From 43fc77265f13c71ccb0f89aa64ea409f35bc34fb Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 7 May 2024 15:36:50 +1000 Subject: [PATCH 09/43] possibly fixing type-hint psalm doesn't complain now, so that should be good --- src/Config/SDK/Instrumentation.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Config/SDK/Instrumentation.php b/src/Config/SDK/Instrumentation.php index 8f4238c54..21fde066e 100644 --- a/src/Config/SDK/Instrumentation.php +++ b/src/Config/SDK/Instrumentation.php @@ -19,7 +19,7 @@ final class Instrumentation { /** - * @param ComponentPlugin $plugin + * @param ComponentPlugin $plugin */ private function __construct( private readonly ComponentPlugin $plugin, From 76ce98f37aab5432e20466749a4631a21ccff34d Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Wed, 8 May 2024 09:33:05 +1000 Subject: [PATCH 10/43] load config files relative to cwd --- .../instrumentation/configure_instrumentation_global.php | 9 +++++---- src/Config/SDK/Configuration/ConfigurationFactory.php | 3 ++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/examples/instrumentation/configure_instrumentation_global.php b/examples/instrumentation/configure_instrumentation_global.php index c1a446789..de82f675b 100644 --- a/examples/instrumentation/configure_instrumentation_global.php +++ b/examples/instrumentation/configure_instrumentation_global.php @@ -8,12 +8,13 @@ use const PHP_EOL; /** - * This example uses SPI (see root composer.json extra.spi) to configure an example auto-instrumentation from a YAML file + * This example uses SPI (see root composer.json extra.spi) to configure an example auto-instrumentation from a YAML file. + * The YAML file paths are relative to the current working directory. */ -// EXAMPLE_INSTRUMENTATION_SPAN_NAME=test1234 php examples/instrumentation/configure_instrumentation.php +// EXAMPLE_INSTRUMENTATION_SPAN_NAME=test1234 php examples/instrumentation/configure_instrumentation_global.php putenv('OTEL_PHP_AUTOLOAD_ENABLED=true'); -putenv(sprintf('OTEL_EXPERIMENTAL_CONFIG_FILE=%s/%s', __DIR__, 'otel-sdk.yaml')); -putenv(sprintf('OTEL_PHP_INSTRUMENTATION_CONFIG_FILE=%s/%s', __DIR__, 'otel-instrumentation.yaml')); +putenv('OTEL_EXPERIMENTAL_CONFIG_FILE=examples/instrumentation/otel-sdk.yaml'); +putenv('OTEL_PHP_INSTRUMENTATION_CONFIG_FILE=examples/instrumentation/otel-instrumentation.yaml'); require __DIR__ . '/../../vendor/autoload.php'; diff --git a/src/Config/SDK/Configuration/ConfigurationFactory.php b/src/Config/SDK/Configuration/ConfigurationFactory.php index cb4987a46..c9eb8b9dc 100644 --- a/src/Config/SDK/Configuration/ConfigurationFactory.php +++ b/src/Config/SDK/Configuration/ConfigurationFactory.php @@ -6,6 +6,7 @@ use function class_exists; use Exception; +use function getcwd; use function is_file; use OpenTelemetry\Config\SDK\Configuration\Environment\EnvReader; use OpenTelemetry\Config\SDK\Configuration\Environment\EnvResourceChecker; @@ -98,7 +99,7 @@ public function parseFile( } $loader = new ConfigurationLoader($resources); - $locator = new FileLocator(); + $locator = new FileLocator(getcwd()); $fileLoader = new DelegatingLoader(new LoaderResolver([ new YamlSymfonyFileLoader($loader, $locator), new YamlExtensionFileLoader($loader, $locator), From 73f83e066d3481b34ee2105e0efbef2f83aad6bc Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Wed, 8 May 2024 10:42:25 +1000 Subject: [PATCH 11/43] fixing hook manager enable/disable + psalm complaints --- .../ExtensionHookManager.php | 13 ++--- .../AutoInstrumentation/HookManager.php | 6 +-- .../AutoInstrumentation/NoopHookManager.php | 6 +-- .../Instrumentation/ConfigurationRegistry.php | 50 ------------------- ... InstrumentationConfigurationRegistry.php} | 5 +- src/Config/SDK/Instrumentation.php | 6 +-- src/SDK/SdkAutoloader.php | 6 +++ 7 files changed, 24 insertions(+), 68 deletions(-) delete mode 100644 src/Config/SDK/ComponentProvider/Instrumentation/ConfigurationRegistry.php rename src/Config/SDK/ComponentProvider/{OpenTelemetryInstrumentation.php => InstrumentationConfigurationRegistry.php} (90%) diff --git a/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php b/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php index ca657f5c6..d779c8de1 100644 --- a/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php +++ b/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php @@ -9,6 +9,7 @@ use function extension_loaded; use Nevay\SPI\ServiceProviderDependency\ExtensionDependency; use OpenTelemetry\Context\Context; +use OpenTelemetry\Context\ContextInterface; use OpenTelemetry\Context\ContextKeyInterface; use ReflectionFunction; @@ -36,12 +37,12 @@ public function hook(?string $class, string $function, ?Closure $preHook = null, \OpenTelemetry\Instrumentation\hook($class, $function, $this->bindHookScope($preHook), $this->bindHookScope($postHook)); } - public function enable(Context $context): Context + public function enable(ContextInterface $context): ContextInterface { return $context->with($this->contextKey, true); } - public function disable(Context $context): Context + public function disable(ContextInterface $context): ContextInterface { return $context->with($this->contextKey, null); } @@ -58,18 +59,18 @@ private function bindHookScope(?Closure $closure): ?Closure // TODO Add an option flag to ext-opentelemetry `hook` that configures whether return values should be used? if (!$reflection->getReturnType() || (string) $reflection->getReturnType() === 'void') { return static function (mixed ...$args) use ($closure, $contextKey): void { - /*if (!Context::getCurrent()->get($contextKey)) { + if (!Context::getCurrent()->get($contextKey)) { return; - }*/ + } $closure(...$args); }; } return static function (mixed ...$args) use ($closure, $contextKey): mixed { - /*if (!Context::getCurrent()->get($contextKey)) { + if (!Context::getCurrent()->get($contextKey)) { return null; - }*/ + } return $closure(...$args); }; diff --git a/src/API/Instrumentation/AutoInstrumentation/HookManager.php b/src/API/Instrumentation/AutoInstrumentation/HookManager.php index db57dbadf..4ba8f38a3 100644 --- a/src/API/Instrumentation/AutoInstrumentation/HookManager.php +++ b/src/API/Instrumentation/AutoInstrumentation/HookManager.php @@ -5,7 +5,7 @@ namespace OpenTelemetry\API\Instrumentation\AutoInstrumentation; use Closure; -use OpenTelemetry\Context\Context; +use OpenTelemetry\Context\ContextInterface; use Throwable; interface HookManager @@ -17,7 +17,7 @@ interface HookManager */ public function hook(?string $class, string $function, ?Closure $preHook = null, ?Closure $postHook = null): void; - public function enable(Context $context): Context; + public function enable(ContextInterface $context): ContextInterface; - public function disable(Context $context): Context; + public function disable(ContextInterface $context): ContextInterface; } diff --git a/src/API/Instrumentation/AutoInstrumentation/NoopHookManager.php b/src/API/Instrumentation/AutoInstrumentation/NoopHookManager.php index 7f1ccde73..5554c3a1d 100644 --- a/src/API/Instrumentation/AutoInstrumentation/NoopHookManager.php +++ b/src/API/Instrumentation/AutoInstrumentation/NoopHookManager.php @@ -5,7 +5,7 @@ namespace OpenTelemetry\API\Instrumentation\AutoInstrumentation; use Closure; -use OpenTelemetry\Context\Context; +use OpenTelemetry\Context\ContextInterface; final class NoopHookManager implements HookManager { @@ -15,12 +15,12 @@ public function hook(?string $class, string $function, ?Closure $preHook = null, // no-op } - public function enable(Context $context): Context + public function enable(ContextInterface $context): ContextInterface { return $context; } - public function disable(Context $context): Context + public function disable(ContextInterface $context): ContextInterface { return $context; } diff --git a/src/Config/SDK/ComponentProvider/Instrumentation/ConfigurationRegistry.php b/src/Config/SDK/ComponentProvider/Instrumentation/ConfigurationRegistry.php deleted file mode 100644 index e58f2ec58..000000000 --- a/src/Config/SDK/ComponentProvider/Instrumentation/ConfigurationRegistry.php +++ /dev/null @@ -1,50 +0,0 @@ - - */ -class ConfigurationRegistry implements ComponentProvider -{ - /** - * @param array{ - * config: list>, - * } $properties - */ - public function createPlugin(array $properties, Context $context): ConfigurationRegistryComponent - { - $configurationRegistry = new ConfigurationRegistryComponent(); - foreach ($properties['config'] as $configuration) { - $configurationRegistry->add($configuration->create($context)); - } - - return $configurationRegistry; - } - - public function getConfig(ComponentProviderRegistry $registry): ArrayNodeDefinition - { - $root = new ArrayNodeDefinition('instrumentation'); - $root - ->children() - // TODO add disabled_instrumentations arrayNode to allow disabling specific instrumentation classes? - ->append($registry->componentList('config', InstrumentationConfiguration::class)) - ->end() - ; - - return $root; - } -} diff --git a/src/Config/SDK/ComponentProvider/OpenTelemetryInstrumentation.php b/src/Config/SDK/ComponentProvider/InstrumentationConfigurationRegistry.php similarity index 90% rename from src/Config/SDK/ComponentProvider/OpenTelemetryInstrumentation.php rename to src/Config/SDK/ComponentProvider/InstrumentationConfigurationRegistry.php index 4eb413ddf..45d1369d5 100644 --- a/src/Config/SDK/ComponentProvider/OpenTelemetryInstrumentation.php +++ b/src/Config/SDK/ComponentProvider/InstrumentationConfigurationRegistry.php @@ -10,15 +10,14 @@ use OpenTelemetry\Config\SDK\Configuration\ComponentProvider; use OpenTelemetry\Config\SDK\Configuration\ComponentProviderRegistry; use OpenTelemetry\Config\SDK\Configuration\Context; -use OpenTelemetry\Config\SDK\Instrumentation; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; /** * @internal * - * @implements ComponentProvider + * @implements ComponentProvider */ -class OpenTelemetryInstrumentation implements ComponentProvider +class InstrumentationConfigurationRegistry implements ComponentProvider { /** * @param array{ diff --git a/src/Config/SDK/Instrumentation.php b/src/Config/SDK/Instrumentation.php index 21fde066e..aa974d8bf 100644 --- a/src/Config/SDK/Instrumentation.php +++ b/src/Config/SDK/Instrumentation.php @@ -6,7 +6,7 @@ use Nevay\SPI\ServiceLoader; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ConfigurationRegistry; -use OpenTelemetry\Config\SDK\ComponentProvider\OpenTelemetryInstrumentation; +use OpenTelemetry\Config\SDK\ComponentProvider\InstrumentationConfigurationRegistry; use OpenTelemetry\Config\SDK\Configuration\ComponentPlugin; use OpenTelemetry\Config\SDK\Configuration\ComponentProvider; use OpenTelemetry\Config\SDK\Configuration\ConfigurationFactory; @@ -45,7 +45,7 @@ public static function parseFile( } /** - * @return ConfigurationFactory + * @return ConfigurationFactory */ private static function factory(): ConfigurationFactory { @@ -53,7 +53,7 @@ private static function factory(): ConfigurationFactory return $factory ??= new ConfigurationFactory( ServiceLoader::load(ComponentProvider::class), - new OpenTelemetryInstrumentation(), + new InstrumentationConfigurationRegistry(), new EnvSourceReader([ new ServerEnvSource(), new PhpIniEnvSource(), diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index d37de6f6b..759309b36 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -121,7 +121,13 @@ private static function configureInstrumentation(): void */ private static function getHookManager(): HookManager { + /** @var HookManager $hookManager */ foreach (ServiceLoader::load(HookManager::class) as $hookManager) { + $scope = $hookManager->enable(Context::getCurrent())->activate(); + ShutdownHandler::register(function () use ($scope) { + $scope->detach(); + }); + return $hookManager; } From 7e57997a46fe1fbc5b7c0512b936cb9f10ccef2b Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Wed, 8 May 2024 10:48:15 +1000 Subject: [PATCH 12/43] fixing 8.1 psalm error --- examples/src/ExampleConfigProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/src/ExampleConfigProvider.php b/examples/src/ExampleConfigProvider.php index c3ba0c1cf..eba88950a 100644 --- a/examples/src/ExampleConfigProvider.php +++ b/examples/src/ExampleConfigProvider.php @@ -33,7 +33,7 @@ public function createPlugin(array $properties, Context $context): Instrumentati } /** - * @psalm-suppress UndefinedInterfaceMethod + * @psalm-suppress UndefinedInterfaceMethod,PossiblyNullReference */ public function getConfig(ComponentProviderRegistry $registry): ArrayNodeDefinition { From f29b71318b40e538d73e23ee665e3751a5ef4605 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Wed, 8 May 2024 15:11:26 +1000 Subject: [PATCH 13/43] use context to pass providers to instrumentations - make "register global" a function of Sdk, but keep the sdk builder's interface intact - invent an API instrumentation context, similar to the one in config/sdk, to pass providers to instrumentations - add an initial test of autoloading from a config file --- .../configure_instrumentation.php | 5 +- examples/src/ExampleInstrumentation.php | 6 +-- src/API/Globals.php | 7 +++ .../AutoInstrumentation/Context.php | 25 ++++++++++ .../AutoInstrumentation/Instrumentation.php | 2 +- src/SDK/Sdk.php | 19 ++++++++ src/SDK/SdkAutoloader.php | 46 ++++--------------- src/SDK/SdkBuilder.php | 13 +----- tests/Unit/SDK/SdkAutoloaderTest.php | 10 ++++ tests/Unit/SDK/fixtures/otel-sdk.yaml | 7 +++ 10 files changed, 86 insertions(+), 54 deletions(-) create mode 100644 src/API/Instrumentation/AutoInstrumentation/Context.php create mode 100644 tests/Unit/SDK/fixtures/otel-sdk.yaml diff --git a/examples/instrumentation/configure_instrumentation.php b/examples/instrumentation/configure_instrumentation.php index 238622580..d1aeeaa53 100644 --- a/examples/instrumentation/configure_instrumentation.php +++ b/examples/instrumentation/configure_instrumentation.php @@ -19,14 +19,15 @@ require __DIR__ . '/../../vendor/autoload.php'; -Configuration::parseFile(__DIR__ . '/otel-sdk.yaml')->create(new Context())->setAutoShutdown(true)->buildAndRegisterGlobal(); +$sdk = Configuration::parseFile(__DIR__ . '/otel-sdk.yaml')->create(new Context())->setAutoShutdown(true)->build(); $configuration = \OpenTelemetry\Config\SDK\Instrumentation::parseFile(__DIR__ . '/otel-instrumentation.yaml')->create(); $hookManager = new ExtensionHookManager(); $storage = \OpenTelemetry\Context\Context::storage(); +$context = new Context($sdk->getTracerProvider(), $sdk->getMeterProvider()); foreach (ServiceLoader::load(Instrumentation::class) as $instrumentation) { - $instrumentation->register($hookManager, $configuration, $storage); + $instrumentation->register($hookManager, $context, $configuration, $storage); } $scope = $storage->attach($hookManager->enable($storage->current())); diff --git a/examples/src/ExampleInstrumentation.php b/examples/src/ExampleInstrumentation.php index 386e1ca77..ef33c0e71 100644 --- a/examples/src/ExampleInstrumentation.php +++ b/examples/src/ExampleInstrumentation.php @@ -5,8 +5,8 @@ namespace OpenTelemetry\Example; use Exception; -use OpenTelemetry\API\Globals; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ConfigurationRegistry; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Context; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation; use OpenTelemetry\API\Trace\Span; @@ -15,14 +15,14 @@ final class ExampleInstrumentation implements Instrumentation { - public function register(HookManager $hookManager, ConfigurationRegistry $configuration, ContextStorageInterface $storage): void + public function register(HookManager $hookManager, Context $context, ConfigurationRegistry $configuration, ContextStorageInterface $storage): void { $config = $configuration->get(ExampleConfig::class) ?? throw new Exception('example instrumentation must be configured'); if (!$config->enabled) { return; } - $tracer = Globals::tracerProvider()->getTracer('example-instrumentation'); + $tracer = $context->tracerProvider->getTracer('example-instrumentation'); $hookManager->hook( Example::class, diff --git a/src/API/Globals.php b/src/API/Globals.php index 7d296f4fc..0761fbadf 100644 --- a/src/API/Globals.php +++ b/src/API/Globals.php @@ -73,6 +73,13 @@ public static function registerInitializer(Closure $initializer): void self::$initializers[] = $initializer; } + public static function init(): void + { + if (self::$globals === null) { + self::globals(); + } + } + /** * @phan-suppress PhanTypeMismatchReturnNullable */ diff --git a/src/API/Instrumentation/AutoInstrumentation/Context.php b/src/API/Instrumentation/AutoInstrumentation/Context.php new file mode 100644 index 000000000..70005ccab --- /dev/null +++ b/src/API/Instrumentation/AutoInstrumentation/Context.php @@ -0,0 +1,25 @@ +propagator; } + + /** + * Globally register the SDK so that its components are available via `Globals` + */ + public function registerGlobal(): ScopeInterface + { + $context = Configurator::create() + ->withPropagator($this->getPropagator()) + ->withTracerProvider($this->getTracerProvider()) + ->withMeterProvider($this->getMeterProvider()) + ->withLoggerProvider($this->getLoggerProvider()) + ->withEventLoggerProvider($this->getEventLoggerProvider()) + ->storeInContext(); + + return Context::storage()->attach($context); + } } diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index 759309b36..fbc828d8c 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -7,6 +7,7 @@ use InvalidArgumentException; use Nevay\SPI\ServiceLoader; use OpenTelemetry\API\Globals; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Context as InstrumentationContext; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\NoopHookManager; use OpenTelemetry\API\Instrumentation\Configurator; @@ -37,8 +38,8 @@ public static function autoload(): bool return false; } if (Configuration::has(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE)) { - Globals::registerInitializer(fn ($configurator) => self::configFileBasedSdkInitializer($configurator)); - self::configureInstrumentation(); + $sdk = self::createAndRegisterFileBasedSdk(); + self::configureInstrumentation($sdk); } else { Globals::registerInitializer(fn ($configurator) => self::environmentBasedInitializer($configurator)); } @@ -81,7 +82,7 @@ public static function environmentBasedInitializer(Configurator $configurator): ; } - public static function configFileBasedSdkInitializer(Configurator $configurator): Configurator + public static function createAndRegisterFileBasedSdk(): Sdk { $file = Configuration::getString(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE); $config = SdkConfiguration::parseFile($file); @@ -89,20 +90,15 @@ public static function configFileBasedSdkInitializer(Configurator $configurator) ->create() ->setAutoShutdown(true) ->build(); + $sdk->registerGlobal(); - return $configurator - ->withTracerProvider($sdk->getTracerProvider()) - ->withMeterProvider($sdk->getMeterProvider()) - ->withLoggerProvider($sdk->getLoggerProvider()) - ->withEventLoggerProvider($sdk->getEventLoggerProvider()) - ->withPropagator($sdk->getPropagator()) - ; + return $sdk; } /** * @phan-suppress PhanUndeclaredClassMethod */ - private static function configureInstrumentation(): void + private static function configureInstrumentation(Sdk $sdk): void { if (!Configuration::has(Variables::OTEL_PHP_INSTRUMENTATION_CONFIG_FILE)) { return; @@ -111,8 +107,10 @@ private static function configureInstrumentation(): void $configuration = Instrumentation::parseFile($file)->create(); $storage = Context::storage(); $hookManager = self::getHookManager(); + $context = new InstrumentationContext($sdk->getTracerProvider(), $sdk->getMeterProvider(), $sdk->getLoggerProvider()); foreach (ServiceLoader::load(\OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation::class) as $instrumentation) { - $instrumentation->register($hookManager, $configuration, $storage); + /** @var \OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation $instrumentation */ + $instrumentation->register($hookManager, $context, $configuration, $storage); } } @@ -134,30 +132,6 @@ private static function getHookManager(): HookManager return new NoopHookManager(); } - /** - * Test whether a request URI is set, and if it matches the excluded urls configuration option - * - * @internal - */ - public static function isIgnoredUrl(): bool - { - $ignoreUrls = Configuration::getList(Variables::OTEL_PHP_EXCLUDED_URLS, []); - if ($ignoreUrls === []) { - return false; - } - $url = $_SERVER['REQUEST_URI'] ?? null; - if (!$url) { - return false; - } - foreach ($ignoreUrls as $ignore) { - if (preg_match(sprintf('|%s|', $ignore), (string) $url) === 1) { - return true; - } - } - - return false; - } - /** * @internal */ diff --git a/src/SDK/SdkBuilder.php b/src/SDK/SdkBuilder.php index 403713e0b..154ff5063 100644 --- a/src/SDK/SdkBuilder.php +++ b/src/SDK/SdkBuilder.php @@ -4,10 +4,8 @@ namespace OpenTelemetry\SDK; -use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; use OpenTelemetry\API\Logs\NoopEventLoggerProvider; -use OpenTelemetry\Context\Context; use OpenTelemetry\Context\Propagation\NoopTextMapPropagator; use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface; use OpenTelemetry\Context\ScopeInterface; @@ -97,15 +95,6 @@ public function build(): Sdk public function buildAndRegisterGlobal(): ScopeInterface { - $sdk = $this->build(); - $context = Configurator::create() - ->withPropagator($sdk->getPropagator()) - ->withTracerProvider($sdk->getTracerProvider()) - ->withMeterProvider($sdk->getMeterProvider()) - ->withLoggerProvider($sdk->getLoggerProvider()) - ->withEventLoggerProvider($sdk->getEventLoggerProvider()) - ->storeInContext(); - - return Context::storage()->attach($context); + return $this->build()->registerGlobal(); } } diff --git a/tests/Unit/SDK/SdkAutoloaderTest.php b/tests/Unit/SDK/SdkAutoloaderTest.php index e3e4df742..aa8da7c50 100644 --- a/tests/Unit/SDK/SdkAutoloaderTest.php +++ b/tests/Unit/SDK/SdkAutoloaderTest.php @@ -33,6 +33,7 @@ public function setUp(): void public function tearDown(): void { $this->restoreEnvironmentVariables(); + Globals::reset(); } public function test_disabled_by_default(): void @@ -149,4 +150,13 @@ public function test_enabled_with_excluded_url(): void $_SERVER['REQUEST_URI'] = '/test'; $this->assertFalse(SdkAutoloader::autoload()); } + + public function test_autoload_from_config_file(): void + { + $this->setEnvironmentVariable(Variables::OTEL_PHP_AUTOLOAD_ENABLED, 'true'); + $this->setEnvironmentVariable(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE, __DIR__ . '/fixtures/otel-sdk.yaml'); + $this->assertTrue(SdkAutoloader::autoload()); + //@todo should file-based config create no-op instances for not-provided config? + $this->assertNotInstanceOf(NoopTracerProvider::class, Globals::tracerProvider()); + } } diff --git a/tests/Unit/SDK/fixtures/otel-sdk.yaml b/tests/Unit/SDK/fixtures/otel-sdk.yaml new file mode 100644 index 000000000..c306cc9c9 --- /dev/null +++ b/tests/Unit/SDK/fixtures/otel-sdk.yaml @@ -0,0 +1,7 @@ +file_format: '0.1' + +tracer_provider: + processors: + - simple: + exporter: + console: {} From 20cd7ef0c605b88b3f6c9d2ffc8937f1a295a847 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Wed, 8 May 2024 15:25:24 +1000 Subject: [PATCH 14/43] adding tests for sdk::registerGlobal in passing, remove some dead code for handling invalid booleans - config already handles this correctly --- src/SDK/SdkAutoloader.php | 9 +--- tests/Unit/SDK/SdkAutoloaderTest.php | 1 + tests/Unit/SDK/SdkTest.php | 47 ++++++++++++++----- .../SDK/fixtures/otel-instrumentation.yaml | 3 ++ 4 files changed, 41 insertions(+), 19 deletions(-) create mode 100644 tests/Unit/SDK/fixtures/otel-instrumentation.yaml diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index fbc828d8c..2508b78f5 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -137,14 +137,7 @@ private static function getHookManager(): HookManager */ public static function isEnabled(): bool { - try { - $enabled = Configuration::getBoolean(Variables::OTEL_PHP_AUTOLOAD_ENABLED); - } catch (InvalidArgumentException) { - //invalid setting, assume false - return false; - } - - return $enabled; + return Configuration::getBoolean(Variables::OTEL_PHP_AUTOLOAD_ENABLED); } /** diff --git a/tests/Unit/SDK/SdkAutoloaderTest.php b/tests/Unit/SDK/SdkAutoloaderTest.php index aa8da7c50..e7d88c616 100644 --- a/tests/Unit/SDK/SdkAutoloaderTest.php +++ b/tests/Unit/SDK/SdkAutoloaderTest.php @@ -155,6 +155,7 @@ public function test_autoload_from_config_file(): void { $this->setEnvironmentVariable(Variables::OTEL_PHP_AUTOLOAD_ENABLED, 'true'); $this->setEnvironmentVariable(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE, __DIR__ . '/fixtures/otel-sdk.yaml'); + $this->setEnvironmentVariable(Variables::OTEL_PHP_INSTRUMENTATION_CONFIG_FILE, __DIR__ . '/fixtures/otel-instrumentation.yaml'); $this->assertTrue(SdkAutoloader::autoload()); //@todo should file-based config create no-op instances for not-provided config? $this->assertNotInstanceOf(NoopTracerProvider::class, Globals::tracerProvider()); diff --git a/tests/Unit/SDK/SdkTest.php b/tests/Unit/SDK/SdkTest.php index be35ab3bb..dde94bc14 100644 --- a/tests/Unit/SDK/SdkTest.php +++ b/tests/Unit/SDK/SdkTest.php @@ -5,6 +5,7 @@ namespace OpenTelemetry\Tests\Unit\SDK; use AssertWell\PHPUnitGlobalState\EnvironmentVariables; +use OpenTelemetry\API\Globals; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface; use OpenTelemetry\SDK\Common\Configuration\Variables; @@ -22,6 +23,21 @@ class SdkTest extends TestCase { use EnvironmentVariables; + private TextMapPropagatorInterface $propagator; + private MeterProviderInterface $meterProvider; + private TracerProviderInterface $tracerProvider; + private LoggerProviderInterface $loggerProvider; + private EventLoggerProviderInterface $eventLoggerProvider; + + public function setUp(): void + { + $this->propagator = $this->createMock(TextMapPropagatorInterface::class); + $this->meterProvider = $this->createMock(MeterProviderInterface::class); + $this->tracerProvider = $this->createMock(TracerProviderInterface::class); + $this->loggerProvider = $this->createMock(LoggerProviderInterface::class); + $this->eventLoggerProvider = $this->createMock(EventLoggerProviderInterface::class); + } + public function tearDown(): void { self::restoreEnvironmentVariables(); @@ -79,16 +95,25 @@ public function test_builder(): void public function test_getters(): void { - $propagator = $this->createMock(TextMapPropagatorInterface::class); - $meterProvider = $this->createMock(MeterProviderInterface::class); - $tracerProvider = $this->createMock(TracerProviderInterface::class); - $loggerProvider = $this->createMock(LoggerProviderInterface::class); - $eventLoggerProvider = $this->createMock(EventLoggerProviderInterface::class); - $sdk = new Sdk($tracerProvider, $meterProvider, $loggerProvider, $eventLoggerProvider, $propagator); - $this->assertSame($propagator, $sdk->getPropagator()); - $this->assertSame($meterProvider, $sdk->getMeterProvider()); - $this->assertSame($tracerProvider, $sdk->getTracerProvider()); - $this->assertSame($loggerProvider, $sdk->getLoggerProvider()); - $this->assertSame($eventLoggerProvider, $sdk->getEventLoggerProvider()); + $sdk = new Sdk($this->tracerProvider, $this->meterProvider, $this->loggerProvider, $this->eventLoggerProvider, $this->propagator); + $this->assertSame($this->propagator, $sdk->getPropagator()); + $this->assertSame($this->meterProvider, $sdk->getMeterProvider()); + $this->assertSame($this->tracerProvider, $sdk->getTracerProvider()); + $this->assertSame($this->loggerProvider, $sdk->getLoggerProvider()); + $this->assertSame($this->eventLoggerProvider, $sdk->getEventLoggerProvider()); + } + + public function test_register_global(): void + { + $sdk = new Sdk($this->tracerProvider, $this->meterProvider, $this->loggerProvider, $this->eventLoggerProvider, $this->propagator); + $scope = $sdk->registerGlobal(); + + $this->assertSame($this->meterProvider, Globals::meterProvider()); + $this->assertSame($this->propagator, Globals::propagator()); + $this->assertSame($this->tracerProvider, Globals::tracerProvider()); + $this->assertSame($this->loggerProvider, Globals::loggerProvider()); + $this->assertSame($this->eventLoggerProvider, Globals::eventLoggerProvider()); + + $scope->detach(); } } diff --git a/tests/Unit/SDK/fixtures/otel-instrumentation.yaml b/tests/Unit/SDK/fixtures/otel-instrumentation.yaml new file mode 100644 index 000000000..c4c182e16 --- /dev/null +++ b/tests/Unit/SDK/fixtures/otel-instrumentation.yaml @@ -0,0 +1,3 @@ +config: + - example_instrumentation: + span_name: my-span From c8373a3a9594a4a4b9c069ef9b1e11325cd21338 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Wed, 8 May 2024 15:28:43 +1000 Subject: [PATCH 15/43] linting --- src/SDK/SdkAutoloader.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index 2508b78f5..82db8f12f 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -4,7 +4,6 @@ namespace OpenTelemetry\SDK; -use InvalidArgumentException; use Nevay\SPI\ServiceLoader; use OpenTelemetry\API\Globals; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Context as InstrumentationContext; From 9ba45bef4d5e1bfc25a42c63427184167e543024 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Wed, 8 May 2024 16:00:04 +1000 Subject: [PATCH 16/43] test coverage for globals --- src/API/Globals.php | 7 +++-- .../Instrumentation/InstrumentationTest.php | 28 ++++++++++++++++++- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/API/Globals.php b/src/API/Globals.php index 0761fbadf..14af34c47 100644 --- a/src/API/Globals.php +++ b/src/API/Globals.php @@ -6,7 +6,7 @@ use function assert; use Closure; -use const E_USER_WARNING; +use OpenTelemetry\API\Behavior\LogsMessagesTrait; use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\API\Instrumentation\ContextKeys; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; @@ -17,13 +17,14 @@ use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface; use function sprintf; use Throwable; -use function trigger_error; /** * Provides access to the globally configured instrumentation instances. */ final class Globals { + use LogsMessagesTrait; + /** @var Closure[] */ private static array $initializers = []; private static ?self $globals = null; @@ -97,7 +98,7 @@ private static function globals(): self try { $configurator = $initializer($configurator); } catch (Throwable $e) { - trigger_error(sprintf("Error during opentelemetry initialization: %s\n%s", $e->getMessage(), $e->getTraceAsString()), E_USER_WARNING); + self::logWarning(sprintf("Error during opentelemetry initialization: %s\n%s", $e->getMessage(), $e->getTraceAsString())); } } } finally { diff --git a/tests/Unit/API/Instrumentation/InstrumentationTest.php b/tests/Unit/API/Instrumentation/InstrumentationTest.php index 90e61f310..85206c850 100644 --- a/tests/Unit/API/Instrumentation/InstrumentationTest.php +++ b/tests/Unit/API/Instrumentation/InstrumentationTest.php @@ -4,6 +4,8 @@ namespace OpenTelemetry\Tests\Unit\API\Instrumentation; +use OpenTelemetry\API\Behavior\Internal\Logging; +use OpenTelemetry\API\Behavior\Internal\LogWriter\LogWriterInterface; use OpenTelemetry\API\Globals; use OpenTelemetry\API\Instrumentation\CachedInstrumentation; use OpenTelemetry\API\Instrumentation\Configurator; @@ -23,7 +25,9 @@ use OpenTelemetry\API\Trace\TracerProviderInterface; use OpenTelemetry\Context\Propagation\NoopTextMapPropagator; use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Log\LogLevel; /** * @covers \OpenTelemetry\API\Globals @@ -33,6 +37,14 @@ */ final class InstrumentationTest extends TestCase { + private LogWriterInterface&MockObject $logWriter; + + public function setUp(): void + { + $this->logWriter = $this->createMock(LogWriterInterface::class); + Logging::setLogWriter($this->logWriter); + } + public function tearDown(): void { Globals::reset(); @@ -128,7 +140,21 @@ public function test_initializers(): void }; Globals::registerInitializer($closure); $this->assertFalse($called); - Globals::propagator(); + Globals::init(); $this->assertTrue($called); //@phpstan-ignore-line } + + public function test_initializer_error(): void + { + $closure = function (Configurator $configurator): Configurator { + throw new \Exception('kaboom'); + }; + Globals::registerInitializer($closure); + $this->logWriter->expects($this->once())->method('write')->with( + $this->equalTo(LogLevel::WARNING), + $this->anything(), + $this->anything(), + ); + Globals::init(); + } } From a4d904679ec8459c5ad246009fd1863c2d9f56ae Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 9 May 2024 13:34:42 +1000 Subject: [PATCH 17/43] add opentelemetry extension to developer image and actions --- .github/workflows/php.yml | 2 +- docker/Dockerfile | 1 + .../configure_instrumentation.php | 3 +- .../ExtensionHookManager.php | 2 +- .../ExtensionHookManagerTest.php | 102 ++++++++++++++++++ 5 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 tests/Unit/API/Instrumentation/AutoInstrumentation/ExtensionHookManagerTest.php diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index ad3362944..e1ec860fc 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -21,7 +21,7 @@ jobs: experimental: true composer_args: "--ignore-platform-reqs" env: - extensions: ast, grpc, protobuf + extensions: ast, grpc, opentelemetry, protobuf steps: - name: Set cache key diff --git a/docker/Dockerfile b/docker/Dockerfile index c78edea4f..36b37ec09 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -11,6 +11,7 @@ RUN chmod +x /usr/local/bin/install-php-extensions \ grpc \ intl\ opcache \ + opentelemetry \ pcntl \ protobuf \ sockets \ diff --git a/examples/instrumentation/configure_instrumentation.php b/examples/instrumentation/configure_instrumentation.php index d1aeeaa53..f614b7669 100644 --- a/examples/instrumentation/configure_instrumentation.php +++ b/examples/instrumentation/configure_instrumentation.php @@ -5,6 +5,7 @@ namespace _; use Nevay\SPI\ServiceLoader; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Context as InstrumentationContext; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ExtensionHookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation; use OpenTelemetry\Config\SDK\Configuration; @@ -24,7 +25,7 @@ $hookManager = new ExtensionHookManager(); $storage = \OpenTelemetry\Context\Context::storage(); -$context = new Context($sdk->getTracerProvider(), $sdk->getMeterProvider()); +$context = new InstrumentationContext($sdk->getTracerProvider(), $sdk->getMeterProvider()); foreach (ServiceLoader::load(Instrumentation::class) as $instrumentation) { $instrumentation->register($hookManager, $context, $configuration, $storage); diff --git a/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php b/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php index d779c8de1..661ba0300 100644 --- a/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php +++ b/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php @@ -69,7 +69,7 @@ private function bindHookScope(?Closure $closure): ?Closure return static function (mixed ...$args) use ($closure, $contextKey): mixed { if (!Context::getCurrent()->get($contextKey)) { - return null; + return $args[2]; } return $closure(...$args); diff --git a/tests/Unit/API/Instrumentation/AutoInstrumentation/ExtensionHookManagerTest.php b/tests/Unit/API/Instrumentation/AutoInstrumentation/ExtensionHookManagerTest.php new file mode 100644 index 000000000..4f0fc0f52 --- /dev/null +++ b/tests/Unit/API/Instrumentation/AutoInstrumentation/ExtensionHookManagerTest.php @@ -0,0 +1,102 @@ +markTestSkipped(); + } + $tracerProvider = $this->createMock(TracerProviderInterface::class); + $this->context = new InstrumentationContext($tracerProvider); + $this->registry = new ConfigurationRegistry(); + $this->hookManager = new ExtensionHookManager(); + $this->storage = Context::storage(); + $this->scope = $this->storage->attach($this->hookManager->enable($this->storage->current())); + $this->target = new class() { + public function test(): int + { + return 3; + } + }; + } + + public function tearDown(): void + { + $this->scope->detach(); + } + + public function test_modify_return_value_from_post_hook(): void + { + $instrumentation = $this->createInstrumentation($this->target::class, 'test', function () { + }, function (): int { + return 99; + }); + $instrumentation->register($this->hookManager, $this->context, $this->registry, $this->storage); + + $returnVal = $this->target->test(); + $this->assertSame(99, $returnVal); + } + + public function test_modify_return_value_from_post_hook_when_hook_manager_disabled(): void + { + $scope = $this->storage->attach($this->hookManager->disable($this->storage->current())); + $instrumentation = $this->createInstrumentation($this->target::class, 'test', function () { + }, function (): int { + return 99; + }); + $instrumentation->register($this->hookManager, $this->context, $this->registry, $this->storage); + + try { + $returnVal = $this->target->test(); + $this->assertSame(3, $returnVal, 'original value, since hook did not run'); + } finally { + $scope->detach(); + } + } + + private function createInstrumentation(string $class, string $method, $pre, $post): Instrumentation + { + return new class($class, $method, $pre, $post) implements Instrumentation { + private $pre; + private $post; + public function __construct(private readonly string $class, private readonly string $method, ?callable $pre = null, ?callable $post = null) + { + $this->pre = $pre; + $this->post = $post; + } + public function register(HookManager $hookManager, InstrumentationContext $context, ConfigurationRegistry $configuration, ContextStorageInterface $storage): void + { + $hookManager->hook($this->class, $this->method, $this->pre, $this->post); /*function(){ + echo 'pre'; + }, function(){ + echo 'post'; + });*/ + } + }; + } +} From e704719cdd3ca3ed4ca705137ab2aca0c2eb3e8e Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 9 May 2024 14:07:52 +1000 Subject: [PATCH 18/43] always register instrumentations via SPI --- src/SDK/SdkAutoloader.php | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index 82db8f12f..a2f7d6c49 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -36,12 +36,13 @@ public static function autoload(): bool if (!self::isEnabled() || self::isExcludedUrl()) { return false; } + $sdk = null; if (Configuration::has(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE)) { $sdk = self::createAndRegisterFileBasedSdk(); - self::configureInstrumentation($sdk); } else { Globals::registerInitializer(fn ($configurator) => self::environmentBasedInitializer($configurator)); } + self::registerInstrumentations($sdk); return true; } @@ -95,9 +96,11 @@ public static function createAndRegisterFileBasedSdk(): Sdk } /** + * Register any auto-instrumentation configured through SPI + * * @phan-suppress PhanUndeclaredClassMethod */ - private static function configureInstrumentation(Sdk $sdk): void + private static function registerInstrumentations(?Sdk $sdk): void { if (!Configuration::has(Variables::OTEL_PHP_INSTRUMENTATION_CONFIG_FILE)) { return; @@ -106,7 +109,11 @@ private static function configureInstrumentation(Sdk $sdk): void $configuration = Instrumentation::parseFile($file)->create(); $storage = Context::storage(); $hookManager = self::getHookManager(); - $context = new InstrumentationContext($sdk->getTracerProvider(), $sdk->getMeterProvider(), $sdk->getLoggerProvider()); + $context = new InstrumentationContext( + $sdk?->getTracerProvider() ?? Globals::tracerProvider(), + $sdk?->getMeterProvider() ?? Globals::meterProvider(), + $sdk?->getLoggerProvider() ?? Globals::loggerProvider(), + ); foreach (ServiceLoader::load(\OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation::class) as $instrumentation) { /** @var \OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation $instrumentation */ $instrumentation->register($hookManager, $context, $configuration, $storage); From d94d4cc9e60041c151f75a47a30483f454433055 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Fri, 10 May 2024 14:10:01 +1000 Subject: [PATCH 19/43] register globals initializer for file-config sdk allow SDK created from config file to coexist with components using globals initializer --- src/SDK/Sdk.php | 19 ------------------- src/SDK/SdkAutoloader.php | 21 ++++++++++++++------- src/SDK/SdkBuilder.php | 13 ++++++++++++- tests/Unit/SDK/SdkAutoloaderTest.php | 20 ++++++++++++++++++++ tests/Unit/SDK/SdkTest.php | 15 --------------- 5 files changed, 46 insertions(+), 42 deletions(-) diff --git a/src/SDK/Sdk.php b/src/SDK/Sdk.php index f18f6f9be..78f541f64 100644 --- a/src/SDK/Sdk.php +++ b/src/SDK/Sdk.php @@ -4,13 +4,10 @@ namespace OpenTelemetry\SDK; -use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; use OpenTelemetry\API\Metrics\MeterProviderInterface; use OpenTelemetry\API\Trace\TracerProviderInterface; -use OpenTelemetry\Context\Context; use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface; -use OpenTelemetry\Context\ScopeInterface; use OpenTelemetry\SDK\Common\Configuration\Configuration; use OpenTelemetry\SDK\Common\Configuration\Variables; use OpenTelemetry\SDK\Logs\LoggerProviderInterface; @@ -72,20 +69,4 @@ public function getPropagator(): TextMapPropagatorInterface { return $this->propagator; } - - /** - * Globally register the SDK so that its components are available via `Globals` - */ - public function registerGlobal(): ScopeInterface - { - $context = Configurator::create() - ->withPropagator($this->getPropagator()) - ->withTracerProvider($this->getTracerProvider()) - ->withMeterProvider($this->getMeterProvider()) - ->withLoggerProvider($this->getLoggerProvider()) - ->withEventLoggerProvider($this->getEventLoggerProvider()) - ->storeInContext(); - - return Context::storage()->attach($context); - } } diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index a2f7d6c49..9da34d11d 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -38,7 +38,16 @@ public static function autoload(): bool } $sdk = null; if (Configuration::has(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE)) { - $sdk = self::createAndRegisterFileBasedSdk(); + $sdk = self::createSdkFromFileConfig(); + Globals::registerInitializer(function (Configurator $configurator) use ($sdk) { + return $configurator + ->withTracerProvider($sdk->getTracerProvider()) + ->withMeterProvider($sdk->getMeterProvider()) + ->withLoggerProvider($sdk->getLoggerProvider()) + ->withPropagator($sdk->getPropagator()) + ->withEventLoggerProvider($sdk->getEventLoggerProvider()) + ; + }); } else { Globals::registerInitializer(fn ($configurator) => self::environmentBasedInitializer($configurator)); } @@ -47,7 +56,7 @@ public static function autoload(): bool return true; } - public static function environmentBasedInitializer(Configurator $configurator): Configurator + private static function environmentBasedInitializer(Configurator $configurator): Configurator { $propagator = (new PropagatorFactory())->create(); if (Sdk::isDisabled()) { @@ -82,17 +91,15 @@ public static function environmentBasedInitializer(Configurator $configurator): ; } - public static function createAndRegisterFileBasedSdk(): Sdk + private static function createSdkFromFileConfig(): Sdk { $file = Configuration::getString(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE); $config = SdkConfiguration::parseFile($file); - $sdk = $config + + return $config ->create() ->setAutoShutdown(true) ->build(); - $sdk->registerGlobal(); - - return $sdk; } /** diff --git a/src/SDK/SdkBuilder.php b/src/SDK/SdkBuilder.php index 154ff5063..403713e0b 100644 --- a/src/SDK/SdkBuilder.php +++ b/src/SDK/SdkBuilder.php @@ -4,8 +4,10 @@ namespace OpenTelemetry\SDK; +use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; use OpenTelemetry\API\Logs\NoopEventLoggerProvider; +use OpenTelemetry\Context\Context; use OpenTelemetry\Context\Propagation\NoopTextMapPropagator; use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface; use OpenTelemetry\Context\ScopeInterface; @@ -95,6 +97,15 @@ public function build(): Sdk public function buildAndRegisterGlobal(): ScopeInterface { - return $this->build()->registerGlobal(); + $sdk = $this->build(); + $context = Configurator::create() + ->withPropagator($sdk->getPropagator()) + ->withTracerProvider($sdk->getTracerProvider()) + ->withMeterProvider($sdk->getMeterProvider()) + ->withLoggerProvider($sdk->getLoggerProvider()) + ->withEventLoggerProvider($sdk->getEventLoggerProvider()) + ->storeInContext(); + + return Context::storage()->attach($context); } } diff --git a/tests/Unit/SDK/SdkAutoloaderTest.php b/tests/Unit/SDK/SdkAutoloaderTest.php index e7d88c616..2ce12071c 100644 --- a/tests/Unit/SDK/SdkAutoloaderTest.php +++ b/tests/Unit/SDK/SdkAutoloaderTest.php @@ -6,12 +6,14 @@ use AssertWell\PHPUnitGlobalState\EnvironmentVariables; use OpenTelemetry\API\Globals; +use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\API\LoggerHolder; use OpenTelemetry\API\Logs\NoopEventLoggerProvider; use OpenTelemetry\API\Logs\NoopLoggerProvider; use OpenTelemetry\API\Metrics\Noop\NoopMeterProvider; use OpenTelemetry\API\Trace\NoopTracerProvider; use OpenTelemetry\Context\Propagation\NoopTextMapPropagator; +use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface; use OpenTelemetry\SDK\Common\Configuration\Variables; use OpenTelemetry\SDK\SdkAutoloader; use PHPUnit\Framework\TestCase; @@ -160,4 +162,22 @@ public function test_autoload_from_config_file(): void //@todo should file-based config create no-op instances for not-provided config? $this->assertNotInstanceOf(NoopTracerProvider::class, Globals::tracerProvider()); } + + /** + * Tests the scenario where the SDK is created from config file, but a custom component + * uses composer's autoload->files to add its own initializer + */ + public function test_autoload_with_late_globals_initializer(): void + { + $this->setEnvironmentVariable(Variables::OTEL_PHP_AUTOLOAD_ENABLED, 'true'); + $this->setEnvironmentVariable(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE, __DIR__ . '/fixtures/otel-sdk.yaml'); + $this->assertTrue(SdkAutoloader::autoload()); + //SDK is configured, but globals have not been initialized yet, so we can add more initializers + $propagator = $this->createMock(TextMapPropagatorInterface::class); + Globals::registerInitializer(function(Configurator $configurator) use ($propagator) { + return $configurator->withPropagator($propagator); + }); + + $this->assertSame($propagator, Globals::propagator()); + } } diff --git a/tests/Unit/SDK/SdkTest.php b/tests/Unit/SDK/SdkTest.php index dde94bc14..74c88b453 100644 --- a/tests/Unit/SDK/SdkTest.php +++ b/tests/Unit/SDK/SdkTest.php @@ -5,7 +5,6 @@ namespace OpenTelemetry\Tests\Unit\SDK; use AssertWell\PHPUnitGlobalState\EnvironmentVariables; -use OpenTelemetry\API\Globals; use OpenTelemetry\API\Logs\EventLoggerProviderInterface; use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface; use OpenTelemetry\SDK\Common\Configuration\Variables; @@ -102,18 +101,4 @@ public function test_getters(): void $this->assertSame($this->loggerProvider, $sdk->getLoggerProvider()); $this->assertSame($this->eventLoggerProvider, $sdk->getEventLoggerProvider()); } - - public function test_register_global(): void - { - $sdk = new Sdk($this->tracerProvider, $this->meterProvider, $this->loggerProvider, $this->eventLoggerProvider, $this->propagator); - $scope = $sdk->registerGlobal(); - - $this->assertSame($this->meterProvider, Globals::meterProvider()); - $this->assertSame($this->propagator, Globals::propagator()); - $this->assertSame($this->tracerProvider, Globals::tracerProvider()); - $this->assertSame($this->loggerProvider, Globals::loggerProvider()); - $this->assertSame($this->eventLoggerProvider, Globals::eventLoggerProvider()); - - $scope->detach(); - } } From 04aa26a3cf877bb6dce50dd5f3cf5c2ead2b5371 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Fri, 10 May 2024 14:15:34 +1000 Subject: [PATCH 20/43] linting --- tests/Unit/SDK/SdkAutoloaderTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/SDK/SdkAutoloaderTest.php b/tests/Unit/SDK/SdkAutoloaderTest.php index 2ce12071c..7a197bf4e 100644 --- a/tests/Unit/SDK/SdkAutoloaderTest.php +++ b/tests/Unit/SDK/SdkAutoloaderTest.php @@ -174,7 +174,7 @@ public function test_autoload_with_late_globals_initializer(): void $this->assertTrue(SdkAutoloader::autoload()); //SDK is configured, but globals have not been initialized yet, so we can add more initializers $propagator = $this->createMock(TextMapPropagatorInterface::class); - Globals::registerInitializer(function(Configurator $configurator) use ($propagator) { + Globals::registerInitializer(function (Configurator $configurator) use ($propagator) { return $configurator->withPropagator($propagator); }); From 7262680b7aac1c70f5fcc94a8ee167cffd51ed1a Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Fri, 10 May 2024 14:39:02 +1000 Subject: [PATCH 21/43] remove globals init function --- src/API/Globals.php | 7 ------ .../ConfigurationRegistryTest.php | 23 ++++++++++++++++++ .../AutoInstrumentation/ContextTest.php | 24 +++++++++++++++++++ .../NoopHookManagerTest.php | 22 +++++++++++++++++ .../Instrumentation/InstrumentationTest.php | 4 ++-- 5 files changed, 71 insertions(+), 9 deletions(-) create mode 100644 tests/Unit/API/Instrumentation/AutoInstrumentation/ConfigurationRegistryTest.php create mode 100644 tests/Unit/API/Instrumentation/AutoInstrumentation/ContextTest.php create mode 100644 tests/Unit/API/Instrumentation/AutoInstrumentation/NoopHookManagerTest.php diff --git a/src/API/Globals.php b/src/API/Globals.php index 14af34c47..97c824e49 100644 --- a/src/API/Globals.php +++ b/src/API/Globals.php @@ -74,13 +74,6 @@ public static function registerInitializer(Closure $initializer): void self::$initializers[] = $initializer; } - public static function init(): void - { - if (self::$globals === null) { - self::globals(); - } - } - /** * @phan-suppress PhanTypeMismatchReturnNullable */ diff --git a/tests/Unit/API/Instrumentation/AutoInstrumentation/ConfigurationRegistryTest.php b/tests/Unit/API/Instrumentation/AutoInstrumentation/ConfigurationRegistryTest.php new file mode 100644 index 000000000..0df4eeabb --- /dev/null +++ b/tests/Unit/API/Instrumentation/AutoInstrumentation/ConfigurationRegistryTest.php @@ -0,0 +1,23 @@ +add($config); + + $this->assertSame($config, $registry->get($config::class)); + } +} diff --git a/tests/Unit/API/Instrumentation/AutoInstrumentation/ContextTest.php b/tests/Unit/API/Instrumentation/AutoInstrumentation/ContextTest.php new file mode 100644 index 000000000..e5cb4c238 --- /dev/null +++ b/tests/Unit/API/Instrumentation/AutoInstrumentation/ContextTest.php @@ -0,0 +1,24 @@ +assertInstanceOf(NoopTracerProvider::class, $context->tracerProvider); + $this->assertInstanceOf(NoopMeterProvider::class, $context->meterProvider); + $this->assertInstanceOf(NoopLoggerProvider::class, $context->loggerProvider); + } +} diff --git a/tests/Unit/API/Instrumentation/AutoInstrumentation/NoopHookManagerTest.php b/tests/Unit/API/Instrumentation/AutoInstrumentation/NoopHookManagerTest.php new file mode 100644 index 000000000..da2bcacee --- /dev/null +++ b/tests/Unit/API/Instrumentation/AutoInstrumentation/NoopHookManagerTest.php @@ -0,0 +1,22 @@ +createMock(ContextInterface::class); + $hookManager = new NoopHookManager(); + $this->assertSame($context, $hookManager->enable($context)); + $this->assertSame($context, $hookManager->disable($context)); + } +} diff --git a/tests/Unit/API/Instrumentation/InstrumentationTest.php b/tests/Unit/API/Instrumentation/InstrumentationTest.php index 85206c850..f002c0110 100644 --- a/tests/Unit/API/Instrumentation/InstrumentationTest.php +++ b/tests/Unit/API/Instrumentation/InstrumentationTest.php @@ -140,7 +140,7 @@ public function test_initializers(): void }; Globals::registerInitializer($closure); $this->assertFalse($called); - Globals::init(); + Globals::propagator(); $this->assertTrue($called); //@phpstan-ignore-line } @@ -155,6 +155,6 @@ public function test_initializer_error(): void $this->anything(), $this->anything(), ); - Globals::init(); + Globals::propagator(); } } From 579713a61472562a6f801ef99843c618e9b5f595 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Fri, 10 May 2024 15:15:15 +1000 Subject: [PATCH 22/43] fix phan warning --- .phan/config.php | 1 - 1 file changed, 1 deletion(-) diff --git a/.phan/config.php b/.phan/config.php index d8c6d47e0..9a20886d7 100644 --- a/.phan/config.php +++ b/.phan/config.php @@ -375,7 +375,6 @@ 'vendor/guzzlehttp', 'vendor/psr', 'vendor/php-http', - 'vendor/phan/phan/src/Phan', 'vendor/phpunit/phpunit/src', 'vendor/google/protobuf/src', ], From 8fd372405074dd483a5c7be7001ac7de14b9c948 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Mon, 13 May 2024 14:15:24 +1000 Subject: [PATCH 23/43] simplify hook manager - drop storage from hook manager: can't guarantee that something else, eg swoole, won't modify storage - drop context from hook manager: we must lazy-load globals via initializers, because not all instrumentations use SPI (although that may change in future) - default hook manager to enabled --- examples/src/ExampleInstrumentation.php | 21 +++-- src/API/Globals.php | 1 + .../ExtensionHookManager.php | 6 +- .../AutoInstrumentation/Instrumentation.php | 4 +- src/SDK/SdkAutoloader.php | 43 +++------ .../ExtensionHookManagerTest.php | 93 ++++++++++++------- tests/Unit/SDK/SdkAutoloaderTest.php | 2 +- 7 files changed, 93 insertions(+), 77 deletions(-) diff --git a/examples/src/ExampleInstrumentation.php b/examples/src/ExampleInstrumentation.php index ef33c0e71..0ae614141 100644 --- a/examples/src/ExampleInstrumentation.php +++ b/examples/src/ExampleInstrumentation.php @@ -5,40 +5,43 @@ namespace OpenTelemetry\Example; use Exception; +use OpenTelemetry\API\Globals; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ConfigurationRegistry; -use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Context; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation; use OpenTelemetry\API\Trace\Span; -use OpenTelemetry\Context\ContextStorageInterface; +use OpenTelemetry\Context\Context; final class ExampleInstrumentation implements Instrumentation { - public function register(HookManager $hookManager, Context $context, ConfigurationRegistry $configuration, ContextStorageInterface $storage): void + /** + * @todo can we pass in just the config for _this_ instrumentation, rather than all? + */ + public function register(HookManager $hookManager, ConfigurationRegistry $configuration): void { $config = $configuration->get(ExampleConfig::class) ?? throw new Exception('example instrumentation must be configured'); if (!$config->enabled) { return; } - $tracer = $context->tracerProvider->getTracer('example-instrumentation'); + $tracer = Globals::tracerProvider()->getTracer('example-instrumentation'); $hookManager->hook( Example::class, 'test', - static function () use ($tracer, $config, $storage): void { - $context = $storage->current(); + static function () use ($tracer, $config): void { + $context = Context::getCurrent(); $span = $tracer ->spanBuilder($config->spanName) ->setParent($context) ->startSpan(); - $storage->attach($span->storeInContext($context)); + Context::storage()->attach($span->storeInContext($context)); }, - static function () use ($storage): void { - if (!$scope = $storage->scope()) { + static function (): void { + if (!$scope = Context::storage()->scope()) { return; } diff --git a/src/API/Globals.php b/src/API/Globals.php index 97c824e49..aa3c06141 100644 --- a/src/API/Globals.php +++ b/src/API/Globals.php @@ -68,6 +68,7 @@ public static function eventLoggerProvider(): EventLoggerProviderInterface * * @internal * @psalm-internal OpenTelemetry + * @todo In a future (breaking) change, we can remove `Registry` and globals initializers, in favor of SPI. */ public static function registerInitializer(Closure $initializer): void { diff --git a/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php b/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php index 661ba0300..82f2b3493 100644 --- a/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php +++ b/src/API/Instrumentation/AutoInstrumentation/ExtensionHookManager.php @@ -44,7 +44,7 @@ public function enable(ContextInterface $context): ContextInterface public function disable(ContextInterface $context): ContextInterface { - return $context->with($this->contextKey, null); + return $context->with($this->contextKey, false); } private function bindHookScope(?Closure $closure): ?Closure @@ -59,7 +59,7 @@ private function bindHookScope(?Closure $closure): ?Closure // TODO Add an option flag to ext-opentelemetry `hook` that configures whether return values should be used? if (!$reflection->getReturnType() || (string) $reflection->getReturnType() === 'void') { return static function (mixed ...$args) use ($closure, $contextKey): void { - if (!Context::getCurrent()->get($contextKey)) { + if (Context::getCurrent()->get($contextKey) === false) { return; } @@ -68,7 +68,7 @@ private function bindHookScope(?Closure $closure): ?Closure } return static function (mixed ...$args) use ($closure, $contextKey): mixed { - if (!Context::getCurrent()->get($contextKey)) { + if (Context::getCurrent()->get($contextKey) === false) { return $args[2]; } diff --git a/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php b/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php index d46644f16..2224e4fa0 100644 --- a/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php +++ b/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php @@ -4,9 +4,7 @@ namespace OpenTelemetry\API\Instrumentation\AutoInstrumentation; -use OpenTelemetry\Context\ContextStorageInterface; - interface Instrumentation { - public function register(HookManager $hookManager, Context $context, ConfigurationRegistry $configuration, ContextStorageInterface $storage): void; + public function register(HookManager $hookManager, ConfigurationRegistry $configuration): void; } diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index 9da34d11d..b7dddc6b4 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -6,13 +6,11 @@ use Nevay\SPI\ServiceLoader; use OpenTelemetry\API\Globals; -use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Context as InstrumentationContext; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\NoopHookManager; use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\Config\SDK\Configuration as SdkConfiguration; use OpenTelemetry\Config\SDK\Instrumentation; -use OpenTelemetry\Context\Context; use OpenTelemetry\SDK\Common\Configuration\Configuration; use OpenTelemetry\SDK\Common\Configuration\Variables; use OpenTelemetry\SDK\Common\Util\ShutdownHandler; @@ -36,22 +34,12 @@ public static function autoload(): bool if (!self::isEnabled() || self::isExcludedUrl()) { return false; } - $sdk = null; if (Configuration::has(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE)) { - $sdk = self::createSdkFromFileConfig(); - Globals::registerInitializer(function (Configurator $configurator) use ($sdk) { - return $configurator - ->withTracerProvider($sdk->getTracerProvider()) - ->withMeterProvider($sdk->getMeterProvider()) - ->withLoggerProvider($sdk->getLoggerProvider()) - ->withPropagator($sdk->getPropagator()) - ->withEventLoggerProvider($sdk->getEventLoggerProvider()) - ; - }); + Globals::registerInitializer(fn ($configurator) => self::fileBasedInitializer($configurator)); } else { Globals::registerInitializer(fn ($configurator) => self::environmentBasedInitializer($configurator)); } - self::registerInstrumentations($sdk); + self::registerInstrumentations(); return true; } @@ -91,15 +79,23 @@ private static function environmentBasedInitializer(Configurator $configurator): ; } - private static function createSdkFromFileConfig(): Sdk + private static function fileBasedInitializer(Configurator $configurator): Configurator { $file = Configuration::getString(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE); $config = SdkConfiguration::parseFile($file); - return $config + $sdk = $config ->create() ->setAutoShutdown(true) ->build(); + + return $configurator + ->withTracerProvider($sdk->getTracerProvider()) + ->withMeterProvider($sdk->getMeterProvider()) + ->withLoggerProvider($sdk->getLoggerProvider()) + ->withPropagator($sdk->getPropagator()) + ->withEventLoggerProvider($sdk->getEventLoggerProvider()) + ; } /** @@ -107,23 +103,17 @@ private static function createSdkFromFileConfig(): Sdk * * @phan-suppress PhanUndeclaredClassMethod */ - private static function registerInstrumentations(?Sdk $sdk): void + private static function registerInstrumentations(): void { if (!Configuration::has(Variables::OTEL_PHP_INSTRUMENTATION_CONFIG_FILE)) { return; } $file = Configuration::getString(Variables::OTEL_PHP_INSTRUMENTATION_CONFIG_FILE); $configuration = Instrumentation::parseFile($file)->create(); - $storage = Context::storage(); $hookManager = self::getHookManager(); - $context = new InstrumentationContext( - $sdk?->getTracerProvider() ?? Globals::tracerProvider(), - $sdk?->getMeterProvider() ?? Globals::meterProvider(), - $sdk?->getLoggerProvider() ?? Globals::loggerProvider(), - ); foreach (ServiceLoader::load(\OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation::class) as $instrumentation) { /** @var \OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation $instrumentation */ - $instrumentation->register($hookManager, $context, $configuration, $storage); + $instrumentation->register($hookManager, $configuration); } } @@ -134,11 +124,6 @@ private static function getHookManager(): HookManager { /** @var HookManager $hookManager */ foreach (ServiceLoader::load(HookManager::class) as $hookManager) { - $scope = $hookManager->enable(Context::getCurrent())->activate(); - ShutdownHandler::register(function () use ($scope) { - $scope->detach(); - }); - return $hookManager; } diff --git a/tests/Unit/API/Instrumentation/AutoInstrumentation/ExtensionHookManagerTest.php b/tests/Unit/API/Instrumentation/AutoInstrumentation/ExtensionHookManagerTest.php index 4f0fc0f52..1bd7c1741 100644 --- a/tests/Unit/API/Instrumentation/AutoInstrumentation/ExtensionHookManagerTest.php +++ b/tests/Unit/API/Instrumentation/AutoInstrumentation/ExtensionHookManagerTest.php @@ -5,13 +5,12 @@ namespace Unit\API\Instrumentation\AutoInstrumentation; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ConfigurationRegistry; -use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Context as InstrumentationContext; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ExtensionHookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation; +use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\API\Trace\TracerProviderInterface; use OpenTelemetry\Context\Context; -use OpenTelemetry\Context\ContextStorageInterface; use OpenTelemetry\Context\ScopeInterface; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; @@ -19,12 +18,9 @@ #[CoversClass(ExtensionHookManager::class)] class ExtensionHookManagerTest extends TestCase { - private object $target; - private InstrumentationContext $context; private ConfigurationRegistry $registry; private ScopeInterface $scope; private HookManager $hookManager; - private ContextStorageInterface $storage; public function setUp(): void { @@ -32,17 +28,11 @@ public function setUp(): void $this->markTestSkipped(); } $tracerProvider = $this->createMock(TracerProviderInterface::class); - $this->context = new InstrumentationContext($tracerProvider); + $this->scope = Configurator::create() + ->withTracerProvider($tracerProvider) + ->activate(); $this->registry = new ConfigurationRegistry(); $this->hookManager = new ExtensionHookManager(); - $this->storage = Context::storage(); - $this->scope = $this->storage->attach($this->hookManager->enable($this->storage->current())); - $this->target = new class() { - public function test(): int - { - return 3; - } - }; } public function tearDown(): void @@ -52,28 +42,65 @@ public function tearDown(): void public function test_modify_return_value_from_post_hook(): void { - $instrumentation = $this->createInstrumentation($this->target::class, 'test', function () { + $target = new class() { + public function test(): int + { + return 1; + } + }; + $instrumentation = $this->createInstrumentation($target::class, 'test', function () { }, function (): int { return 99; }); - $instrumentation->register($this->hookManager, $this->context, $this->registry, $this->storage); + $instrumentation->register($this->hookManager, $this->registry); - $returnVal = $this->target->test(); + $returnVal = $target->test(); $this->assertSame(99, $returnVal); } - public function test_modify_return_value_from_post_hook_when_hook_manager_disabled(): void + public function test_hook_manager_disabled(): void { - $scope = $this->storage->attach($this->hookManager->disable($this->storage->current())); - $instrumentation = $this->createInstrumentation($this->target::class, 'test', function () { + $target = new class() { + public function test(): int + { + return 2; + } + }; + $instrumentation = $this->createInstrumentation($target::class, 'test', function () { }, function (): int { - return 99; + $this->fail('post hook not expected to be called'); + }); + $instrumentation->register($this->hookManager, $this->registry); + + $scope = $this->hookManager->disable(Context::getCurrent())->activate(); + + try { + $returnVal = $target->test(); + } finally { + $scope->detach(); + } + $this->assertSame(2, $returnVal, 'original value, since hook did not run'); + } + + public function test_disable_hook_manager_after_use(): void + { + $target = new class() { + public function test(): int + { + return 3; + } + }; + $instrumentation = $this->createInstrumentation($target::class, 'test', function () { + }, function (): int { + return 123; }); - $instrumentation->register($this->hookManager, $this->context, $this->registry, $this->storage); + $instrumentation->register($this->hookManager, $this->registry); + $this->assertSame(123, $target->test(), 'post hook function ran and modified return value'); + + $scope = $this->hookManager->disable(Context::getCurrent())->activate(); try { - $returnVal = $this->target->test(); - $this->assertSame(3, $returnVal, 'original value, since hook did not run'); + $this->assertSame(3, $target->test(), 'post hook function did not run'); } finally { $scope->detach(); } @@ -84,18 +111,20 @@ private function createInstrumentation(string $class, string $method, $pre, $pos return new class($class, $method, $pre, $post) implements Instrumentation { private $pre; private $post; - public function __construct(private readonly string $class, private readonly string $method, ?callable $pre = null, ?callable $post = null) - { + + public function __construct( + private readonly string $class, + private readonly string $method, + ?callable $pre = null, + ?callable $post = null, + ) { $this->pre = $pre; $this->post = $post; } - public function register(HookManager $hookManager, InstrumentationContext $context, ConfigurationRegistry $configuration, ContextStorageInterface $storage): void + + public function register(HookManager $hookManager, ConfigurationRegistry $configuration): void { - $hookManager->hook($this->class, $this->method, $this->pre, $this->post); /*function(){ - echo 'pre'; - }, function(){ - echo 'post'; - });*/ + $hookManager->hook($this->class, $this->method, $this->pre, $this->post); } }; } diff --git a/tests/Unit/SDK/SdkAutoloaderTest.php b/tests/Unit/SDK/SdkAutoloaderTest.php index 7a197bf4e..a45334574 100644 --- a/tests/Unit/SDK/SdkAutoloaderTest.php +++ b/tests/Unit/SDK/SdkAutoloaderTest.php @@ -158,8 +158,8 @@ public function test_autoload_from_config_file(): void $this->setEnvironmentVariable(Variables::OTEL_PHP_AUTOLOAD_ENABLED, 'true'); $this->setEnvironmentVariable(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE, __DIR__ . '/fixtures/otel-sdk.yaml'); $this->setEnvironmentVariable(Variables::OTEL_PHP_INSTRUMENTATION_CONFIG_FILE, __DIR__ . '/fixtures/otel-instrumentation.yaml'); + $this->assertTrue(SdkAutoloader::autoload()); - //@todo should file-based config create no-op instances for not-provided config? $this->assertNotInstanceOf(NoopTracerProvider::class, Globals::tracerProvider()); } From e6f85febedcee72432299c5f605cfcad2573d13d Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Mon, 13 May 2024 14:22:00 +1000 Subject: [PATCH 24/43] add todo to deprecate Registry in future --- src/SDK/Registry.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/SDK/Registry.php b/src/SDK/Registry.php index 32ffe1522..300a8a40c 100644 --- a/src/SDK/Registry.php +++ b/src/SDK/Registry.php @@ -16,6 +16,7 @@ /** * A registry to enable central registration of components that the SDK requires but which may be provided * by non-SDK modules, such as contrib and extension. + * @todo [breaking] deprecate this mechanism of setting up components, in favor of using SPI. */ class Registry { From d032942510851c104d9d36214dcfffabd52f007a Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Mon, 13 May 2024 15:09:36 +1000 Subject: [PATCH 25/43] autoload instrumentations without config if no config provided, still try to load them. wrap registration in a try/catch/log --- src/SDK/SdkAutoloader.php | 18 +++++++++++++----- tests/Unit/SDK/SdkAutoloaderTest.php | 8 ++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index b7dddc6b4..e5e07028f 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -5,6 +5,7 @@ namespace OpenTelemetry\SDK; use Nevay\SPI\ServiceLoader; +use OpenTelemetry\API\Behavior\LogsMessagesTrait; use OpenTelemetry\API\Globals; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\NoopHookManager; @@ -23,12 +24,15 @@ use OpenTelemetry\SDK\Trace\SamplerFactory; use OpenTelemetry\SDK\Trace\SpanProcessorFactory; use OpenTelemetry\SDK\Trace\TracerProviderBuilder; +use Throwable; /** * @psalm-suppress RedundantCast */ class SdkAutoloader { + use LogsMessagesTrait; + public static function autoload(): bool { if (!self::isEnabled() || self::isExcludedUrl()) { @@ -105,15 +109,19 @@ private static function fileBasedInitializer(Configurator $configurator): Config */ private static function registerInstrumentations(): void { - if (!Configuration::has(Variables::OTEL_PHP_INSTRUMENTATION_CONFIG_FILE)) { - return; - } - $file = Configuration::getString(Variables::OTEL_PHP_INSTRUMENTATION_CONFIG_FILE); + $file = Configuration::has(Variables::OTEL_PHP_INSTRUMENTATION_CONFIG_FILE) + ? Configuration::getString(Variables::OTEL_PHP_INSTRUMENTATION_CONFIG_FILE) + : []; $configuration = Instrumentation::parseFile($file)->create(); $hookManager = self::getHookManager(); foreach (ServiceLoader::load(\OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation::class) as $instrumentation) { /** @var \OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation $instrumentation */ - $instrumentation->register($hookManager, $configuration); + try { + $instrumentation->register($hookManager, $configuration); + } catch (Throwable $t) { + self::logError(sprintf('Unable to load instrumentation: %s', $instrumentation::class), ['exception' => $t]); + } + } } diff --git a/tests/Unit/SDK/SdkAutoloaderTest.php b/tests/Unit/SDK/SdkAutoloaderTest.php index a45334574..2492c0d98 100644 --- a/tests/Unit/SDK/SdkAutoloaderTest.php +++ b/tests/Unit/SDK/SdkAutoloaderTest.php @@ -163,6 +163,14 @@ public function test_autoload_from_config_file(): void $this->assertNotInstanceOf(NoopTracerProvider::class, Globals::tracerProvider()); } + public function test_autoload_with_no_instrumentation_config_does_not_fail(): void + { + $this->setEnvironmentVariable(Variables::OTEL_PHP_AUTOLOAD_ENABLED, 'true'); + $this->setEnvironmentVariable(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE, __DIR__ . '/fixtures/otel-sdk.yaml'); + + $this->assertTrue(SdkAutoloader::autoload()); + } + /** * Tests the scenario where the SDK is created from config file, but a custom component * uses composer's autoload->files to add its own initializer From 82d62db6983e2ab224f3a0966c4125b28b7a6e22 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 14 May 2024 10:09:12 +1000 Subject: [PATCH 26/43] fixing phan ref, update doc --- .phan/config.php | 1 + src/SDK/SdkAutoloader.php | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.phan/config.php b/.phan/config.php index 9a20886d7..0baafa345 100644 --- a/.phan/config.php +++ b/.phan/config.php @@ -373,6 +373,7 @@ 'vendor/composer', 'vendor/grpc/grpc/src/lib', 'vendor/guzzlehttp', + 'vendor/tbachert/spi/src', 'vendor/psr', 'vendor/php-http', 'vendor/phpunit/phpunit/src', diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index e5e07028f..84dbdceca 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -103,9 +103,7 @@ private static function fileBasedInitializer(Configurator $configurator): Config } /** - * Register any auto-instrumentation configured through SPI - * - * @phan-suppress PhanUndeclaredClassMethod + * Register all {@link Instrumentation} configured through SPI */ private static function registerInstrumentations(): void { From 0e5ffac82013ada0224e8d9c9c5987f489c5d294 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 14 May 2024 10:18:16 +1000 Subject: [PATCH 27/43] remove phan suppress --- src/SDK/SdkAutoloader.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index 84dbdceca..1a57817cc 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -123,9 +123,6 @@ private static function registerInstrumentations(): void } } - /** - * @phan-suppress PhanUndeclaredClassMethod - */ private static function getHookManager(): HookManager { /** @var HookManager $hookManager */ From de4262b98c66d03ff763c2f06f005d9ad7f05f22 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 14 May 2024 10:28:21 +1000 Subject: [PATCH 28/43] fix example --- .../instrumentation/configure_instrumentation.php | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/examples/instrumentation/configure_instrumentation.php b/examples/instrumentation/configure_instrumentation.php index f614b7669..4d660c4bd 100644 --- a/examples/instrumentation/configure_instrumentation.php +++ b/examples/instrumentation/configure_instrumentation.php @@ -20,21 +20,12 @@ require __DIR__ . '/../../vendor/autoload.php'; -$sdk = Configuration::parseFile(__DIR__ . '/otel-sdk.yaml')->create(new Context())->setAutoShutdown(true)->build(); +Configuration::parseFile(__DIR__ . '/otel-sdk.yaml')->create(new Context())->setAutoShutdown(true)->buildAndRegisterGlobal(); $configuration = \OpenTelemetry\Config\SDK\Instrumentation::parseFile(__DIR__ . '/otel-instrumentation.yaml')->create(); - $hookManager = new ExtensionHookManager(); -$storage = \OpenTelemetry\Context\Context::storage(); -$context = new InstrumentationContext($sdk->getTracerProvider(), $sdk->getMeterProvider()); foreach (ServiceLoader::load(Instrumentation::class) as $instrumentation) { - $instrumentation->register($hookManager, $context, $configuration, $storage); + $instrumentation->register($hookManager, $configuration); } -$scope = $storage->attach($hookManager->enable($storage->current())); - -try { - echo (new Example())->test(), PHP_EOL; -} finally { - $scope->detach(); -} +echo (new Example())->test(), PHP_EOL; From be50116f9a6e15ee619c647c8f503c00d4745b04 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Wed, 15 May 2024 09:37:56 +1000 Subject: [PATCH 29/43] bump SPI to 0.2.1 --- composer.json | 2 +- examples/instrumentation/configure_instrumentation.php | 1 - src/Config/SDK/composer.json | 2 +- src/SDK/composer.json | 3 ++- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index 0e8745588..7401cb1ac 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,7 @@ "symfony/config": "^5.4 || ^6.4 || ^7.0", "symfony/polyfill-mbstring": "^1.23", "symfony/polyfill-php82": "^1.26", - "tbachert/spi": "^0.2" + "tbachert/spi": ">= 0.2.1" }, "config": { "sort-packages": true, diff --git a/examples/instrumentation/configure_instrumentation.php b/examples/instrumentation/configure_instrumentation.php index 4d660c4bd..65e227078 100644 --- a/examples/instrumentation/configure_instrumentation.php +++ b/examples/instrumentation/configure_instrumentation.php @@ -5,7 +5,6 @@ namespace _; use Nevay\SPI\ServiceLoader; -use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Context as InstrumentationContext; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ExtensionHookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation; use OpenTelemetry\Config\SDK\Configuration; diff --git a/src/Config/SDK/composer.json b/src/Config/SDK/composer.json index 75285bb65..4a8deb95a 100644 --- a/src/Config/SDK/composer.json +++ b/src/Config/SDK/composer.json @@ -21,7 +21,7 @@ "open-telemetry/context": "^1.0", "open-telemetry/sdk": "^1.0", "symfony/config": "^5.4 || ^6.4 || ^7.0", - "tbachert/spi": "^0.2" + "tbachert/spi": ">= 0.2.1" }, "autoload": { "psr-4": { diff --git a/src/SDK/composer.json b/src/SDK/composer.json index b84b94aa5..0d6d893ee 100644 --- a/src/SDK/composer.json +++ b/src/SDK/composer.json @@ -29,7 +29,8 @@ "psr/http-message": "^1.0.1|^2.0", "psr/log": "^1.1|^2.0|^3.0", "symfony/polyfill-mbstring": "^1.23", - "symfony/polyfill-php82": "^1.26" + "symfony/polyfill-php82": "^1.26", + "tbachert/spi": ">= 0.2.1" }, "autoload": { "psr-4": { From 7acacb6fb3de17a2000bbb2239c712f19d4bd617 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Fri, 17 May 2024 15:56:14 +1000 Subject: [PATCH 30/43] adding late-binding tracer+provider per review from Nevay, this will allow instrumentations to get things from Globals as late as possible --- examples/src/ExampleInstrumentation.php | 3 +- .../AutoInstrumentation/Instrumentation.php | 2 +- src/API/Trace/LateBindingTracerProvider.php | 6 ++ src/SDK/SdkAutoloader.php | 47 ++++++++++-- .../ExtensionHookManagerTest.php | 17 ++++- .../LateBindingProviderTest.php | 75 +++++++++++++++++++ tests/Unit/SDK/SdkAutoloaderTest.php | 1 + 7 files changed, 140 insertions(+), 11 deletions(-) create mode 100644 tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php diff --git a/examples/src/ExampleInstrumentation.php b/examples/src/ExampleInstrumentation.php index 0ae614141..66a36ed43 100644 --- a/examples/src/ExampleInstrumentation.php +++ b/examples/src/ExampleInstrumentation.php @@ -7,6 +7,7 @@ use Exception; use OpenTelemetry\API\Globals; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ConfigurationRegistry; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Context as InstrumentationContext; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation; use OpenTelemetry\API\Trace\Span; @@ -18,7 +19,7 @@ final class ExampleInstrumentation implements Instrumentation /** * @todo can we pass in just the config for _this_ instrumentation, rather than all? */ - public function register(HookManager $hookManager, ConfigurationRegistry $configuration): void + public function register(HookManager $hookManager, ConfigurationRegistry $configuration, InstrumentationContext $context): void { $config = $configuration->get(ExampleConfig::class) ?? throw new Exception('example instrumentation must be configured'); if (!$config->enabled) { diff --git a/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php b/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php index 2224e4fa0..7810a00c7 100644 --- a/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php +++ b/src/API/Instrumentation/AutoInstrumentation/Instrumentation.php @@ -6,5 +6,5 @@ interface Instrumentation { - public function register(HookManager $hookManager, ConfigurationRegistry $configuration): void; + public function register(HookManager $hookManager, ConfigurationRegistry $configuration, Context $context): void; } diff --git a/src/API/Trace/LateBindingTracerProvider.php b/src/API/Trace/LateBindingTracerProvider.php index 2a0ce483e..84552f427 100644 --- a/src/API/Trace/LateBindingTracerProvider.php +++ b/src/API/Trace/LateBindingTracerProvider.php @@ -6,6 +6,12 @@ use Closure; +/** + * Late binding providers are designed to be used by Instrumentation, while we do not have control over when all components (propagators, etc) + * which are registered through composer.autoload.files are actually loaded. It means that tracers etc are not fetched + * from Globals until the last possible instant (ie, when they try to create a span, get an instrument, etc). + * In the future, when everything uses SPI, this will be removed. + */ class LateBindingTracerProvider implements TracerProviderInterface { private ?TracerProviderInterface $tracerProvider = null; diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index 1a57817cc..4ae8c3519 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -7,11 +7,20 @@ use Nevay\SPI\ServiceLoader; use OpenTelemetry\API\Behavior\LogsMessagesTrait; use OpenTelemetry\API\Globals; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Context as InstrumentationContext; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\NoopHookManager; use OpenTelemetry\API\Instrumentation\Configurator; +use OpenTelemetry\API\Logs\LoggerProviderInterface; +use OpenTelemetry\API\Logs\NoopLoggerProvider; +use OpenTelemetry\API\Metrics\MeterProviderInterface; +use OpenTelemetry\API\Metrics\Noop\NoopMeterProvider; +use OpenTelemetry\API\Trace\LateBindingTracerProvider; +use OpenTelemetry\API\Trace\TracerProviderInterface; use OpenTelemetry\Config\SDK\Configuration as SdkConfiguration; -use OpenTelemetry\Config\SDK\Instrumentation; +use OpenTelemetry\Config\SDK\Instrumentation as SdkInstrumentation; +use OpenTelemetry\Context\Context; use OpenTelemetry\SDK\Common\Configuration\Configuration; use OpenTelemetry\SDK\Common\Configuration\Variables; use OpenTelemetry\SDK\Common\Util\ShutdownHandler; @@ -110,12 +119,17 @@ private static function registerInstrumentations(): void $file = Configuration::has(Variables::OTEL_PHP_INSTRUMENTATION_CONFIG_FILE) ? Configuration::getString(Variables::OTEL_PHP_INSTRUMENTATION_CONFIG_FILE) : []; - $configuration = Instrumentation::parseFile($file)->create(); + $configuration = SdkInstrumentation::parseFile($file)->create(); $hookManager = self::getHookManager(); - foreach (ServiceLoader::load(\OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation::class) as $instrumentation) { - /** @var \OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation $instrumentation */ + $tracerProvider = self::createLateBindingTracerProvider(); + $meterProvider = self::createLateBindingMeterProvider(); + $loggerProvider = self::createLateBindingLoggerProvider(); + + $context = new InstrumentationContext($tracerProvider, $meterProvider, $loggerProvider); + foreach (ServiceLoader::load(Instrumentation::class) as $instrumentation) { + /** @var Instrumentation $instrumentation */ try { - $instrumentation->register($hookManager, $configuration); + $instrumentation->register($hookManager, $configuration, $context); } catch (Throwable $t) { self::logError(sprintf('Unable to load instrumentation: %s', $instrumentation::class), ['exception' => $t]); } @@ -123,6 +137,29 @@ private static function registerInstrumentations(): void } } + private static function createLateBindingTracerProvider(): TracerProviderInterface + { + return new LateBindingTracerProvider(static function (): TracerProviderInterface { + $scope = Context::getRoot()->activate(); + + try { + return Globals::tracerProvider(); + } finally { + $scope->detach(); + } + }); + } + + private static function createLateBindingMeterProvider(): MeterProviderInterface + { + //@todo + return new NoopMeterProvider(); + } + private static function createLateBindingLoggerProvider(): LoggerProviderInterface + { + //@todo + return new NoopLoggerProvider(); + } private static function getHookManager(): HookManager { /** @var HookManager $hookManager */ diff --git a/tests/Unit/API/Instrumentation/AutoInstrumentation/ExtensionHookManagerTest.php b/tests/Unit/API/Instrumentation/AutoInstrumentation/ExtensionHookManagerTest.php index 1bd7c1741..50772ac1a 100644 --- a/tests/Unit/API/Instrumentation/AutoInstrumentation/ExtensionHookManagerTest.php +++ b/tests/Unit/API/Instrumentation/AutoInstrumentation/ExtensionHookManagerTest.php @@ -5,10 +5,13 @@ namespace Unit\API\Instrumentation\AutoInstrumentation; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ConfigurationRegistry; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Context as InstrumentationContext; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ExtensionHookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation; use OpenTelemetry\API\Instrumentation\Configurator; +use OpenTelemetry\API\Logs\LoggerProviderInterface; +use OpenTelemetry\API\Metrics\MeterProviderInterface; use OpenTelemetry\API\Trace\TracerProviderInterface; use OpenTelemetry\Context\Context; use OpenTelemetry\Context\ScopeInterface; @@ -21,6 +24,7 @@ class ExtensionHookManagerTest extends TestCase private ConfigurationRegistry $registry; private ScopeInterface $scope; private HookManager $hookManager; + private InstrumentationContext $context; public function setUp(): void { @@ -33,6 +37,11 @@ public function setUp(): void ->activate(); $this->registry = new ConfigurationRegistry(); $this->hookManager = new ExtensionHookManager(); + $this->context = new InstrumentationContext( + $tracerProvider, + $this->createMock(MeterProviderInterface::class), + $this->createMock(LoggerProviderInterface::class) + ); } public function tearDown(): void @@ -52,7 +61,7 @@ public function test(): int }, function (): int { return 99; }); - $instrumentation->register($this->hookManager, $this->registry); + $instrumentation->register($this->hookManager, $this->registry, $this->context); $returnVal = $target->test(); $this->assertSame(99, $returnVal); @@ -70,7 +79,7 @@ public function test(): int }, function (): int { $this->fail('post hook not expected to be called'); }); - $instrumentation->register($this->hookManager, $this->registry); + $instrumentation->register($this->hookManager, $this->registry, $this->context); $scope = $this->hookManager->disable(Context::getCurrent())->activate(); @@ -94,7 +103,7 @@ public function test(): int }, function (): int { return 123; }); - $instrumentation->register($this->hookManager, $this->registry); + $instrumentation->register($this->hookManager, $this->registry, $this->context); $this->assertSame(123, $target->test(), 'post hook function ran and modified return value'); $scope = $this->hookManager->disable(Context::getCurrent())->activate(); @@ -122,7 +131,7 @@ public function __construct( $this->post = $post; } - public function register(HookManager $hookManager, ConfigurationRegistry $configuration): void + public function register(HookManager $hookManager, ConfigurationRegistry $configuration, InstrumentationContext $context): void { $hookManager->hook($this->class, $this->method, $this->pre, $this->post); } diff --git a/tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php b/tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php new file mode 100644 index 000000000..4401346b3 --- /dev/null +++ b/tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php @@ -0,0 +1,75 @@ +tracerProvider->getTracer('test'); + } + }; + $this->setEnvironmentVariable(Variables::OTEL_PHP_AUTOLOAD_ENABLED, 'true'); + $called = false; + //the "real" tracer, which will be accessed through a late binding tracer + $tracerProvider = $this->createMock(TracerProviderInterface::class); + $tracer = $this->createMock(TracerInterface::class); + $tracerProvider->method('getTracer')->willReturnCallback(function () use (&$called, $tracer): TracerInterface { + $called = true; + + return $tracer; + }); + ServiceLoader::register(Instrumentation::class, $instrumentation::class); + //@todo reset? + $this->assertTrue(SdkAutoloader::autoload()); + //tracer initializer added _after_ autoloader has run and instrumentation registered + Globals::registerInitializer(function (Configurator $configurator) use ($tracerProvider): Configurator { + return $configurator->withTracerProvider($tracerProvider); + }); + + $this->assertFalse($called); + $tracer = $instrumentation->getTracer(); + $this->assertFalse($called); + $tracer->spanBuilder('test-span')->startSpan(); /** @phpstan-ignore-next-line */ + $this->assertTrue($called); + } +} diff --git a/tests/Unit/SDK/SdkAutoloaderTest.php b/tests/Unit/SDK/SdkAutoloaderTest.php index 100feb09c..2db1c30be 100644 --- a/tests/Unit/SDK/SdkAutoloaderTest.php +++ b/tests/Unit/SDK/SdkAutoloaderTest.php @@ -6,6 +6,7 @@ use OpenTelemetry\API\Behavior\Internal\Logging; use OpenTelemetry\API\Globals; +use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\API\Logs\NoopEventLoggerProvider; use OpenTelemetry\API\Logs\NoopLoggerProvider; use OpenTelemetry\API\Metrics\Noop\NoopMeterProvider; From 77653d429a7aff7e58d033968a3e18cb8b79f162 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Sat, 18 May 2024 18:38:16 +1000 Subject: [PATCH 31/43] adding late binding logger and meter providers --- .../configure_instrumentation.php | 6 +- src/API/Logs/LateBindingLogger.php | 23 +++++++ src/API/Logs/LateBindingLoggerProvider.php | 24 ++++++++ src/API/Metrics/LateBindingMeter.php | 61 +++++++++++++++++++ src/API/Metrics/LateBindingMeterProvider.php | 24 ++++++++ src/SDK/SdkAutoloader.php | 27 ++++++-- 6 files changed, 158 insertions(+), 7 deletions(-) create mode 100644 src/API/Logs/LateBindingLogger.php create mode 100644 src/API/Logs/LateBindingLoggerProvider.php create mode 100644 src/API/Metrics/LateBindingMeter.php create mode 100644 src/API/Metrics/LateBindingMeterProvider.php diff --git a/examples/instrumentation/configure_instrumentation.php b/examples/instrumentation/configure_instrumentation.php index 65e227078..0ad2e57dd 100644 --- a/examples/instrumentation/configure_instrumentation.php +++ b/examples/instrumentation/configure_instrumentation.php @@ -7,6 +7,9 @@ use Nevay\SPI\ServiceLoader; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ExtensionHookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation; +use OpenTelemetry\API\Logs\NoopLoggerProvider; +use OpenTelemetry\API\Metrics\Noop\NoopMeterProvider; +use OpenTelemetry\API\Trace\NoopTracerProvider; use OpenTelemetry\Config\SDK\Configuration; use OpenTelemetry\Config\SDK\Configuration\Context; use OpenTelemetry\Example\Example; @@ -22,9 +25,10 @@ Configuration::parseFile(__DIR__ . '/otel-sdk.yaml')->create(new Context())->setAutoShutdown(true)->buildAndRegisterGlobal(); $configuration = \OpenTelemetry\Config\SDK\Instrumentation::parseFile(__DIR__ . '/otel-instrumentation.yaml')->create(); $hookManager = new ExtensionHookManager(); +$context = new \OpenTelemetry\API\Instrumentation\AutoInstrumentation\Context(new NoopTracerProvider(), new NoopMeterProvider(), new NoopLoggerProvider()); foreach (ServiceLoader::load(Instrumentation::class) as $instrumentation) { - $instrumentation->register($hookManager, $configuration); + $instrumentation->register($hookManager, $configuration, $context); } echo (new Example())->test(), PHP_EOL; diff --git a/src/API/Logs/LateBindingLogger.php b/src/API/Logs/LateBindingLogger.php new file mode 100644 index 000000000..84f9ed81f --- /dev/null +++ b/src/API/Logs/LateBindingLogger.php @@ -0,0 +1,23 @@ +logger ??= ($this->factory)())->emit($logRecord); + } +} diff --git a/src/API/Logs/LateBindingLoggerProvider.php b/src/API/Logs/LateBindingLoggerProvider.php new file mode 100644 index 000000000..755325de1 --- /dev/null +++ b/src/API/Logs/LateBindingLoggerProvider.php @@ -0,0 +1,24 @@ +loggerProvider?->getLogger($name, $version, $schemaUrl, $attributes) + ?? new LateBindingLogger(fn (): LoggerInterface => ($this->loggerProvider ??= ($this->factory)())->getLogger($name, $version, $schemaUrl, $attributes)); + } +} diff --git a/src/API/Metrics/LateBindingMeter.php b/src/API/Metrics/LateBindingMeter.php new file mode 100644 index 000000000..b146bfbbb --- /dev/null +++ b/src/API/Metrics/LateBindingMeter.php @@ -0,0 +1,61 @@ +meter ??= ($this->factory)())->batchObserve($callback, $instrument, ...$instruments); + } + + public function createCounter(string $name, ?string $unit = null, ?string $description = null, array $advisory = []): CounterInterface + { + return ($this->meter ??= ($this->factory)())->createCounter($name, $unit, $description, $advisory); + } + + public function createObservableCounter(string $name, ?string $unit = null, ?string $description = null, callable|array $advisory = [], callable ...$callbacks): ObservableCounterInterface + { + return ($this->meter ??= ($this->factory)())->createObservableCounter($name, $unit, $description, $advisory, $callbacks); + } + + public function createHistogram(string $name, ?string $unit = null, ?string $description = null, array $advisory = []): HistogramInterface + { + return ($this->meter ??= ($this->factory)())->createHistogram($name, $unit, $description, $advisory); + } + + public function createGauge(string $name, ?string $unit = null, ?string $description = null, array $advisory = []): GaugeInterface + { + return ($this->meter ??= ($this->factory)())->createGauge($name, $unit, $description, $advisory); + } + + public function createObservableGauge(string $name, ?string $unit = null, ?string $description = null, callable|array $advisory = [], callable ...$callbacks): ObservableGaugeInterface + { + return ($this->meter ??= ($this->factory)())->createObservableGauge($name, $unit, $description, $advisory, $callbacks); + } + + public function createUpDownCounter(string $name, ?string $unit = null, ?string $description = null, array $advisory = []): UpDownCounterInterface + { + return ($this->meter ??= ($this->factory)())->createUpDownCounter($name, $unit, $description, $advisory); + } + + public function createObservableUpDownCounter(string $name, ?string $unit = null, ?string $description = null, callable|array $advisory = [], callable ...$callbacks): ObservableUpDownCounterInterface + { + return ($this->meter ??= ($this->factory)())->createObservableUpDownCounter($name, $unit, $description, $advisory, $callbacks); + } +} diff --git a/src/API/Metrics/LateBindingMeterProvider.php b/src/API/Metrics/LateBindingMeterProvider.php new file mode 100644 index 000000000..39238f03f --- /dev/null +++ b/src/API/Metrics/LateBindingMeterProvider.php @@ -0,0 +1,24 @@ +meterProvider?->getMeter($name, $version, $schemaUrl, $attributes) + ?? new LateBindingMeter(fn (): MeterInterface => ($this->meterProvider ??= ($this->factory)())->getMeter($name, $version, $schemaUrl, $attributes)); + } +} diff --git a/src/SDK/SdkAutoloader.php b/src/SDK/SdkAutoloader.php index 4ae8c3519..544232643 100644 --- a/src/SDK/SdkAutoloader.php +++ b/src/SDK/SdkAutoloader.php @@ -12,10 +12,10 @@ use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\NoopHookManager; use OpenTelemetry\API\Instrumentation\Configurator; +use OpenTelemetry\API\Logs\LateBindingLoggerProvider; use OpenTelemetry\API\Logs\LoggerProviderInterface; -use OpenTelemetry\API\Logs\NoopLoggerProvider; +use OpenTelemetry\API\Metrics\LateBindingMeterProvider; use OpenTelemetry\API\Metrics\MeterProviderInterface; -use OpenTelemetry\API\Metrics\Noop\NoopMeterProvider; use OpenTelemetry\API\Trace\LateBindingTracerProvider; use OpenTelemetry\API\Trace\TracerProviderInterface; use OpenTelemetry\Config\SDK\Configuration as SdkConfiguration; @@ -152,14 +152,29 @@ private static function createLateBindingTracerProvider(): TracerProviderInterfa private static function createLateBindingMeterProvider(): MeterProviderInterface { - //@todo - return new NoopMeterProvider(); + return new LateBindingMeterProvider(static function (): MeterProviderInterface { + $scope = Context::getRoot()->activate(); + + try { + return Globals::meterProvider(); + } finally { + $scope->detach(); + } + }); } private static function createLateBindingLoggerProvider(): LoggerProviderInterface { - //@todo - return new NoopLoggerProvider(); + return new LateBindingLoggerProvider(static function (): LoggerProviderInterface { + $scope = Context::getRoot()->activate(); + + try { + return Globals::loggerProvider(); + } finally { + $scope->detach(); + } + }); } + private static function getHookManager(): HookManager { /** @var HookManager $hookManager */ From c0cd4ed5f8c9d3fc2b41004b0e4661bbce338486 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Sat, 18 May 2024 19:49:31 +1000 Subject: [PATCH 32/43] more late binding test coverage --- .../LateBindingProviderTest.php | 82 ++++++++++++++++--- 1 file changed, 70 insertions(+), 12 deletions(-) diff --git a/tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php b/tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php index 4401346b3..e3b219095 100644 --- a/tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php +++ b/tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php @@ -12,6 +12,15 @@ use OpenTelemetry\API\Instrumentation\AutoInstrumentation\HookManager; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\Instrumentation; use OpenTelemetry\API\Instrumentation\Configurator; +use OpenTelemetry\API\Logs\LateBindingLogger; +use OpenTelemetry\API\Logs\LateBindingLoggerProvider; +use OpenTelemetry\API\Logs\LoggerInterface; +use OpenTelemetry\API\Logs\LoggerProviderInterface; +use OpenTelemetry\API\Logs\LogRecord; +use OpenTelemetry\API\Metrics\LateBindingMeter; +use OpenTelemetry\API\Metrics\LateBindingMeterProvider; +use OpenTelemetry\API\Metrics\MeterInterface; +use OpenTelemetry\API\Metrics\MeterProviderInterface; use OpenTelemetry\API\Trace\LateBindingTracer; use OpenTelemetry\API\Trace\LateBindingTracerProvider; use OpenTelemetry\API\Trace\TracerInterface; @@ -20,10 +29,18 @@ use OpenTelemetry\SDK\SdkAutoloader; use OpenTelemetry\Tests\TestState; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\TestCase; +#[CoversClass(LateBindingLoggerProvider::class)] +#[CoversClass(LateBindingLogger::class)] +#[CoversClass(LateBindingMeterProvider::class)] +#[CoversClass(LateBindingMeter::class)] #[CoversClass(LateBindingTracerProvider::class)] #[CoversClass(LateBindingTracer::class)] +#[CoversMethod(SdkAutoloader::class, 'createLateBindingLoggerProvider')] +#[CoversMethod(SdkAutoloader::class, 'createLateBindingMeterProvider')] +#[CoversMethod(SdkAutoloader::class, 'createLateBindingTracerProvider')] class LateBindingProviderTest extends TestCase { use TestState; @@ -33,7 +50,7 @@ public function setUp(): void Logging::disable(); } - public function test_late_binding_tracer(): void + public function test_late_binding_providers(): void { $instrumentation = new class() implements Instrumentation { private static ?Context $context; @@ -47,29 +64,70 @@ public function getTracer(): TracerInterface return self::$context->tracerProvider->getTracer('test'); } + public function getMeter(): MeterInterface + { + assert(self::$context !== null); + + return self::$context->meterProvider->getMeter('test'); + } + public function getLogger(): LoggerInterface + { + assert(self::$context !== null); + + return self::$context->loggerProvider->getLogger('test'); + } }; $this->setEnvironmentVariable(Variables::OTEL_PHP_AUTOLOAD_ENABLED, 'true'); - $called = false; - //the "real" tracer, which will be accessed through a late binding tracer + $tracer_called = false; + $logger_called = false; + $meter_called = false; + //the "real" tracer+meter+logger, which will be accessed through late binding providers $tracerProvider = $this->createMock(TracerProviderInterface::class); - $tracer = $this->createMock(TracerInterface::class); - $tracerProvider->method('getTracer')->willReturnCallback(function () use (&$called, $tracer): TracerInterface { - $called = true; + $tracerProvider->method('getTracer')->willReturnCallback(function () use (&$tracer_called): TracerInterface { + $tracer_called = true; + + return $this->createMock(TracerInterface::class); + }); + $meterProvider = $this->createMock(MeterProviderInterface::class); + $meterProvider->method('getMeter')->willReturnCallback(function () use (&$meter_called): MeterInterface { + $meter_called = true; - return $tracer; + return $this->createMock(MeterInterface::class); + }); + $loggerProvider = $this->createMock(LoggerProviderInterface::class); + $loggerProvider->method('getLogger')->willReturnCallback(function () use (&$logger_called): LoggerInterface { + $logger_called = true; + + return $this->createMock(LoggerInterface::class); }); ServiceLoader::register(Instrumentation::class, $instrumentation::class); //@todo reset? $this->assertTrue(SdkAutoloader::autoload()); //tracer initializer added _after_ autoloader has run and instrumentation registered - Globals::registerInitializer(function (Configurator $configurator) use ($tracerProvider): Configurator { - return $configurator->withTracerProvider($tracerProvider); + Globals::registerInitializer(function (Configurator $configurator) use ($tracerProvider, $loggerProvider, $meterProvider): Configurator { + return $configurator + ->withTracerProvider($tracerProvider) + ->withMeterProvider($meterProvider) + ->withLoggerProvider($loggerProvider) + ; }); - $this->assertFalse($called); + $this->assertFalse($tracer_called); $tracer = $instrumentation->getTracer(); - $this->assertFalse($called); + $this->assertFalse($tracer_called); $tracer->spanBuilder('test-span')->startSpan(); /** @phpstan-ignore-next-line */ - $this->assertTrue($called); + $this->assertTrue($tracer_called); + + $this->assertFalse($meter_called); + $meter = $instrumentation->getMeter(); + $this->assertFalse($meter_called); + $meter->createCounter('cnt'); /** @phpstan-ignore-next-line */ + $this->assertTrue($meter_called); + + $this->assertFalse($logger_called); + $logger = $instrumentation->getLogger(); + $this->assertFalse($logger_called); + $logger->emit(new LogRecord()); /** @phpstan-ignore-next-line */ + $this->assertTrue($logger_called); } } From fe5fe3e7c4793297059d668f2517e7b9e62e1be2 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Sat, 18 May 2024 19:51:37 +1000 Subject: [PATCH 33/43] tidy --- .../LateBindingProviderTest.php | 43 +++++++++---------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php b/tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php index e3b219095..e29d7b047 100644 --- a/tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php +++ b/tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php @@ -78,32 +78,31 @@ public function getLogger(): LoggerInterface } }; $this->setEnvironmentVariable(Variables::OTEL_PHP_AUTOLOAD_ENABLED, 'true'); - $tracer_called = false; - $logger_called = false; - $meter_called = false; - //the "real" tracer+meter+logger, which will be accessed through late binding providers + $tracer_accessed = false; + $logger_accessed = false; + $meter_accessed = false; + $tracerProvider = $this->createMock(TracerProviderInterface::class); - $tracerProvider->method('getTracer')->willReturnCallback(function () use (&$tracer_called): TracerInterface { - $tracer_called = true; + $tracerProvider->method('getTracer')->willReturnCallback(function () use (&$tracer_accessed): TracerInterface { + $tracer_accessed = true; return $this->createMock(TracerInterface::class); }); $meterProvider = $this->createMock(MeterProviderInterface::class); - $meterProvider->method('getMeter')->willReturnCallback(function () use (&$meter_called): MeterInterface { - $meter_called = true; + $meterProvider->method('getMeter')->willReturnCallback(function () use (&$meter_accessed): MeterInterface { + $meter_accessed = true; return $this->createMock(MeterInterface::class); }); $loggerProvider = $this->createMock(LoggerProviderInterface::class); - $loggerProvider->method('getLogger')->willReturnCallback(function () use (&$logger_called): LoggerInterface { - $logger_called = true; + $loggerProvider->method('getLogger')->willReturnCallback(function () use (&$logger_accessed): LoggerInterface { + $logger_accessed = true; return $this->createMock(LoggerInterface::class); }); ServiceLoader::register(Instrumentation::class, $instrumentation::class); - //@todo reset? $this->assertTrue(SdkAutoloader::autoload()); - //tracer initializer added _after_ autoloader has run and instrumentation registered + //initializer added _after_ autoloader has run and instrumentation registered Globals::registerInitializer(function (Configurator $configurator) use ($tracerProvider, $loggerProvider, $meterProvider): Configurator { return $configurator ->withTracerProvider($tracerProvider) @@ -112,22 +111,22 @@ public function getLogger(): LoggerInterface ; }); - $this->assertFalse($tracer_called); + $this->assertFalse($tracer_accessed); $tracer = $instrumentation->getTracer(); - $this->assertFalse($tracer_called); - $tracer->spanBuilder('test-span')->startSpan(); /** @phpstan-ignore-next-line */ - $this->assertTrue($tracer_called); + $this->assertFalse($tracer_accessed); + $tracer->spanBuilder('test-span'); /** @phpstan-ignore-next-line */ + $this->assertTrue($tracer_accessed); - $this->assertFalse($meter_called); + $this->assertFalse($meter_accessed); $meter = $instrumentation->getMeter(); - $this->assertFalse($meter_called); + $this->assertFalse($meter_accessed); $meter->createCounter('cnt'); /** @phpstan-ignore-next-line */ - $this->assertTrue($meter_called); + $this->assertTrue($meter_accessed); - $this->assertFalse($logger_called); + $this->assertFalse($logger_accessed); $logger = $instrumentation->getLogger(); - $this->assertFalse($logger_called); + $this->assertFalse($logger_accessed); $logger->emit(new LogRecord()); /** @phpstan-ignore-next-line */ - $this->assertTrue($logger_called); + $this->assertTrue($logger_accessed); } } From 1c0e1b5a9a8b6641115f682b009b05c32a1e71e9 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 21 May 2024 14:57:00 +1000 Subject: [PATCH 34/43] dont use CoversMethod yet not available in phpunit 10, so comment out and leave a todo --- .../AutoInstrumentation/LateBindingProviderTest.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php b/tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php index e29d7b047..d8a860114 100644 --- a/tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php +++ b/tests/Unit/API/Instrumentation/AutoInstrumentation/LateBindingProviderTest.php @@ -29,7 +29,6 @@ use OpenTelemetry\SDK\SdkAutoloader; use OpenTelemetry\Tests\TestState; use PHPUnit\Framework\Attributes\CoversClass; -use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\TestCase; #[CoversClass(LateBindingLoggerProvider::class)] @@ -38,9 +37,11 @@ #[CoversClass(LateBindingMeter::class)] #[CoversClass(LateBindingTracerProvider::class)] #[CoversClass(LateBindingTracer::class)] -#[CoversMethod(SdkAutoloader::class, 'createLateBindingLoggerProvider')] -#[CoversMethod(SdkAutoloader::class, 'createLateBindingMeterProvider')] -#[CoversMethod(SdkAutoloader::class, 'createLateBindingTracerProvider')] +// @todo phpunit 11 (8.2+) only, replace CoversClass(SdkAutoloader::class) +#[CoversClass(SdkAutoloader::class)] +//#[CoversMethod(SdkAutoloader::class, 'createLateBindingLoggerProvider')] +//#[CoversMethod(SdkAutoloader::class, 'createLateBindingMeterProvider')] +//#[CoversMethod(SdkAutoloader::class, 'createLateBindingTracerProvider')] class LateBindingProviderTest extends TestCase { use TestState; From 4b6deb86e2912aba5b3449a6c44176c61501b018 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 20 Jun 2024 14:49:06 +1000 Subject: [PATCH 35/43] kitchen sink, remove unused var --- .../instrumentation/otel-instrumentation.yaml | 3 - src/SDK/Common/Configuration/Defaults.php | 1 - .../Config/configurations/kitchen-sink.yaml | 104 ++++++++++++++++++ 3 files changed, 104 insertions(+), 4 deletions(-) delete mode 100644 examples/instrumentation/otel-instrumentation.yaml diff --git a/examples/instrumentation/otel-instrumentation.yaml b/examples/instrumentation/otel-instrumentation.yaml deleted file mode 100644 index 908e10659..000000000 --- a/examples/instrumentation/otel-instrumentation.yaml +++ /dev/null @@ -1,3 +0,0 @@ -config: - - example_instrumentation: - span_name: ${EXAMPLE_INSTRUMENTATION_SPAN_NAME:-example span} diff --git a/src/SDK/Common/Configuration/Defaults.php b/src/SDK/Common/Configuration/Defaults.php index a58d927b5..52709a15c 100644 --- a/src/SDK/Common/Configuration/Defaults.php +++ b/src/SDK/Common/Configuration/Defaults.php @@ -120,5 +120,4 @@ interface Defaults public const OTEL_PHP_LOGS_PROCESSOR = 'batch'; public const OTEL_PHP_LOG_DESTINATION = 'default'; public const OTEL_EXPERIMENTAL_CONFIG_FILE = 'sdk-config.yaml'; - public const OTEL_PHP_INSTRUMENTATION_CONFIG_FILE = 'instrumentation-config.yaml'; } diff --git a/tests/Integration/Config/configurations/kitchen-sink.yaml b/tests/Integration/Config/configurations/kitchen-sink.yaml index 2c63f16d8..d0bdb7f2f 100644 --- a/tests/Integration/Config/configurations/kitchen-sink.yaml +++ b/tests/Integration/Config/configurations/kitchen-sink.yaml @@ -350,3 +350,107 @@ resource: service.name: !!str "unknown_service" # Configure the resource schema URL. schema_url: https://opentelemetry.io/schemas/1.25.0 + +# Configure instrumentation. +instrumentation: + # Configure general SemConv options that may apply to multiple languages and instrumentations. + # + # Instrumenation may merge general config options with the language specific configuration at .instrumentation.. + general: + # Configure instrumentations following the peer semantic conventions. + # + # See peer semantic conventions: https://opentelemetry.io/docs/specs/semconv/attributes-registry/peer/ + peer: + # Configure the service mapping for instrumentations following peer.service semantic conventions. + # + # Each entry is a key value pair where "peer" defines the IP address and "service" defines the corresponding logical name of the service. + # + # See peer.service semantic conventions: https://opentelemetry.io/docs/specs/semconv/general/attributes/#general-remote-service-attributes + service_mapping: + - peer: 1.2.3.4 + service: FooService + - peer: 2.3.4.5 + service: BarService + # Configure instrumentations following the http semantic conventions. + # + # See http semantic conventions: https://opentelemetry.io/docs/specs/semconv/http/ + http: + # Configure instrumentations following the http client semantic conventions. + client: + # Configure headers to capture for outbound http requests. + request_captured_headers: + - Content-Type + - Accept + # Configure headers to capture for outbound http responses. + response_captured_headers: + - Content-Type + - Content-Encoding + # Configure instrumentations following the http server semantic conventions. + server: + # Configure headers to capture for inbound http requests. + request_captured_headers: + - Content-Type + - Accept + # Configure headers to capture for outbound http responses. + response_captured_headers: + - Content-Type + - Content-Encoding + # Configure language-specific instrumentation libraries. + # + # Keys may refer to instrumentation libraries or collections of related configuration. Because there is no central schema defining the keys or their contents, instrumentation must carefully document their schema and avoid key collisions with other instrumentations. + # + # Configure C++ language-specific instrumentation libraries. + cpp: + # Configure the instrumentation corresponding to key "example". + example: + property: "value" + # Configure .NET language-specific instrumentation libraries. + dotnet: + # Configure the instrumentation corresponding to key "example". + example: + property: "value" + # Configure Erlang language-specific instrumentation libraries. + erlang: + # Configure the instrumentation corresponding to key "example". + example: + property: "value" + # Configure Go language-specific instrumentation libraries. + go: + # Configure the instrumentation corresponding to key "example". + example: + property: "value" + # Configure Java language-specific instrumentation libraries. + java: + # Configure the instrumentation corresponding to key "example". + example: + property: "value" + # Configure JavaScript language-specific instrumentation libraries. + js: + # Configure the instrumentation corresponding to key "example". + example: + property: "value" + # Configure PHP language-specific instrumentation libraries. + php: + # Configure the instrumentation corresponding to key "example". + example: + property: "value" + # Configure Python language-specific instrumentation libraries. + python: + # Configure the instrumentation corresponding to key "example". + example: + property: "value" + # Configure Ruby language-specific instrumentation libraries. + ruby: + # Configure the instrumentation corresponding to key "example". + example: + property: "value" + # Configure Rust language-specific instrumentation libraries. + rust: + # Configure the instrumentation corresponding to key "example". + example: + property: "value" + # Configure Swift language-specific instrumentation libraries. + swift: + # Configure the instrumentation corresponding to key "example". + example: + property: "value" \ No newline at end of file From 610832bacda04a6a0a988014e6a7d6d1d56cb521 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Mon, 24 Jun 2024 20:01:24 +1000 Subject: [PATCH 36/43] instrumentation config as a map, per config file spec --- .../InstrumentationConfigurationRegistry.php | 4 +- .../ComponentProvider/OpenTelemetrySdk.php | 6 +-- .../SDK/Configuration/ComponentProvider.php | 1 + .../ComponentProviderRegistry.php | 23 ++++++++- .../Internal/ComponentProviderRegistry.php | 50 +++++++++++++------ .../ExampleSdk/OpenTelemetryConfiguration.php | 6 +-- .../configurations/kitchen-sink.yaml | 30 ++++++++++- tests/Unit/SDK/SdkAutoloaderTest.php | 14 +++++- tests/Unit/SDK/fixtures/otel-sdk.yaml | 26 +++++++++- 9 files changed, 132 insertions(+), 28 deletions(-) diff --git a/src/Config/SDK/ComponentProvider/InstrumentationConfigurationRegistry.php b/src/Config/SDK/ComponentProvider/InstrumentationConfigurationRegistry.php index b49c36fee..dc9c3ba5d 100644 --- a/src/Config/SDK/ComponentProvider/InstrumentationConfigurationRegistry.php +++ b/src/Config/SDK/ComponentProvider/InstrumentationConfigurationRegistry.php @@ -45,10 +45,12 @@ public function getConfig(ComponentProviderRegistry $registry): ArrayNodeDefinit ->ignoreExtraKeys() ->children() ->arrayNode('instrumentation') + ->ignoreExtraKeys() ->children() ->append($registry->componentList('php', InstrumentationConfiguration::class)) - ->variableNode('general')->end() + ->variableNode('general')->end() //@todo ->end() + ->end() ->end() ; diff --git a/src/Config/SDK/ComponentProvider/OpenTelemetrySdk.php b/src/Config/SDK/ComponentProvider/OpenTelemetrySdk.php index aaaa9a902..3cd1c1b44 100644 --- a/src/Config/SDK/ComponentProvider/OpenTelemetrySdk.php +++ b/src/Config/SDK/ComponentProvider/OpenTelemetrySdk.php @@ -323,7 +323,7 @@ private function getTracerProviderConfig(ComponentProviderRegistry $registry): A ->end() ->end() ->append($registry->component('sampler', SamplerInterface::class)) - ->append($registry->componentList('processors', SpanProcessorInterface::class)) + ->append($registry->componentArrayList('processors', SpanProcessorInterface::class)) ->end() ; @@ -374,7 +374,7 @@ private function getMeterProviderConfig(ComponentProviderRegistry $registry): Ar ->end() ->end() ->end() - ->append($registry->componentList('readers', MetricReaderInterface::class)) + ->append($registry->componentArrayList('readers', MetricReaderInterface::class)) ->end() ; @@ -394,7 +394,7 @@ private function getLoggerProviderConfig(ComponentProviderRegistry $registry): A ->integerNode('attribute_count_limit')->min(0)->defaultNull()->end() ->end() ->end() - ->append($registry->componentList('processors', LogRecordProcessorInterface::class)) + ->append($registry->componentArrayList('processors', LogRecordProcessorInterface::class)) ->end() ; diff --git a/src/Config/SDK/Configuration/ComponentProvider.php b/src/Config/SDK/Configuration/ComponentProvider.php index 18c605d83..86e079d73 100644 --- a/src/Config/SDK/Configuration/ComponentProvider.php +++ b/src/Config/SDK/Configuration/ComponentProvider.php @@ -33,6 +33,7 @@ public function createPlugin(array $properties, Context $context): mixed; * * @see ComponentProviderRegistry::component() * @see ComponentProviderRegistry::componentList() + * @see ComponentProviderRegistry::componentArrayList() * @see ComponentProviderRegistry::componentNames() */ public function getConfig(ComponentProviderRegistry $registry): ArrayNodeDefinition; diff --git a/src/Config/SDK/Configuration/ComponentProviderRegistry.php b/src/Config/SDK/Configuration/ComponentProviderRegistry.php index c1d1f3f30..363ca638a 100644 --- a/src/Config/SDK/Configuration/ComponentProviderRegistry.php +++ b/src/Config/SDK/Configuration/ComponentProviderRegistry.php @@ -37,6 +37,26 @@ public function component(string $name, string $type): NodeDefinition; * * ``` * $name: + * provider1: + * property: value + * anotherProperty: value + * provider2: + * property: value + * anotherProperty: value + * ``` + * + * @param string $name name of configuration node + * @param string $type type of the component plugin + */ + public function componentList(string $name, string $type): ArrayNodeDefinition; + + /** + * Creates a node to specify a list of component plugins represented as an array. + * + * `$name: list>` + * + * ``` + * $name: * - provider1: * property: value * anotherProperty: value @@ -48,8 +68,7 @@ public function component(string $name, string $type): NodeDefinition; * @param string $name name of configuration node * @param string $type type of the component plugin */ - public function componentList(string $name, string $type): ArrayNodeDefinition; - + public function componentArrayList(string $name, string $type): ArrayNodeDefinition; /** * Creates a node to specify a list of component plugin names. * diff --git a/src/Config/SDK/Configuration/Internal/ComponentProviderRegistry.php b/src/Config/SDK/Configuration/Internal/ComponentProviderRegistry.php index 472716636..266319869 100644 --- a/src/Config/SDK/Configuration/Internal/ComponentProviderRegistry.php +++ b/src/Config/SDK/Configuration/Internal/ComponentProviderRegistry.php @@ -69,6 +69,15 @@ public function component(string $name, string $type): NodeDefinition } public function componentList(string $name, string $type): ArrayNodeDefinition + { + //@todo clobbers + $node = new ArrayNodeDefinition($name); + $this->applyToArrayNode($node, $type, true); + + return $node; + } + + public function componentArrayList(string $name, string $type): ArrayNodeDefinition { $node = new ArrayNodeDefinition($name); $this->applyToArrayNode($node->arrayPrototype(), $type); @@ -107,7 +116,7 @@ public function componentNames(string $name, string $type): ArrayNodeDefinition return $node; } - private function applyToArrayNode(ArrayNodeDefinition $node, string $type): void + private function applyToArrayNode(ArrayNodeDefinition $node, string $type, bool $isArray = false): void { $node->info(sprintf('Component "%s"', $type)); $node->performNoDeepMerging(); @@ -122,21 +131,34 @@ private function applyToArrayNode(ArrayNodeDefinition $node, string $type): void } } - $node->validate()->always(function (array $value) use ($type): ComponentPlugin { - if (count($value) !== 1) { - throw new InvalidArgumentException(sprintf( - 'Component "%s" must have exactly one element defined, got %s', - $type, - implode(', ', array_map(json_encode(...), array_keys($value)) ?: ['none']) - )); - } + if ($isArray) { + $node->validate()->always(function (array $value) use ($type): array { + $validated = []; + foreach ($value as $name => $v) { + $provider = $this->providers[$type][$name]; + $this->resources?->addClassResource($provider); + $validated[] = new ComponentPlugin($v, $this->providers[$type][$name]); + } + + return $validated; + }); + } else { + $node->validate()->always(function (array $value) use ($type): ComponentPlugin { + if (count($value) !== 1) { + throw new InvalidArgumentException(sprintf( + 'Component "%s" must have exactly one element defined, got %s', + $type, + implode(', ', array_map(json_encode(...), array_keys($value)) ?: ['none']) + )); + } - $name = array_key_first($value); - $provider = $this->providers[$type][$name]; - $this->resources?->addClassResource($provider); + $name = array_key_first($value); + $provider = $this->providers[$type][$name]; + $this->resources?->addClassResource($provider); - return new ComponentPlugin($value[$name], $this->providers[$type][$name]); - }); + return new ComponentPlugin($value[$name], $this->providers[$type][$name]); + }); + } } /** diff --git a/tests/Unit/Config/SDK/Configuration/ExampleSdk/OpenTelemetryConfiguration.php b/tests/Unit/Config/SDK/Configuration/ExampleSdk/OpenTelemetryConfiguration.php index 489f7f5b1..3731ab722 100644 --- a/tests/Unit/Config/SDK/Configuration/ExampleSdk/OpenTelemetryConfiguration.php +++ b/tests/Unit/Config/SDK/Configuration/ExampleSdk/OpenTelemetryConfiguration.php @@ -151,7 +151,7 @@ private function getTracerProviderConfig(ComponentProviderRegistry $registry): A ->end() ->end() ->append($registry->component('sampler', Sampler::class)) - ->append($registry->componentList('processors', SpanProcessor::class)) + ->append($registry->componentArrayList('processors', SpanProcessor::class)) ->end() ; @@ -203,7 +203,7 @@ private function getMeterProviderConfig(ComponentProviderRegistry $registry): Ar ->end() ->end() ->end() - ->append($registry->componentList('readers', MetricReader::class)) + ->append($registry->componentArrayList('readers', MetricReader::class)) ->end() ; @@ -223,7 +223,7 @@ private function getLoggerProviderConfig(ComponentProviderRegistry $registry): A ->integerNode('attribute_count_limit')->min(0)->defaultNull()->end() ->end() ->end() - ->append($registry->componentList('processors', LogRecordProcessor::class)) + ->append($registry->componentArrayList('processors', LogRecordProcessor::class)) ->end() ; diff --git a/tests/Unit/Config/SDK/Configuration/configurations/kitchen-sink.yaml b/tests/Unit/Config/SDK/Configuration/configurations/kitchen-sink.yaml index 8a8a5b800..41e9cfafc 100644 --- a/tests/Unit/Config/SDK/Configuration/configurations/kitchen-sink.yaml +++ b/tests/Unit/Config/SDK/Configuration/configurations/kitchen-sink.yaml @@ -385,4 +385,32 @@ resource: # Environment variable: OTEL_SERVICE_NAME service.name: !!str "unknown_service" # Configure the resource schema URL. - schema_url: https://opentelemetry.io/schemas/1.16.0 \ No newline at end of file + schema_url: https://opentelemetry.io/schemas/1.16.0 + +instrumentation: + php: + example_instrumentation: + span_name: ${EXAMPLE_INSTRUMENTATION_SPAN_NAME:-example span} + java: + general: + peer: + service_mapping: + - peer: 1.2.3.4 + service: FooService + - peer: 2.3.4.5 + service: BarService + http: + client: + request_captured_headers: + - Content-Type + - Accept + response_captured_headers: + - Content-Type + - Content-Encoding + server: + request_captured_headers: + - Content-Type + - Accept + response_captured_headers: + - Content-Type + - Content-Encoding diff --git a/tests/Unit/SDK/SdkAutoloaderTest.php b/tests/Unit/SDK/SdkAutoloaderTest.php index b87d4d84a..67585e195 100644 --- a/tests/Unit/SDK/SdkAutoloaderTest.php +++ b/tests/Unit/SDK/SdkAutoloaderTest.php @@ -4,9 +4,9 @@ namespace OpenTelemetry\Tests\Unit\SDK; -use OpenTelemetry\API\Behavior\Internal\Logging; use OpenTelemetry\API\Globals; use OpenTelemetry\API\Instrumentation\Configurator; +use OpenTelemetry\API\LoggerHolder; use OpenTelemetry\API\Logs\NoopEventLoggerProvider; use OpenTelemetry\API\Logs\NoopLoggerProvider; use OpenTelemetry\API\Metrics\Noop\NoopMeterProvider; @@ -18,16 +18,25 @@ use OpenTelemetry\Tests\TestState; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; +use Psr\Log\LogLevel; #[CoversClass(SdkAutoloader::class)] class SdkAutoloaderTest extends TestCase { use TestState; + /** + * @var LoggerInterface&MockObject + */ + private LoggerInterface $logger; + public function setUp(): void { - Logging::disable(); + $this->logger = $this->createMock(LoggerInterface::class); + LoggerHolder::set($this->logger); Globals::reset(); } @@ -146,6 +155,7 @@ public function test_enabled_with_excluded_url(): void public function test_autoload_from_config_file(): void { + $this->logger->expects($this->never())->method('log')->with($this->equalTo(LogLevel::ERROR)); $this->setEnvironmentVariable(Variables::OTEL_PHP_AUTOLOAD_ENABLED, 'true'); $this->setEnvironmentVariable(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE, __DIR__ . '/fixtures/otel-sdk.yaml'); diff --git a/tests/Unit/SDK/fixtures/otel-sdk.yaml b/tests/Unit/SDK/fixtures/otel-sdk.yaml index 025bb35e0..6cce68b3f 100644 --- a/tests/Unit/SDK/fixtures/otel-sdk.yaml +++ b/tests/Unit/SDK/fixtures/otel-sdk.yaml @@ -8,5 +8,27 @@ tracer_provider: instrumentation: php: - - example_instrumentation: - span_name: my-span \ No newline at end of file + example_instrumentation: + span_name: my-span + general: + peer: + service_mapping: + - peer: 1.2.3.4 + service: FooService + - peer: 2.3.4.5 + service: BarService + http: + client: + request_captured_headers: + - Content-Type + - Accept + response_captured_headers: + - Content-Type + - Content-Encoding + server: + request_captured_headers: + - Content-Type + - Accept + response_captured_headers: + - Content-Type + - Content-Encoding From a85982f8dd05177a9ff459749a278cb1c38e2a15 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 25 Jun 2024 12:01:09 +1000 Subject: [PATCH 37/43] adding general config --- examples/instrumentation/otel-sdk.yaml | 26 ++++++++- .../ConfigurationRegistry.php | 18 +++++++ .../GeneralInstrumentationConfiguration.php | 9 ++++ src/Config/SDK/Component/HttpConfig.php | 14 +++++ src/Config/SDK/Component/PeerConfig.php | 14 +++++ .../General/HttpConfigProvider.php | 54 +++++++++++++++++++ .../General/PeerConfigProvider.php | 42 +++++++++++++++ .../InstrumentationConfigurationRegistry.php | 7 ++- .../Internal/ComponentProviderRegistry.php | 6 +-- tests/Unit/SDK/SdkAutoloaderTest.php | 8 --- tests/Unit/SDK/fixtures/otel-sdk.yaml | 30 +++++------ 11 files changed, 199 insertions(+), 29 deletions(-) create mode 100644 src/API/Instrumentation/AutoInstrumentation/GeneralInstrumentationConfiguration.php create mode 100644 src/Config/SDK/Component/HttpConfig.php create mode 100644 src/Config/SDK/Component/PeerConfig.php create mode 100644 src/Config/SDK/ComponentProvider/Instrumentation/General/HttpConfigProvider.php create mode 100644 src/Config/SDK/ComponentProvider/Instrumentation/General/PeerConfigProvider.php diff --git a/examples/instrumentation/otel-sdk.yaml b/examples/instrumentation/otel-sdk.yaml index bf421824a..f898859b1 100644 --- a/examples/instrumentation/otel-sdk.yaml +++ b/examples/instrumentation/otel-sdk.yaml @@ -8,5 +8,27 @@ tracer_provider: instrumentation: php: - - example_instrumentation: - span_name: ${EXAMPLE_INSTRUMENTATION_SPAN_NAME:-example span} \ No newline at end of file + example_instrumentation: + span_name: ${EXAMPLE_INSTRUMENTATION_SPAN_NAME:-example span} + general: + peer: + service_mapping: + - peer: 1.2.3.4 + service: FooService + - peer: 2.3.4.5 + service: BarService + http: + client: + request_captured_headers: + - Content-Type + - Accept + response_captured_headers: + - Content-Type + - Content-Encoding + server: + request_captured_headers: + - Content-Type + - Accept + response_captured_headers: + - Content-Type + - Content-Encoding diff --git a/src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php b/src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php index 06413c186..3b99ed7ba 100644 --- a/src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php +++ b/src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php @@ -8,6 +8,7 @@ final class ConfigurationRegistry { private array $configurations = []; + private array $general = []; public function add(InstrumentationConfiguration $configuration): self { @@ -16,6 +17,13 @@ public function add(InstrumentationConfiguration $configuration): self return $this; } + public function addGeneral(GeneralInstrumentationConfiguration $configuration): self + { + $this->general[$configuration::class] = $configuration; + + return $this; + } + /** * @template C of InstrumentationConfiguration * @param class-string $id @@ -25,4 +33,14 @@ public function get(string $id): ?InstrumentationConfiguration { return $this->configurations[$id] ?? null; } + + /** + * @template C of GeneralInstrumentationConfiguration + * @param class-string $id + * @return C|null + */ + public function getGeneral(string $id): ?GeneralInstrumentationConfiguration + { + return $this->general[$id] ?? null; + } } diff --git a/src/API/Instrumentation/AutoInstrumentation/GeneralInstrumentationConfiguration.php b/src/API/Instrumentation/AutoInstrumentation/GeneralInstrumentationConfiguration.php new file mode 100644 index 000000000..4459cc646 --- /dev/null +++ b/src/API/Instrumentation/AutoInstrumentation/GeneralInstrumentationConfiguration.php @@ -0,0 +1,9 @@ + + */ +class HttpConfigProvider implements ComponentProvider +{ + + public function createPlugin(array $properties, Context $context): GeneralInstrumentationConfiguration + { + return new HttpConfig($properties); + } + + public function getConfig(ComponentProviderRegistry $registry): ArrayNodeDefinition + { + $node = new ArrayNodeDefinition('http'); + $node + ->children() + ->append($this->capturedHeaders('client')) + ->append($this->capturedHeaders('server')) + ->end() + ; + + return $node; + } + + private function capturedHeaders(string $name): ArrayNodeDefinition + { + $node = new ArrayNodeDefinition($name); + $node + ->children() + ->arrayNode('request_captured_headers') + ->scalarPrototype()->end() + ->end() + ->arrayNode('response_captured_headers') + ->scalarPrototype()->end() + ->end() + ->end() + ; + + return $node; + } +} diff --git a/src/Config/SDK/ComponentProvider/Instrumentation/General/PeerConfigProvider.php b/src/Config/SDK/ComponentProvider/Instrumentation/General/PeerConfigProvider.php new file mode 100644 index 000000000..715313adc --- /dev/null +++ b/src/Config/SDK/ComponentProvider/Instrumentation/General/PeerConfigProvider.php @@ -0,0 +1,42 @@ + + */ +class PeerConfigProvider implements ComponentProvider +{ + public function createPlugin(array $properties, Context $context): GeneralInstrumentationConfiguration + { + return new PeerConfig($properties); + } + + public function getConfig(ComponentProviderRegistry $registry): ArrayNodeDefinition + { + $node = new ArrayNodeDefinition('peer'); + $node + ->children() + ->arrayNode('service_mapping') + ->arrayPrototype() + ->children() + ->scalarNode('peer')->end() + ->scalarNode('service')->end() + ->end() + ->end() + ->end() + ->end() + ; + + return $node; + } +} diff --git a/src/Config/SDK/ComponentProvider/InstrumentationConfigurationRegistry.php b/src/Config/SDK/ComponentProvider/InstrumentationConfigurationRegistry.php index dc9c3ba5d..f3fa0f4f8 100644 --- a/src/Config/SDK/ComponentProvider/InstrumentationConfigurationRegistry.php +++ b/src/Config/SDK/ComponentProvider/InstrumentationConfigurationRegistry.php @@ -5,6 +5,7 @@ namespace OpenTelemetry\Config\SDK\ComponentProvider; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\ConfigurationRegistry; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\GeneralInstrumentationConfiguration; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\InstrumentationConfiguration; use OpenTelemetry\Config\SDK\Configuration\ComponentPlugin; use OpenTelemetry\Config\SDK\Configuration\ComponentProvider; @@ -34,6 +35,10 @@ public function createPlugin(array $properties, Context $context): Configuration foreach ($properties['instrumentation']['php'] ?? [] as $configuration) { $configurationRegistry->add($configuration->create($context)); } + /** @phpstan-ignore-next-line */ + foreach ($properties['instrumentation']['general'] ?? [] as $configuration) { + $configurationRegistry->addGeneral($configuration->create($context)); + } return $configurationRegistry; } @@ -48,7 +53,7 @@ public function getConfig(ComponentProviderRegistry $registry): ArrayNodeDefinit ->ignoreExtraKeys() ->children() ->append($registry->componentList('php', InstrumentationConfiguration::class)) - ->variableNode('general')->end() //@todo + ->append($registry->componentList('general', GeneralInstrumentationConfiguration::class)) ->end() ->end() ->end() diff --git a/src/Config/SDK/Configuration/Internal/ComponentProviderRegistry.php b/src/Config/SDK/Configuration/Internal/ComponentProviderRegistry.php index 266319869..8e817af4b 100644 --- a/src/Config/SDK/Configuration/Internal/ComponentProviderRegistry.php +++ b/src/Config/SDK/Configuration/Internal/ComponentProviderRegistry.php @@ -70,7 +70,6 @@ public function component(string $name, string $type): NodeDefinition public function componentList(string $name, string $type): ArrayNodeDefinition { - //@todo clobbers $node = new ArrayNodeDefinition($name); $this->applyToArrayNode($node, $type, true); @@ -116,7 +115,7 @@ public function componentNames(string $name, string $type): ArrayNodeDefinition return $node; } - private function applyToArrayNode(ArrayNodeDefinition $node, string $type, bool $isArray = false): void + private function applyToArrayNode(ArrayNodeDefinition $node, string $type, bool $forceArray = false): void { $node->info(sprintf('Component "%s"', $type)); $node->performNoDeepMerging(); @@ -131,7 +130,8 @@ private function applyToArrayNode(ArrayNodeDefinition $node, string $type, bool } } - if ($isArray) { + if ($forceArray) { + /* if the config was a map rather than an array, force it back to an array */ $node->validate()->always(function (array $value) use ($type): array { $validated = []; foreach ($value as $name => $v) { diff --git a/tests/Unit/SDK/SdkAutoloaderTest.php b/tests/Unit/SDK/SdkAutoloaderTest.php index 67585e195..7cdded3e5 100644 --- a/tests/Unit/SDK/SdkAutoloaderTest.php +++ b/tests/Unit/SDK/SdkAutoloaderTest.php @@ -163,14 +163,6 @@ public function test_autoload_from_config_file(): void $this->assertNotInstanceOf(NoopTracerProvider::class, Globals::tracerProvider()); } - public function test_autoload_with_no_instrumentation_config_does_not_fail(): void - { - $this->setEnvironmentVariable(Variables::OTEL_PHP_AUTOLOAD_ENABLED, 'true'); - $this->setEnvironmentVariable(Variables::OTEL_EXPERIMENTAL_CONFIG_FILE, __DIR__ . '/fixtures/otel-sdk.yaml'); - - $this->assertTrue(SdkAutoloader::autoload()); - } - /** * Tests the scenario where the SDK is created from config file, but a custom component * uses composer's autoload->files to add its own initializer diff --git a/tests/Unit/SDK/fixtures/otel-sdk.yaml b/tests/Unit/SDK/fixtures/otel-sdk.yaml index 6cce68b3f..510810ea1 100644 --- a/tests/Unit/SDK/fixtures/otel-sdk.yaml +++ b/tests/Unit/SDK/fixtures/otel-sdk.yaml @@ -17,18 +17,18 @@ instrumentation: service: FooService - peer: 2.3.4.5 service: BarService - http: - client: - request_captured_headers: - - Content-Type - - Accept - response_captured_headers: - - Content-Type - - Content-Encoding - server: - request_captured_headers: - - Content-Type - - Accept - response_captured_headers: - - Content-Type - - Content-Encoding + http: + client: + request_captured_headers: + - Content-Type + - Accept + response_captured_headers: + - Content-Type + - Content-Encoding + server: + request_captured_headers: + - Content-Type + - Accept + response_captured_headers: + - Content-Type + - Content-Encoding From ccde00db97f590d6f5c436d0524f303de8d19376 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 25 Jun 2024 13:49:10 +1000 Subject: [PATCH 38/43] move general config into sdk --- composer.json | 3 +++ examples/instrumentation/otel-sdk.yaml | 22 ------------------- .../ConfigurationRegistry.php | 18 --------------- .../GeneralInstrumentationConfiguration.php | 2 +- .../General/HttpConfigProvider.php | 2 +- .../General/PeerConfigProvider.php | 2 +- .../InstrumentationConfigurationRegistry.php | 4 ++-- .../Internal/ComponentProviderRegistry.php | 2 +- src/Config/SDK/composer.json | 5 ++++- .../Configuration}/HttpConfig.php | 2 +- .../Configuration}/PeerConfig.php | 2 +- 11 files changed, 15 insertions(+), 49 deletions(-) rename src/{Config/SDK/Component => SDK/Instrumentation/Configuration}/HttpConfig.php (82%) rename src/{Config/SDK/Component => SDK/Instrumentation/Configuration}/PeerConfig.php (82%) diff --git a/composer.json b/composer.json index 184b22c1d..875fd8ebc 100644 --- a/composer.json +++ b/composer.json @@ -150,6 +150,9 @@ "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Logs\\LogRecordProcessorBatch", "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Logs\\LogRecordProcessorSimple", + "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Instrumentation\\General\\HttpConfigProvider", + "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Instrumentation\\General\\PeerConfigProvider", + "OpenTelemetry\\Example\\ExampleConfigProvider" ], "OpenTelemetry\\API\\Instrumentation\\AutoInstrumentation\\HookManager": [ diff --git a/examples/instrumentation/otel-sdk.yaml b/examples/instrumentation/otel-sdk.yaml index f898859b1..53ce87914 100644 --- a/examples/instrumentation/otel-sdk.yaml +++ b/examples/instrumentation/otel-sdk.yaml @@ -10,25 +10,3 @@ instrumentation: php: example_instrumentation: span_name: ${EXAMPLE_INSTRUMENTATION_SPAN_NAME:-example span} - general: - peer: - service_mapping: - - peer: 1.2.3.4 - service: FooService - - peer: 2.3.4.5 - service: BarService - http: - client: - request_captured_headers: - - Content-Type - - Accept - response_captured_headers: - - Content-Type - - Content-Encoding - server: - request_captured_headers: - - Content-Type - - Accept - response_captured_headers: - - Content-Type - - Content-Encoding diff --git a/src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php b/src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php index 3b99ed7ba..06413c186 100644 --- a/src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php +++ b/src/API/Instrumentation/AutoInstrumentation/ConfigurationRegistry.php @@ -8,7 +8,6 @@ final class ConfigurationRegistry { private array $configurations = []; - private array $general = []; public function add(InstrumentationConfiguration $configuration): self { @@ -17,13 +16,6 @@ public function add(InstrumentationConfiguration $configuration): self return $this; } - public function addGeneral(GeneralInstrumentationConfiguration $configuration): self - { - $this->general[$configuration::class] = $configuration; - - return $this; - } - /** * @template C of InstrumentationConfiguration * @param class-string $id @@ -33,14 +25,4 @@ public function get(string $id): ?InstrumentationConfiguration { return $this->configurations[$id] ?? null; } - - /** - * @template C of GeneralInstrumentationConfiguration - * @param class-string $id - * @return C|null - */ - public function getGeneral(string $id): ?GeneralInstrumentationConfiguration - { - return $this->general[$id] ?? null; - } } diff --git a/src/API/Instrumentation/AutoInstrumentation/GeneralInstrumentationConfiguration.php b/src/API/Instrumentation/AutoInstrumentation/GeneralInstrumentationConfiguration.php index 4459cc646..6360e5225 100644 --- a/src/API/Instrumentation/AutoInstrumentation/GeneralInstrumentationConfiguration.php +++ b/src/API/Instrumentation/AutoInstrumentation/GeneralInstrumentationConfiguration.php @@ -4,6 +4,6 @@ namespace OpenTelemetry\API\Instrumentation\AutoInstrumentation; -interface GeneralInstrumentationConfiguration +interface GeneralInstrumentationConfiguration extends InstrumentationConfiguration { } diff --git a/src/Config/SDK/ComponentProvider/Instrumentation/General/HttpConfigProvider.php b/src/Config/SDK/ComponentProvider/Instrumentation/General/HttpConfigProvider.php index bbab7e790..081fc29b5 100644 --- a/src/Config/SDK/ComponentProvider/Instrumentation/General/HttpConfigProvider.php +++ b/src/Config/SDK/ComponentProvider/Instrumentation/General/HttpConfigProvider.php @@ -5,10 +5,10 @@ namespace OpenTelemetry\Config\SDK\ComponentProvider\Instrumentation\General; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\GeneralInstrumentationConfiguration; -use OpenTelemetry\Config\SDK\Component\HttpConfig; use OpenTelemetry\Config\SDK\Configuration\ComponentProvider; use OpenTelemetry\Config\SDK\Configuration\ComponentProviderRegistry; use OpenTelemetry\Config\SDK\Configuration\Context; +use OpenTelemetry\SDK\Instrumentation\Configuration\HttpConfig; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; /** diff --git a/src/Config/SDK/ComponentProvider/Instrumentation/General/PeerConfigProvider.php b/src/Config/SDK/ComponentProvider/Instrumentation/General/PeerConfigProvider.php index 715313adc..47ffa39af 100644 --- a/src/Config/SDK/ComponentProvider/Instrumentation/General/PeerConfigProvider.php +++ b/src/Config/SDK/ComponentProvider/Instrumentation/General/PeerConfigProvider.php @@ -5,10 +5,10 @@ namespace OpenTelemetry\Config\SDK\ComponentProvider\Instrumentation\General; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\GeneralInstrumentationConfiguration; -use OpenTelemetry\Config\SDK\Component\PeerConfig; use OpenTelemetry\Config\SDK\Configuration\ComponentProvider; use OpenTelemetry\Config\SDK\Configuration\ComponentProviderRegistry; use OpenTelemetry\Config\SDK\Configuration\Context; +use OpenTelemetry\SDK\Instrumentation\Configuration\PeerConfig; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; /** diff --git a/src/Config/SDK/ComponentProvider/InstrumentationConfigurationRegistry.php b/src/Config/SDK/ComponentProvider/InstrumentationConfigurationRegistry.php index f3fa0f4f8..44f4ae4c7 100644 --- a/src/Config/SDK/ComponentProvider/InstrumentationConfigurationRegistry.php +++ b/src/Config/SDK/ComponentProvider/InstrumentationConfigurationRegistry.php @@ -24,7 +24,7 @@ class InstrumentationConfigurationRegistry implements ComponentProvider * @param array{ * instrumentation: array{ * php: list>, - * general: array + * general: list> * } * } $properties */ @@ -37,7 +37,7 @@ public function createPlugin(array $properties, Context $context): Configuration } /** @phpstan-ignore-next-line */ foreach ($properties['instrumentation']['general'] ?? [] as $configuration) { - $configurationRegistry->addGeneral($configuration->create($context)); + $configurationRegistry->add($configuration->create($context)); } return $configurationRegistry; diff --git a/src/Config/SDK/Configuration/Internal/ComponentProviderRegistry.php b/src/Config/SDK/Configuration/Internal/ComponentProviderRegistry.php index 8e817af4b..1873f4d3a 100644 --- a/src/Config/SDK/Configuration/Internal/ComponentProviderRegistry.php +++ b/src/Config/SDK/Configuration/Internal/ComponentProviderRegistry.php @@ -131,7 +131,7 @@ private function applyToArrayNode(ArrayNodeDefinition $node, string $type, bool } if ($forceArray) { - /* if the config was a map rather than an array, force it back to an array */ + // if the config was a map rather than an array, force it back to an array $node->validate()->always(function (array $value) use ($type): array { $validated = []; foreach ($value as $name => $v) { diff --git a/src/Config/SDK/composer.json b/src/Config/SDK/composer.json index 4a8deb95a..8cb1259ae 100644 --- a/src/Config/SDK/composer.json +++ b/src/Config/SDK/composer.json @@ -64,7 +64,10 @@ "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Logs\\LogRecordExporterConsole", "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Logs\\LogRecordExporterOtlp", "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Logs\\LogRecordProcessorBatch", - "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Logs\\LogRecordProcessorSimple" + "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Logs\\LogRecordProcessorSimple", + + "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Instrumentation\\General\\HttpConfigProvider", + "OpenTelemetry\\Config\\SDK\\ComponentProvider\\Instrumentation\\General\\PeerConfigProvider" ] } } diff --git a/src/Config/SDK/Component/HttpConfig.php b/src/SDK/Instrumentation/Configuration/HttpConfig.php similarity index 82% rename from src/Config/SDK/Component/HttpConfig.php rename to src/SDK/Instrumentation/Configuration/HttpConfig.php index b1f96e3a7..9fa504d5e 100644 --- a/src/Config/SDK/Component/HttpConfig.php +++ b/src/SDK/Instrumentation/Configuration/HttpConfig.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace OpenTelemetry\Config\SDK\Component; +namespace OpenTelemetry\SDK\Instrumentation\Configuration; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\GeneralInstrumentationConfiguration; diff --git a/src/Config/SDK/Component/PeerConfig.php b/src/SDK/Instrumentation/Configuration/PeerConfig.php similarity index 82% rename from src/Config/SDK/Component/PeerConfig.php rename to src/SDK/Instrumentation/Configuration/PeerConfig.php index f5e8c27d4..3329f8df8 100644 --- a/src/Config/SDK/Component/PeerConfig.php +++ b/src/SDK/Instrumentation/Configuration/PeerConfig.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace OpenTelemetry\Config\SDK\Component; +namespace OpenTelemetry\SDK\Instrumentation\Configuration; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\GeneralInstrumentationConfiguration; From 552a7e88c305d10784e9028babc086849775cb38 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 25 Jun 2024 15:37:14 +1000 Subject: [PATCH 39/43] test config caching --- .../Configuration/ConfigurationFactory.php | 2 +- .../ConfigurationFactoryTest.php | 24 ++++++++++++++++++- .../Configuration/configurations/.gitignore | 1 + 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 tests/Unit/Config/SDK/Configuration/configurations/.gitignore diff --git a/src/Config/SDK/Configuration/ConfigurationFactory.php b/src/Config/SDK/Configuration/ConfigurationFactory.php index c9eb8b9dc..66ce09c32 100644 --- a/src/Config/SDK/Configuration/ConfigurationFactory.php +++ b/src/Config/SDK/Configuration/ConfigurationFactory.php @@ -116,7 +116,7 @@ public function parseFile( class_exists(VarExporter::class) ? sprintf('toArray() //@todo $resources possible null + $resources->toArray() ); return $configuration; diff --git a/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php b/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php index af6ca88f0..666f5366e 100644 --- a/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php +++ b/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php @@ -26,8 +26,19 @@ #[CoversClass(ConfigurationFactory::class)] final class ConfigurationFactoryTest extends TestCase { - + public string $cacheDir; public $properties; + + public function setUp(): void + { + $this->cacheDir = __DIR__ . '/configurations'; + } + + public function tearDown(): void + { + array_map('unlink', array_filter((array) glob($this->cacheDir . "/*cache*"))); + } + /** * @psalm-suppress MissingTemplateParam */ @@ -238,4 +249,15 @@ private function factory(): ConfigurationFactory ]), ); } + + public function test_cache_configuration(): void + { + $file = $this->cacheDir . '/kitchen-sink.yaml'; + $cacheFile = $this->cacheDir . '/kitchen-sink.cache'; + $this->assertFalse(file_exists($cacheFile), 'cache does not initially exist'); + $plugin = self::factory()->parseFile($file, $cacheFile); + $this->assertTrue(file_exists($cacheFile)); + $fromCache = self::factory()->parseFile($file, $cacheFile); + $this->assertEquals($fromCache, $plugin); + } } diff --git a/tests/Unit/Config/SDK/Configuration/configurations/.gitignore b/tests/Unit/Config/SDK/Configuration/configurations/.gitignore new file mode 100644 index 000000000..69758e218 --- /dev/null +++ b/tests/Unit/Config/SDK/Configuration/configurations/.gitignore @@ -0,0 +1 @@ +**cache** From 1164e6bdfeb7a0371b8b102d4b9d61126b8a43f7 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 25 Jun 2024 16:29:07 +1000 Subject: [PATCH 40/43] move general instrumentation config to api --- rector.php | 4 ---- .../Instrumentation/Configuration/General}/HttpConfig.php | 2 +- .../Instrumentation/Configuration/General}/PeerConfig.php | 2 +- .../Instrumentation/General/HttpConfigProvider.php | 2 +- .../Instrumentation/General/PeerConfigProvider.php | 2 +- .../Config/SDK/Configuration/ConfigurationFactoryTest.php | 2 +- 6 files changed, 5 insertions(+), 9 deletions(-) rename src/{SDK/Instrumentation/Configuration => API/Instrumentation/Configuration/General}/HttpConfig.php (80%) rename src/{SDK/Instrumentation/Configuration => API/Instrumentation/Configuration/General}/PeerConfig.php (80%) diff --git a/rector.php b/rector.php index 6526f4542..c04b6d53a 100644 --- a/rector.php +++ b/rector.php @@ -28,10 +28,6 @@ PHPUnitSetList::PHPUNIT_100, ]); $rectorConfig->skip([ - CallableThisArrayToAnonymousFunctionRector::class => [ - __DIR__ . '/src/SDK/SdkBuilder.php', - __DIR__ . '/src/SDK/SdkAutoloader.php', - ], FlipTypeControlToUseExclusiveTypeRector::class, NewInInitializerRector::class => [ __DIR__ . '/src/SDK/Trace/Sampler/ParentBased.php', diff --git a/src/SDK/Instrumentation/Configuration/HttpConfig.php b/src/API/Instrumentation/Configuration/General/HttpConfig.php similarity index 80% rename from src/SDK/Instrumentation/Configuration/HttpConfig.php rename to src/API/Instrumentation/Configuration/General/HttpConfig.php index 9fa504d5e..ce4015aee 100644 --- a/src/SDK/Instrumentation/Configuration/HttpConfig.php +++ b/src/API/Instrumentation/Configuration/General/HttpConfig.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace OpenTelemetry\SDK\Instrumentation\Configuration; +namespace OpenTelemetry\API\Instrumentation\Configuration\General; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\GeneralInstrumentationConfiguration; diff --git a/src/SDK/Instrumentation/Configuration/PeerConfig.php b/src/API/Instrumentation/Configuration/General/PeerConfig.php similarity index 80% rename from src/SDK/Instrumentation/Configuration/PeerConfig.php rename to src/API/Instrumentation/Configuration/General/PeerConfig.php index 3329f8df8..32502e157 100644 --- a/src/SDK/Instrumentation/Configuration/PeerConfig.php +++ b/src/API/Instrumentation/Configuration/General/PeerConfig.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace OpenTelemetry\SDK\Instrumentation\Configuration; +namespace OpenTelemetry\API\Instrumentation\Configuration\General; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\GeneralInstrumentationConfiguration; diff --git a/src/Config/SDK/ComponentProvider/Instrumentation/General/HttpConfigProvider.php b/src/Config/SDK/ComponentProvider/Instrumentation/General/HttpConfigProvider.php index 081fc29b5..01c86f73c 100644 --- a/src/Config/SDK/ComponentProvider/Instrumentation/General/HttpConfigProvider.php +++ b/src/Config/SDK/ComponentProvider/Instrumentation/General/HttpConfigProvider.php @@ -5,10 +5,10 @@ namespace OpenTelemetry\Config\SDK\ComponentProvider\Instrumentation\General; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\GeneralInstrumentationConfiguration; +use OpenTelemetry\API\Instrumentation\Configuration\General\HttpConfig; use OpenTelemetry\Config\SDK\Configuration\ComponentProvider; use OpenTelemetry\Config\SDK\Configuration\ComponentProviderRegistry; use OpenTelemetry\Config\SDK\Configuration\Context; -use OpenTelemetry\SDK\Instrumentation\Configuration\HttpConfig; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; /** diff --git a/src/Config/SDK/ComponentProvider/Instrumentation/General/PeerConfigProvider.php b/src/Config/SDK/ComponentProvider/Instrumentation/General/PeerConfigProvider.php index 47ffa39af..f9273a422 100644 --- a/src/Config/SDK/ComponentProvider/Instrumentation/General/PeerConfigProvider.php +++ b/src/Config/SDK/ComponentProvider/Instrumentation/General/PeerConfigProvider.php @@ -5,10 +5,10 @@ namespace OpenTelemetry\Config\SDK\ComponentProvider\Instrumentation\General; use OpenTelemetry\API\Instrumentation\AutoInstrumentation\GeneralInstrumentationConfiguration; +use OpenTelemetry\API\Instrumentation\Configuration\General\PeerConfig; use OpenTelemetry\Config\SDK\Configuration\ComponentProvider; use OpenTelemetry\Config\SDK\Configuration\ComponentProviderRegistry; use OpenTelemetry\Config\SDK\Configuration\Context; -use OpenTelemetry\SDK\Instrumentation\Configuration\PeerConfig; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; /** diff --git a/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php b/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php index 666f5366e..04302026d 100644 --- a/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php +++ b/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php @@ -36,7 +36,7 @@ public function setUp(): void public function tearDown(): void { - array_map('unlink', array_filter((array) glob($this->cacheDir . "/*cache*"))); + array_map('unlink', array_filter((array) glob($this->cacheDir . '/*cache*'))); } /** From 36956304017484baa321779f421ee4ddaee55875 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 25 Jun 2024 16:29:41 +1000 Subject: [PATCH 41/43] avoid bad version of sebastian/exporter --- composer.json | 1 + 1 file changed, 1 insertion(+) diff --git a/composer.json b/composer.json index 875fd8ebc..402393bef 100644 --- a/composer.json +++ b/composer.json @@ -101,6 +101,7 @@ "phpstan/phpstan-mockery": "^1.1", "phpstan/phpstan-phpunit": "^1.3", "phpunit/phpunit": "^10 || ^11", + "sebastian/exporter": "<= 6.0.1 || >= 6.1.3", "symfony/http-client": "^5.2", "symfony/var-exporter": "^5.4 || ^6.4 || ^7.0", "symfony/yaml": "^5.4 || ^6.4 || ^7.0" From 7c0131b0932d7c3cda72745e8b1b829dbbf41e42 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Wed, 26 Jun 2024 09:37:47 +1000 Subject: [PATCH 42/43] bump config yaml files to 0.3 --- examples/load_config.yaml | 2 +- examples/load_config_env.yaml | 2 +- tests/Unit/Config/SDK/Configuration/configurations/anchors.yaml | 2 +- .../Config/SDK/Configuration/configurations/kitchen-sink.yaml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/load_config.yaml b/examples/load_config.yaml index 9e1901de8..d4d8d4de4 100644 --- a/examples/load_config.yaml +++ b/examples/load_config.yaml @@ -1,4 +1,4 @@ -file_format: '0.1' +file_format: '0.3' resource: attributes: diff --git a/examples/load_config_env.yaml b/examples/load_config_env.yaml index b62262d29..b593419be 100644 --- a/examples/load_config_env.yaml +++ b/examples/load_config_env.yaml @@ -1,4 +1,4 @@ -file_format: '0.1' +file_format: '0.3' disabled: ${OTEL_SDK_DISABLED} diff --git a/tests/Unit/Config/SDK/Configuration/configurations/anchors.yaml b/tests/Unit/Config/SDK/Configuration/configurations/anchors.yaml index f5c0f1a10..48993b2f7 100644 --- a/tests/Unit/Config/SDK/Configuration/configurations/anchors.yaml +++ b/tests/Unit/Config/SDK/Configuration/configurations/anchors.yaml @@ -2,7 +2,7 @@ # anchors.yaml demonstrates anchor substitution to reuse OTLP exporter configuration across signals. -file_format: "0.1" +file_format: "0.3" exporters: otlp: &otlp-exporter protocol: http/protobuf diff --git a/tests/Unit/Config/SDK/Configuration/configurations/kitchen-sink.yaml b/tests/Unit/Config/SDK/Configuration/configurations/kitchen-sink.yaml index 41e9cfafc..aed29733b 100644 --- a/tests/Unit/Config/SDK/Configuration/configurations/kitchen-sink.yaml +++ b/tests/Unit/Config/SDK/Configuration/configurations/kitchen-sink.yaml @@ -10,7 +10,7 @@ # Configuration values are set to their defaults when default values are defined. # The file format version -file_format: "0.1" +file_format: "0.3" # Configure if the SDK is disabled or not. This is not required to be provided # to ensure the SDK isn't disabled, the default value when this is not provided From 90a69ce9b650f091a6126823306b9b6b80243fee Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Wed, 26 Jun 2024 10:23:47 +1000 Subject: [PATCH 43/43] fix tests --- tests/Unit/Config/SDK/Configuration/configurations/anchors.yaml | 2 +- .../Config/SDK/Configuration/configurations/kitchen-sink.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Unit/Config/SDK/Configuration/configurations/anchors.yaml b/tests/Unit/Config/SDK/Configuration/configurations/anchors.yaml index 48993b2f7..f5c0f1a10 100644 --- a/tests/Unit/Config/SDK/Configuration/configurations/anchors.yaml +++ b/tests/Unit/Config/SDK/Configuration/configurations/anchors.yaml @@ -2,7 +2,7 @@ # anchors.yaml demonstrates anchor substitution to reuse OTLP exporter configuration across signals. -file_format: "0.3" +file_format: "0.1" exporters: otlp: &otlp-exporter protocol: http/protobuf diff --git a/tests/Unit/Config/SDK/Configuration/configurations/kitchen-sink.yaml b/tests/Unit/Config/SDK/Configuration/configurations/kitchen-sink.yaml index aed29733b..41e9cfafc 100644 --- a/tests/Unit/Config/SDK/Configuration/configurations/kitchen-sink.yaml +++ b/tests/Unit/Config/SDK/Configuration/configurations/kitchen-sink.yaml @@ -10,7 +10,7 @@ # Configuration values are set to their defaults when default values are defined. # The file format version -file_format: "0.3" +file_format: "0.1" # Configure if the SDK is disabled or not. This is not required to be provided # to ensure the SDK isn't disabled, the default value when this is not provided