diff --git a/src/API/Behavior/Internal/LogWriterFactory.php b/src/API/Behavior/Internal/LogWriterFactory.php index e774287cd..07c48cea5 100644 --- a/src/API/Behavior/Internal/LogWriterFactory.php +++ b/src/API/Behavior/Internal/LogWriterFactory.php @@ -9,7 +9,7 @@ use OpenTelemetry\API\Behavior\Internal\LogWriter\NoopLogWriter; use OpenTelemetry\API\Behavior\Internal\LogWriter\Psr3LogWriter; use OpenTelemetry\API\Behavior\Internal\LogWriter\StreamLogWriter; -use OpenTelemetry\API\Globals; +use OpenTelemetry\API\Instrumentation\ConfigurationResolver; use OpenTelemetry\API\LoggerHolder; class LogWriterFactory @@ -18,16 +18,7 @@ class LogWriterFactory public function create(): LogWriterInterface { - $dest = Globals::configurationResolver()->getEnum(self::OTEL_PHP_LOG_DESTINATION); - //we might not have an SDK, so attempt to get from environment - if (!$dest) { - $dest = array_key_exists(self::OTEL_PHP_LOG_DESTINATION, $_SERVER) - ? $_SERVER[self::OTEL_PHP_LOG_DESTINATION] - : getenv(self::OTEL_PHP_LOG_DESTINATION); - } - if (!$dest) { - $dest = ini_get(self::OTEL_PHP_LOG_DESTINATION); - } + $dest = (new ConfigurationResolver())->getString(self::OTEL_PHP_LOG_DESTINATION); $logger = LoggerHolder::get(); switch ($dest) { diff --git a/src/API/Globals.php b/src/API/Globals.php index 8dc8e4b11..8f04b0b42 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\ConfigurationResolverInterface; use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\API\Instrumentation\ContextKeys; use OpenTelemetry\API\Logs\LoggerProviderInterface; @@ -32,20 +31,17 @@ final class Globals private MeterProviderInterface $meterProvider; private TextMapPropagatorInterface $propagator; private LoggerProviderInterface $loggerProvider; - private ConfigurationResolverInterface $configurationResolver; public function __construct( TracerProviderInterface $tracerProvider, MeterProviderInterface $meterProvider, LoggerProviderInterface $loggerProvider, - TextMapPropagatorInterface $propagator, - ConfigurationResolverInterface $configurationResolver + TextMapPropagatorInterface $propagator ) { $this->tracerProvider = $tracerProvider; $this->meterProvider = $meterProvider; $this->loggerProvider = $loggerProvider; $this->propagator = $propagator; - $this->configurationResolver = $configurationResolver; } public static function tracerProvider(): TracerProviderInterface @@ -68,11 +64,6 @@ public static function loggerProvider(): LoggerProviderInterface return Context::getCurrent()->get(ContextKeys::loggerProvider()) ?? self::globals()->loggerProvider; } - public static function configurationResolver(): ConfigurationResolverInterface - { - return Context::getCurrent()->get(ContextKeys::configurationResolver()) ?? self::globals()->configurationResolver; - } - /** * @param Closure(Configurator): Configurator $initializer * @@ -113,11 +104,10 @@ private static function globals(): self $meterProvider = $context->get(ContextKeys::meterProvider()); $propagator = $context->get(ContextKeys::propagator()); $loggerProvider = $context->get(ContextKeys::loggerProvider()); - $configurationResolver = $context->get(ContextKeys::configurationResolver()); - assert(isset($tracerProvider, $meterProvider, $loggerProvider, $propagator, $configurationResolver)); + assert(isset($tracerProvider, $meterProvider, $loggerProvider, $propagator)); - return self::$globals = new self($tracerProvider, $meterProvider, $loggerProvider, $propagator, $configurationResolver); + return self::$globals = new self($tracerProvider, $meterProvider, $loggerProvider, $propagator); } /** diff --git a/src/API/Instrumentation/ConfigurationResolver.php b/src/API/Instrumentation/ConfigurationResolver.php new file mode 100644 index 000000000..bb5619c30 --- /dev/null +++ b/src/API/Instrumentation/ConfigurationResolver.php @@ -0,0 +1,77 @@ +getVariable($name) !== null; + } + + public function getString(string $name): ?string + { + return $this->getVariable($name); + } + + public function getBoolean(string $name): ?bool + { + $value = $this->getVariable($name); + if ($value === null) { + return null; + } + + return ($value === 'true'); + } + + public function getInt(string $name): ?int + { + $value = $this->getVariable($name); + if ($value === null) { + return null; + } + if (filter_var($value, FILTER_VALIDATE_INT) === false) { + //log warning + return null; + } + + return (int) $value; + } + + public function getList(string $name): array + { + $value = $this->getVariable($name); + if ($value === null) { + return []; + } + + return explode(',', $value); + } + + private function getVariable(string $name): ?string + { + $value = $_SERVER[$name] ?? null; + if ($value !== false && !self::isEmpty($value)) { + assert(is_string($value)); + + return $value; + } + $value = getenv($name); + if ($value !== false && !self::isEmpty($value)) { + return $value; + } + $value = ini_get($name); + if ($value !== false && !self::isEmpty($value)) { + return $value; + } + + return null; + } + + private static function isEmpty($value): bool + { + return $value === false || $value === null || $value === ''; + } +} diff --git a/src/API/Instrumentation/ConfigurationResolverInterface.php b/src/API/Instrumentation/ConfigurationResolverInterface.php index 47c73f2e9..79bd94047 100644 --- a/src/API/Instrumentation/ConfigurationResolverInterface.php +++ b/src/API/Instrumentation/ConfigurationResolverInterface.php @@ -11,5 +11,4 @@ public function getString(string $name): ?string; public function getBoolean(string $name): ?bool; public function getInt(string $name): ?int; public function getList(string $name): array; - public function getEnum(string $name): ?string; } diff --git a/src/API/Instrumentation/Configurator.php b/src/API/Instrumentation/Configurator.php index ab8ee9455..71d301363 100644 --- a/src/API/Instrumentation/Configurator.php +++ b/src/API/Instrumentation/Configurator.php @@ -28,7 +28,6 @@ final class Configurator implements ImplicitContextKeyedInterface private ?MeterProviderInterface $meterProvider = null; private ?TextMapPropagatorInterface $propagator = null; private ?LoggerProviderInterface $loggerProvider = null; - private ?ConfigurationResolverInterface $configurationResolver = null; private function __construct() { @@ -52,7 +51,6 @@ public static function createNoop(): Configurator ->withMeterProvider(new NoopMeterProvider()) ->withPropagator(new NoopTextMapPropagator()) ->withLoggerProvider(new NoopLoggerProvider()) - ->withConfigurationResolver(new NoopConfigurationResolver()) ; } @@ -77,9 +75,6 @@ public function storeInContext(?ContextInterface $context = null): ContextInterf if ($this->loggerProvider !== null) { $context = $context->with(ContextKeys::loggerProvider(), $this->loggerProvider); } - if ($this->configurationResolver !== null) { - $context = $context->with(ContextKeys::configurationResolver(), $this->configurationResolver); - } return $context; } @@ -115,12 +110,4 @@ public function withLoggerProvider(?LoggerProviderInterface $loggerProvider): Co return $self; } - - public function withConfigurationResolver(?ConfigurationResolverInterface $configurationResolver): Configurator - { - $self = clone $this; - $self->configurationResolver = $configurationResolver; - - return $self; - } } diff --git a/src/API/Instrumentation/ContextKeys.php b/src/API/Instrumentation/ContextKeys.php index d6824b180..ea1a66416 100644 --- a/src/API/Instrumentation/ContextKeys.php +++ b/src/API/Instrumentation/ContextKeys.php @@ -55,14 +55,4 @@ public static function loggerProvider(): ContextKeyInterface return $instance ??= Context::createKey(LoggerProviderInterface::class); } - - /** - * @return ContextKeyInterface - */ - public static function configurationResolver(): ContextKeyInterface - { - static $instance; - - return $instance ??= Context::createKey(ConfigurationResolverInterface::class); - } } diff --git a/src/API/Instrumentation/NoopConfigurationResolver.php b/src/API/Instrumentation/NoopConfigurationResolver.php deleted file mode 100644 index 3697c30b8..000000000 --- a/src/API/Instrumentation/NoopConfigurationResolver.php +++ /dev/null @@ -1,38 +0,0 @@ -withMeterProvider($meterProvider) ->withLoggerProvider($loggerProvider) ->withPropagator($propagator) - ->withConfigurationResolver(new ConfigurationResolver()) ; }); diff --git a/tests/Unit/API/Instrumentation/ConfigurationResolverTest.php b/tests/Unit/API/Instrumentation/ConfigurationResolverTest.php new file mode 100644 index 000000000..4b7a24c9c --- /dev/null +++ b/tests/Unit/API/Instrumentation/ConfigurationResolverTest.php @@ -0,0 +1,123 @@ +resolver = new ConfigurationResolver(); + } + + public function tearDown(): void + { + $this->restoreEnvironmentVariables(); + } + + /** + * @dataProvider hasProvider + */ + public function test_has(?string $value, bool $expected): void + { + $this->assertFalse($this->resolver->has('OTEL_FOO')); + $this->setEnvironmentVariable('OTEL_FOO', $value); + $this->assertSame($expected, $this->resolver->has('OTEL_FOO')); + } + + public static function hasProvider(): array + { + return [ + ['bar', true], + ['', false], + [null, false], + ]; + } + + public function test_get_string(): void + { + $this->assertFalse($this->resolver->has('OTEL_FOO')); + $this->setEnvironmentVariable('OTEL_FOO', 'bar'); + $this->assertSame('bar', $this->resolver->getString('OTEL_FOO')); + } + + /** + * @dataProvider booleanProvider + */ + public function test_get_boolean(?string $value, ?bool $expected): void + { + $this->assertFalse($this->resolver->has('OTEL_FOO')); + $this->setEnvironmentVariable('OTEL_FOO', $value); + $this->assertSame($expected, $this->resolver->getBoolean('OTEL_FOO')); + } + + public static function booleanProvider(): array + { + return [ + ['true', true], + ['false', false], + ['random', false], + ['', null], + [null, null], + ]; + } + + /** + * @dataProvider intProvider + */ + public function test_get_int(?string $value, ?int $expected): void + { + $this->assertFalse($this->resolver->has('OTEL_FOO')); + $this->setEnvironmentVariable('OTEL_FOO', $value); + $this->assertSame($expected, $this->resolver->getInt('OTEL_FOO')); + } + + public static function intProvider(): array + { + return [ + ['0', 0], + ['1', 1], + ['-1', -1], + ['', null], + [null, null], + ['3.14159', null], + ]; + } + + /** + * @dataProvider listProvider + */ + public function test_get_list(?string $value, array $expected): void + { + $this->assertFalse($this->resolver->has('OTEL_FOO')); + $this->setEnvironmentVariable('OTEL_FOO', $value); + $this->assertSame($expected, $this->resolver->getList('OTEL_FOO')); + } + + public static function listProvider(): array + { + return [ + ['foo,bar,baz', ['foo','bar','baz']], + ['foo', ['foo']], + ['', []], + [null, []], + ]; + } + + public function test_has_when_missing(): void + { + $this->assertFalse($this->resolver->has('OTEL_MISSING')); + } +} diff --git a/tests/Unit/API/Instrumentation/InstrumentationTest.php b/tests/Unit/API/Instrumentation/InstrumentationTest.php index 9b9918fd4..ce96727b5 100644 --- a/tests/Unit/API/Instrumentation/InstrumentationTest.php +++ b/tests/Unit/API/Instrumentation/InstrumentationTest.php @@ -6,9 +6,7 @@ use OpenTelemetry\API\Globals; use OpenTelemetry\API\Instrumentation\CachedInstrumentation; -use OpenTelemetry\API\Instrumentation\ConfigurationResolverInterface; use OpenTelemetry\API\Instrumentation\Configurator; -use OpenTelemetry\API\Instrumentation\NoopConfigurationResolver; use OpenTelemetry\API\Logs\LoggerInterface; use OpenTelemetry\API\Logs\LoggerProviderInterface; use OpenTelemetry\API\Logs\NoopLoggerProvider; @@ -43,7 +41,6 @@ public function test_globals_not_configured_returns_noop_instances(): void $this->assertInstanceOf(NoopMeterProvider::class, Globals::meterProvider()); $this->assertInstanceOf(NoopTextMapPropagator::class, Globals::propagator()); $this->assertInstanceOf(NoopLoggerProvider::class, Globals::loggerProvider()); - $this->assertInstanceOf(NoopConfigurationResolver::class, Globals::configurationResolver()); } public function test_globals_returns_configured_instances(): void @@ -52,14 +49,12 @@ public function test_globals_returns_configured_instances(): void $meterProvider = $this->createMock(MeterProviderInterface::class); $propagator = $this->createMock(TextMapPropagatorInterface::class); $loggerProvider = $this->createMock(LoggerProviderInterface::class); - $configurationResolver = $this->createMock(ConfigurationResolverInterface::class); $scope = Configurator::create() ->withTracerProvider($tracerProvider) ->withMeterProvider($meterProvider) ->withPropagator($propagator) ->withLoggerProvider($loggerProvider) - ->withConfigurationResolver($configurationResolver) ->activate(); try { @@ -67,7 +62,6 @@ public function test_globals_returns_configured_instances(): void $this->assertSame($meterProvider, Globals::meterProvider()); $this->assertSame($propagator, Globals::propagator()); $this->assertSame($loggerProvider, Globals::loggerProvider()); - $this->assertSame($configurationResolver, Globals::configurationResolver()); } finally { $scope->detach(); } diff --git a/tests/Unit/SDK/SdkAutoloaderTest.php b/tests/Unit/SDK/SdkAutoloaderTest.php index 1b60a1f9e..51cb66df4 100644 --- a/tests/Unit/SDK/SdkAutoloaderTest.php +++ b/tests/Unit/SDK/SdkAutoloaderTest.php @@ -6,7 +6,6 @@ use AssertWell\PHPUnitGlobalState\EnvironmentVariables; use OpenTelemetry\API\Globals; -use OpenTelemetry\API\Instrumentation\NoopConfigurationResolver; use OpenTelemetry\API\LoggerHolder; use OpenTelemetry\API\Logs\NoopLoggerProvider; use OpenTelemetry\API\Metrics\Noop\NoopMeterProvider; @@ -42,7 +41,6 @@ public function test_disabled_by_default(): void $this->assertInstanceOf(NoopMeterProvider::class, Globals::meterProvider()); $this->assertInstanceOf(NoopTracerProvider::class, Globals::tracerProvider()); $this->assertInstanceOf(NoopLoggerProvider::class, Globals::loggerProvider()); - $this->assertInstanceOf(NoopConfigurationResolver::class, Globals::configurationResolver()); $this->assertInstanceOf(NoopTextMapPropagator::class, Globals::propagator(), 'propagator not initialized by disabled autoloader'); } @@ -60,7 +58,6 @@ public function test_sdk_disabled_does_not_disable_propagator(): void $this->assertNotInstanceOf(NoopTextMapPropagator::class, Globals::propagator()); $this->assertInstanceOf(NoopMeterProvider::class, Globals::meterProvider()); $this->assertInstanceOf(NoopTracerProvider::class, Globals::tracerProvider()); - $this->assertInstanceOf(NoopConfigurationResolver::class, Globals::configurationResolver()); } public function test_enabled_by_configuration(): void @@ -71,6 +68,5 @@ public function test_enabled_by_configuration(): void $this->assertNotInstanceOf(NoopMeterProvider::class, Globals::meterProvider()); $this->assertNotInstanceOf(NoopTracerProvider::class, Globals::tracerProvider()); $this->assertNotInstanceOf(NoopLoggerProvider::class, Globals::loggerProvider()); - $this->assertNotInstanceOf(NoopConfigurationResolver::class, Globals::configurationResolver()); } }