Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

create text map propagators from registry #885

Merged
merged 3 commits into from
Dec 9, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
create text map propagators from registry
removing undocumented dependency between sdk and extension by adding text map propagators to a registry
as part of composer autoloading, similar to how exporters, transports etc are treated.
  • Loading branch information
brettmc committed Dec 6, 2022
commit 0f20a643cb86229bf856c5bf727758d16afcfde9
2 changes: 2 additions & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@
"src/Contrib/Grpc/_register.php",
"src/Contrib/Newrelic/_register.php",
"src/Contrib/Zipkin/_register.php",
"src/Extension/Propagator/B3/_register.php",
"src/SDK/Metrics/MetricExporter/_register.php",
"src/SDK/Propagation/_register.php",
"src/SDK/Trace/SpanExporter/_register.php",
"src/SDK/Common/Dev/Compatibility/_load.php",
"src/SDK/Common/Util/functions.php",
Expand Down
1 change: 1 addition & 0 deletions examples/autoload_sdk.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
putenv('OTEL_EXPORTER_OTLP_METRICS_PROTOCOL=grpc');
putenv('OTEL_EXPORTER_OTLP_ENDPOINT=http://collector:4317');
putenv('OTEL_PHP_TRACES_PROCESSOR=batch');
putenv('OTEL_PROPAGATORS=b3,baggage,tracecontext');

echo 'autoloading SDK example starting...' . PHP_EOL;

Expand Down
15 changes: 15 additions & 0 deletions src/API/Baggage/Propagation/BaggagePropagatorFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\API\Baggage\Propagation;

use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface;

class BaggagePropagatorFactory implements \OpenTelemetry\Context\Propagation\TextMapPropagatorFactoryInterface
{
public function create(): TextMapPropagatorInterface
{
return BaggagePropagator::getInstance();
}
}
16 changes: 16 additions & 0 deletions src/API/Trace/Propagation/TraceContextPropagatorFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\API\Trace\Propagation;

use OpenTelemetry\Context\Propagation\TextMapPropagatorFactoryInterface;
use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface;

class TraceContextPropagatorFactory implements TextMapPropagatorFactoryInterface
{
public function create(): TextMapPropagatorInterface
{
return TraceContextPropagator::getInstance();
}
}
13 changes: 13 additions & 0 deletions src/Context/Propagation/NoopTextMapPropagatorFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\Context\Propagation;

class NoopTextMapPropagatorFactory implements TextMapPropagatorFactoryInterface
{
public function create(): TextMapPropagatorInterface
{
return NoopTextMapPropagator::getInstance();
}
}
10 changes: 10 additions & 0 deletions src/Context/Propagation/TextMapPropagatorFactoryInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\Context\Propagation;

interface TextMapPropagatorFactoryInterface
{
public function create(): TextMapPropagatorInterface;
}
16 changes: 16 additions & 0 deletions src/Extension/Propagator/B3/B3MultiPropagatorFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\Extension\Propagator\B3;

use OpenTelemetry\Context\Propagation\TextMapPropagatorFactoryInterface;
use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface;

class B3MultiPropagatorFactory implements TextMapPropagatorFactoryInterface
{
public function create(): TextMapPropagatorInterface
{
return B3Propagator::getB3MultiHeaderInstance();
}
}
16 changes: 16 additions & 0 deletions src/Extension/Propagator/B3/B3SinglePropagatorFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\Extension\Propagator\B3;

use OpenTelemetry\Context\Propagation\TextMapPropagatorFactoryInterface;
use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface;

class B3SinglePropagatorFactory implements TextMapPropagatorFactoryInterface
{
public function create(): TextMapPropagatorInterface
{
return B3Propagator::getB3SingleHeaderInstance();
}
}
12 changes: 12 additions & 0 deletions src/Extension/Propagator/B3/_register.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

declare(strict_types=1);

\OpenTelemetry\SDK\FactoryRegistry::registerTextMapPropagatorFactory(
\OpenTelemetry\SDK\Common\Configuration\KnownValues::VALUE_B3,
\OpenTelemetry\Extension\Propagator\B3\B3SinglePropagatorFactory::class
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if we add a ClosureTextMapPropagatorFactory (or allow registering closure factories directly) to avoid adding a factory class for every implementation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just removed the factories altogether, and registered the propagator instances instead. It's simpler, but also adding non-factories to a FactoryRegistry which is a little shady.

Copy link
Member

@pdelewski pdelewski Dec 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, that it's simpler, but as you written it breaks rule of storing only factories there. I don't have strong opinions for or against. Maybe, we should consider some renaming as FactoryRegistry will no longer store factories only?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've renamed it to Registry, which isn't very imaginative but seems valid.

);
\OpenTelemetry\SDK\FactoryRegistry::registerTextMapPropagatorFactory(
\OpenTelemetry\SDK\Common\Configuration\KnownValues::VALUE_B3_MULTI,
\OpenTelemetry\Extension\Propagator\B3\B3MultiPropagatorFactory::class
);
5 changes: 4 additions & 1 deletion src/Extension/Propagator/B3/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
"autoload": {
"psr-4": {
"OpenTelemetry\\Extension\\Propagator\\B3\\": "."
}
},
"files": [
"_register.php"
]
}
}
34 changes: 34 additions & 0 deletions src/SDK/FactoryRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace OpenTelemetry\SDK;

use OpenTelemetry\Context\Propagation\TextMapPropagatorFactoryInterface;
use OpenTelemetry\SDK\Common\Export\TransportFactoryInterface;
use OpenTelemetry\SDK\Metrics\MetricExporterFactoryInterface;
use OpenTelemetry\SDK\Trace\SpanExporter\SpanExporterFactoryInterface;
Expand All @@ -14,6 +15,7 @@ class FactoryRegistry
private static array $spanExporterFactories = [];
private static array $transportFactories = [];
private static array $metricExporterFactories = [];
private static array $textMapPropagatorFactories = [];

/**
* @param TransportFactoryInterface|class-string<TransportFactoryInterface> $factory
Expand Down Expand Up @@ -84,6 +86,26 @@ public static function registerMetricExporterFactory(string $exporter, $factory,
self::$metricExporterFactories[$exporter] = $factory;
}

public static function registerTextMapPropagatorFactory(string $name, $factory, bool $clobber = false): void
{
if (!$clobber && array_key_exists($name, self::$textMapPropagatorFactories)) {
return;
}
if (!is_subclass_of($factory, TextMapPropagatorFactoryInterface::class)) {
trigger_error(
sprintf(
'Cannot register text map propagator factory: %s must exist and implement %s',
is_string($factory) ? $factory : get_class($factory),
TextMapPropagatorFactoryInterface::class
),
E_USER_WARNING
);

return;
}
self::$textMapPropagatorFactories[$name] = $factory;
}

public static function spanExporterFactory(string $exporter): SpanExporterFactoryInterface
{
if (!array_key_exists($exporter, self::$spanExporterFactories)) {
Expand Down Expand Up @@ -124,4 +146,16 @@ public static function metricExporterFactory(string $exporter): MetricExporterFa

return $factory;
}

public static function textMapPropagatorFactory(string $name): TextMapPropagatorFactoryInterface
{
if (!array_key_exists($name, self::$textMapPropagatorFactories)) {
throw new RuntimeException('Text map propagator factory not registered for: ' . $name);
}
$class = self::$textMapPropagatorFactories[$name];
$factory = (is_callable($class)) ? $class : new $class();
assert($factory instanceof TextMapPropagatorFactoryInterface);

return $factory;
}
}
48 changes: 11 additions & 37 deletions src/SDK/Propagation/PropagatorFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,21 @@
use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface;
use OpenTelemetry\SDK\Behavior\LogsMessagesTrait;
use OpenTelemetry\SDK\Common\Configuration\Configuration;
use OpenTelemetry\SDK\Common\Configuration\KnownValues;
use OpenTelemetry\SDK\Common\Configuration\Variables;
use OpenTelemetry\SDK\FactoryRegistry;

class PropagatorFactory
{
use LogsMessagesTrait;

private const KNOWN_PROPAGATORS = [
KnownValues::VALUE_TRACECONTEXT => ['\OpenTelemetry\API\Trace\Propagation\TraceContextPropagator', 'getInstance'],
KnownValues::VALUE_BAGGAGE => ['\OpenTelemetry\API\Baggage\Propagation\BaggagePropagator', 'getInstance'],
KnownValues::VALUE_B3 => ['\OpenTelemetry\Extension\Propagator\B3\B3Propagator', 'getB3SingleHeaderInstance'],
KnownValues::VALUE_B3_MULTI => ['\OpenTelemetry\Extension\Propagator\B3\B3Propagator', 'getB3MultiHeaderInstance'],
];

public function create(): TextMapPropagatorInterface
{
$propagators = Configuration::getList(Variables::OTEL_PROPAGATORS);
switch (count($propagators)) {
case 0:
return new NoopTextMapPropagator();
case 1:
return $this->buildPropagator($propagators[0]) ?? new NoopTextMapPropagator();
return $this->buildPropagator($propagators[0]);
default:
return new MultiTextMapPropagator($this->buildPropagators($propagators));
}
Expand All @@ -43,41 +36,22 @@ private function buildPropagators(array $names): array
{
$propagators = [];
foreach ($names as $name) {
$propagator = $this->buildPropagator($name);
if ($propagator !== null) {
$propagators[] = $propagator;
}
$propagators[] = $this->buildPropagator($name);
}

return $propagators;
}

private function buildPropagator(string $name): ?TextMapPropagatorInterface
private function buildPropagator(string $name): TextMapPropagatorInterface
{
switch ($name) {
case KnownValues::VALUE_NONE:
return null;
case KnownValues::VALUE_XRAY:
case KnownValues::VALUE_OTTRACE:
self::logWarning('Unimplemented propagator: ' . $name);

return null;
default:
if (!array_key_exists($name, self::KNOWN_PROPAGATORS)) {
self::logWarning('Unknown propagator: ' . $name);

return null;
}
$parts = self::KNOWN_PROPAGATORS[$name];

try {
return call_user_func($parts);
} catch (\Throwable $e) {
self::logError(sprintf('Unable to create %s propagator: %s', $name, $e->getMessage()));

return null;
}
try {
$factory = FactoryRegistry::textMapPropagatorFactory($name);

return $factory->create();
} catch (\RuntimeException $e) {
self::logWarning($e->getMessage());
}

return NoopTextMapPropagator::getInstance();
}
}
16 changes: 16 additions & 0 deletions src/SDK/Propagation/_register.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

declare(strict_types=1);

\OpenTelemetry\SDK\FactoryRegistry::registerTextMapPropagatorFactory(
\OpenTelemetry\SDK\Common\Configuration\KnownValues::VALUE_BAGGAGE,
\OpenTelemetry\API\Baggage\Propagation\BaggagePropagatorFactory::class
);
\OpenTelemetry\SDK\FactoryRegistry::registerTextMapPropagatorFactory(
\OpenTelemetry\SDK\Common\Configuration\KnownValues::VALUE_TRACECONTEXT,
\OpenTelemetry\API\Trace\Propagation\TraceContextPropagatorFactory::class
);
\OpenTelemetry\SDK\FactoryRegistry::registerTextMapPropagatorFactory(
\OpenTelemetry\SDK\Common\Configuration\KnownValues::VALUE_NONE,
\OpenTelemetry\Context\Propagation\NoopTextMapPropagatorFactory::class
);
1 change: 1 addition & 0 deletions src/SDK/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"files": [
"Common/Util/functions.php",
"Metrics/MetricExporter/_register.php",
"Propagation/_register.php",
"Trace/SpanExporter/_register.php",
"_autoload.php"
]
Expand Down