Skip to content

Commit d577d73

Browse files
authored
remove global configuration resolver (#1118)
Exposing it through Globals can cause a race condition. If something (eg, a contrib module) retrieves the configuration resolver from Globals too early during autoloading, then it causes the rest of the objects from Globals to be initialized. This can happen before all required modules have been registered (depending on autoload->files execution order, which is variable). Instead, create a simple API config resolver which does the basics of what the SDK one does (without all the type checking and validation).
1 parent 6cb00cd commit d577d73

File tree

7 files changed

+82
-86
lines changed

7 files changed

+82
-86
lines changed

Behavior/Internal/LogWriterFactory.php

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
use OpenTelemetry\API\Behavior\Internal\LogWriter\NoopLogWriter;
1010
use OpenTelemetry\API\Behavior\Internal\LogWriter\Psr3LogWriter;
1111
use OpenTelemetry\API\Behavior\Internal\LogWriter\StreamLogWriter;
12-
use OpenTelemetry\API\Globals;
12+
use OpenTelemetry\API\Instrumentation\ConfigurationResolver;
1313
use OpenTelemetry\API\LoggerHolder;
1414

1515
class LogWriterFactory
@@ -18,16 +18,7 @@ class LogWriterFactory
1818

1919
public function create(): LogWriterInterface
2020
{
21-
$dest = Globals::configurationResolver()->getEnum(self::OTEL_PHP_LOG_DESTINATION);
22-
//we might not have an SDK, so attempt to get from environment
23-
if (!$dest) {
24-
$dest = array_key_exists(self::OTEL_PHP_LOG_DESTINATION, $_SERVER)
25-
? $_SERVER[self::OTEL_PHP_LOG_DESTINATION]
26-
: getenv(self::OTEL_PHP_LOG_DESTINATION);
27-
}
28-
if (!$dest) {
29-
$dest = ini_get(self::OTEL_PHP_LOG_DESTINATION);
30-
}
21+
$dest = (new ConfigurationResolver())->getString(self::OTEL_PHP_LOG_DESTINATION);
3122
$logger = LoggerHolder::get();
3223

3324
switch ($dest) {

Globals.php

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
use function assert;
88
use Closure;
99
use const E_USER_WARNING;
10-
use OpenTelemetry\API\Instrumentation\ConfigurationResolverInterface;
1110
use OpenTelemetry\API\Instrumentation\Configurator;
1211
use OpenTelemetry\API\Instrumentation\ContextKeys;
1312
use OpenTelemetry\API\Logs\LoggerProviderInterface;
@@ -32,20 +31,17 @@ final class Globals
3231
private MeterProviderInterface $meterProvider;
3332
private TextMapPropagatorInterface $propagator;
3433
private LoggerProviderInterface $loggerProvider;
35-
private ConfigurationResolverInterface $configurationResolver;
3634

3735
public function __construct(
3836
TracerProviderInterface $tracerProvider,
3937
MeterProviderInterface $meterProvider,
4038
LoggerProviderInterface $loggerProvider,
41-
TextMapPropagatorInterface $propagator,
42-
ConfigurationResolverInterface $configurationResolver
39+
TextMapPropagatorInterface $propagator
4340
) {
4441
$this->tracerProvider = $tracerProvider;
4542
$this->meterProvider = $meterProvider;
4643
$this->loggerProvider = $loggerProvider;
4744
$this->propagator = $propagator;
48-
$this->configurationResolver = $configurationResolver;
4945
}
5046

5147
public static function tracerProvider(): TracerProviderInterface
@@ -68,11 +64,6 @@ public static function loggerProvider(): LoggerProviderInterface
6864
return Context::getCurrent()->get(ContextKeys::loggerProvider()) ?? self::globals()->loggerProvider;
6965
}
7066

71-
public static function configurationResolver(): ConfigurationResolverInterface
72-
{
73-
return Context::getCurrent()->get(ContextKeys::configurationResolver()) ?? self::globals()->configurationResolver;
74-
}
75-
7667
/**
7768
* @param Closure(Configurator): Configurator $initializer
7869
*
@@ -113,11 +104,10 @@ private static function globals(): self
113104
$meterProvider = $context->get(ContextKeys::meterProvider());
114105
$propagator = $context->get(ContextKeys::propagator());
115106
$loggerProvider = $context->get(ContextKeys::loggerProvider());
116-
$configurationResolver = $context->get(ContextKeys::configurationResolver());
117107

118-
assert(isset($tracerProvider, $meterProvider, $loggerProvider, $propagator, $configurationResolver));
108+
assert(isset($tracerProvider, $meterProvider, $loggerProvider, $propagator));
119109

120-
return self::$globals = new self($tracerProvider, $meterProvider, $loggerProvider, $propagator, $configurationResolver);
110+
return self::$globals = new self($tracerProvider, $meterProvider, $loggerProvider, $propagator);
121111
}
122112

123113
/**
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace OpenTelemetry\API\Instrumentation;
6+
7+
class ConfigurationResolver implements ConfigurationResolverInterface
8+
{
9+
public function has(string $name): bool
10+
{
11+
return $this->getVariable($name) !== null;
12+
}
13+
14+
public function getString(string $name): ?string
15+
{
16+
return $this->getVariable($name);
17+
}
18+
19+
public function getBoolean(string $name): ?bool
20+
{
21+
$value = $this->getVariable($name);
22+
if ($value === null) {
23+
return null;
24+
}
25+
26+
return ($value === 'true');
27+
}
28+
29+
public function getInt(string $name): ?int
30+
{
31+
$value = $this->getVariable($name);
32+
if ($value === null) {
33+
return null;
34+
}
35+
if (filter_var($value, FILTER_VALIDATE_INT) === false) {
36+
//log warning
37+
return null;
38+
}
39+
40+
return (int) $value;
41+
}
42+
43+
public function getList(string $name): array
44+
{
45+
$value = $this->getVariable($name);
46+
if ($value === null) {
47+
return [];
48+
}
49+
50+
return explode(',', $value);
51+
}
52+
53+
private function getVariable(string $name): ?string
54+
{
55+
$value = $_SERVER[$name] ?? null;
56+
if ($value !== false && !self::isEmpty($value)) {
57+
assert(is_string($value));
58+
59+
return $value;
60+
}
61+
$value = getenv($name);
62+
if ($value !== false && !self::isEmpty($value)) {
63+
return $value;
64+
}
65+
$value = ini_get($name);
66+
if ($value !== false && !self::isEmpty($value)) {
67+
return $value;
68+
}
69+
70+
return null;
71+
}
72+
73+
private static function isEmpty($value): bool
74+
{
75+
return $value === false || $value === null || $value === '';
76+
}
77+
}

Instrumentation/ConfigurationResolverInterface.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,4 @@ public function getString(string $name): ?string;
1111
public function getBoolean(string $name): ?bool;
1212
public function getInt(string $name): ?int;
1313
public function getList(string $name): array;
14-
public function getEnum(string $name): ?string;
1514
}

Instrumentation/Configurator.php

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ final class Configurator implements ImplicitContextKeyedInterface
2828
private ?MeterProviderInterface $meterProvider = null;
2929
private ?TextMapPropagatorInterface $propagator = null;
3030
private ?LoggerProviderInterface $loggerProvider = null;
31-
private ?ConfigurationResolverInterface $configurationResolver = null;
3231

3332
private function __construct()
3433
{
@@ -52,7 +51,6 @@ public static function createNoop(): Configurator
5251
->withMeterProvider(new NoopMeterProvider())
5352
->withPropagator(new NoopTextMapPropagator())
5453
->withLoggerProvider(new NoopLoggerProvider())
55-
->withConfigurationResolver(new NoopConfigurationResolver())
5654
;
5755
}
5856

@@ -77,9 +75,6 @@ public function storeInContext(?ContextInterface $context = null): ContextInterf
7775
if ($this->loggerProvider !== null) {
7876
$context = $context->with(ContextKeys::loggerProvider(), $this->loggerProvider);
7977
}
80-
if ($this->configurationResolver !== null) {
81-
$context = $context->with(ContextKeys::configurationResolver(), $this->configurationResolver);
82-
}
8378

8479
return $context;
8580
}
@@ -115,12 +110,4 @@ public function withLoggerProvider(?LoggerProviderInterface $loggerProvider): Co
115110

116111
return $self;
117112
}
118-
119-
public function withConfigurationResolver(?ConfigurationResolverInterface $configurationResolver): Configurator
120-
{
121-
$self = clone $this;
122-
$self->configurationResolver = $configurationResolver;
123-
124-
return $self;
125-
}
126113
}

Instrumentation/ContextKeys.php

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,4 @@ public static function loggerProvider(): ContextKeyInterface
5555

5656
return $instance ??= Context::createKey(LoggerProviderInterface::class);
5757
}
58-
59-
/**
60-
* @return ContextKeyInterface<ConfigurationResolverInterface>
61-
*/
62-
public static function configurationResolver(): ContextKeyInterface
63-
{
64-
static $instance;
65-
66-
return $instance ??= Context::createKey(ConfigurationResolverInterface::class);
67-
}
6858
}

Instrumentation/NoopConfigurationResolver.php

Lines changed: 0 additions & 38 deletions
This file was deleted.

0 commit comments

Comments
 (0)