diff --git a/Makefile b/Makefile index 94ca7022a2..ef9dfec03f 100644 --- a/Makefile +++ b/Makefile @@ -846,6 +846,7 @@ TEST_INTEGRATIONS_81 := \ test_integrations_mysqli \ test_integrations_openai \ test_opentelemetry_1 \ + test_opentelemetry_beta \ test_integrations_guzzle7 \ test_integrations_pcntl \ test_integrations_pdo \ @@ -898,6 +899,7 @@ TEST_INTEGRATIONS_82 := \ test_integrations_mysqli \ test_integrations_openai \ test_opentelemetry_1 \ + test_opentelemetry_beta \ test_integrations_guzzle7 \ test_integrations_pcntl \ test_integrations_pdo \ @@ -958,6 +960,7 @@ TEST_INTEGRATIONS_83 := \ test_integrations_mysqli \ test_integrations_openai \ test_opentelemetry_1 \ + test_opentelemetry_beta \ test_integrations_guzzle7 \ test_integrations_pcntl \ test_integrations_pdo \ @@ -1133,10 +1136,27 @@ benchmarks: benchmarks_run_dependencies call_benchmarks benchmarks_opcache: benchmarks_run_dependencies call_benchmarks_opcache -test_opentelemetry_1: global_test_run_dependencies tests/Frameworks/Custom/OpenTelemetry/composer.lock-php$(PHP_MAJOR_MINOR) tests/OpenTelemetry/composer.lock-php$(PHP_MAJOR_MINOR) +define setup_opentelemetry + cp $(1) $(dir $(1))/composer.json +endef + +define run_opentelemetry_tests $(eval TEST_EXTRA_ENV=$(shell [ $(PHP_MAJOR_MINOR) -ge 81 ] && echo "OTEL_PHP_FIBERS_ENABLED=1" || echo '') DD_TRACE_OTEL_ENABLED=1 DD_TRACE_GENERATE_ROOT_SPAN=0) $(call run_tests,--testsuite=opentelemetry1 $(TESTS)) $(eval TEST_EXTRA_ENV=) +endef + +_test_opentelemetry_beta_setup: global_test_run_dependencies + $(call setup_opentelemetry,tests/OpenTelemetry/composer-beta.json) + +test_opentelemetry_beta: _test_opentelemetry_beta_setup tests/Frameworks/Custom/OpenTelemetry/composer.lock-php$(PHP_MAJOR_MINOR) tests/OpenTelemetry/composer.lock-php$(PHP_MAJOR_MINOR) + $(call run_opentelemetry_tests) + +_test_opentelemetry_1_setup: global_test_run_dependencies + $(call setup_opentelemetry,tests/OpenTelemetry/composer-1.json) + +test_opentelemetry_1: _test_opentelemetry_1_setup tests/Frameworks/Custom/OpenTelemetry/composer.lock-php$(PHP_MAJOR_MINOR) tests/OpenTelemetry/composer.lock-php$(PHP_MAJOR_MINOR) + $(call run_opentelemetry_tests) test_opentracing_10: global_test_run_dependencies tests/OpenTracer1Unit/composer.lock-php$(PHP_MAJOR_MINOR) tests/Frameworks/Custom/OpenTracing/composer.lock-php$(PHP_MAJOR_MINOR) $(call run_tests,tests/OpenTracer1Unit) diff --git a/src/DDTrace/OpenTelemetry/Context.php b/src/DDTrace/OpenTelemetry/Context.php index 44910695e9..a05f6b1445 100644 --- a/src/DDTrace/OpenTelemetry/Context.php +++ b/src/DDTrace/OpenTelemetry/Context.php @@ -27,6 +27,9 @@ final class Context implements ContextInterface /** @var ContextStorageInterface&ExecutionContextAwareInterface */ private static ContextStorageInterface $storage; + /** @var string $storageClass */ + private static string $storageClass = ''; + // Optimization for spans to avoid copying the context array. private static ContextKeyInterface $spanContextKey; private ?object $span = null; @@ -58,8 +61,13 @@ public static function setStorage(ContextStorageInterface $storage): void */ public static function storage(): ContextStorageInterface { - /** @psalm-suppress RedundantPropertyInitializationCheck */ - return self::$storage ??= new ContextStorage(); + if (self::$storageClass === '') { + self::$storageClass = class_exists('OpenTelemetry\Context\FiberBoundContextStorageExecutionAwareBC') + ? 'OpenTelemetry\Context\FiberBoundContextStorageExecutionAwareBC' // v1.1+ + : 'OpenTelemetry\Context\ContextStorage'; + } + + return self::$storage ??= new self::$storageClass(); } /** diff --git a/src/DDTrace/OpenTelemetry/Span.php b/src/DDTrace/OpenTelemetry/Span.php index 52779a8775..c6aa4ae95e 100644 --- a/src/DDTrace/OpenTelemetry/Span.php +++ b/src/DDTrace/OpenTelemetry/Span.php @@ -112,28 +112,18 @@ private function __construct( foreach ($links as $link) { /** @var LinkInterface $link */ $linkContext = $link->getSpanContext(); - - $spanLink = new SpanLink(); - $spanLink->traceId = $linkContext->getTraceId(); - $spanLink->spanId = $linkContext->getSpanId(); - $spanLink->traceState = (string)$linkContext->getTraceState(); // __toString() - $spanLink->attributes = $link->getAttributes()->toArray(); - $spanLink->droppedAttributesCount = 0; // Attributes limit aren't supported/meaningful in DD - - // Save the link - ObjectKVStore::put($spanLink, "link", $link); - $span->links[] = $spanLink; + $span->links[] = $this->createAndSaveSpanLink($linkContext, $link->getAttributes()->toArray(), $link); } foreach ($events as $event) { /** @var EventInterface $event */ $spanEvent = new SpanEvent( - $event->getName(), + $event->getName(), $event->getAttributes()->toArray(), $event->getEpochNanos() ); - + // Save the event ObjectKVStore::put($spanEvent, "event", $event); $span->events[] = $spanEvent; @@ -235,17 +225,33 @@ public function toSpanData(): SpanDataInterface $this->updateSpanLinks(); $this->updateSpanEvents(); - return new ImmutableSpan( - $this, - $this->getName(), - $this->links, - $this->events, - Attributes::create(array_merge($this->span->meta, $this->span->metrics)), - 0, - StatusData::create($this->status->getCode(), $this->status->getDescription()), - $hasEnded ? $this->span->getStartTime() + $this->span->getDuration() : 0, - $this->hasEnded() - ); + if (method_exists(SpanInterface::class, 'addLink')) { + // v1.1 backward compatibility: totalRecordedLinks parameter added + return new ImmutableSpan( + $this, + $this->getName(), + $this->links, + $this->events, + Attributes::create(array_merge($this->span->meta, $this->span->metrics)), + $this->totalRecordedEvents, + $this->totalRecordedLinks, + StatusData::create($this->status->getCode(), $this->status->getDescription()), + $hasEnded ? $this->span->getStartTime() + $this->span->getDuration() : 0, + $this->hasEnded(), + ); + } else { + return new ImmutableSpan( + $this, + $this->getName(), + $this->links, + $this->events, + Attributes::create(array_merge($this->span->meta, $this->span->metrics)), + $this->totalRecordedEvents, + StatusData::create($this->status->getCode(), $this->status->getDescription()), + $hasEnded ? $this->span->getStartTime() + $this->span->getDuration() : 0, + $this->hasEnded(), + ); + } } /** @@ -358,6 +364,19 @@ public function setAttributes(iterable $attributes): SpanInterface return $this; } + /** + * @inheritDoc + */ + public function addLink(SpanContextInterface $context, iterable $attributes = []): SpanInterface + { + if ($this->hasEnded() || !$context->isValid()) { + return $this; + } + + $this->span->links[] = $this->createAndSaveSpanLink($context, $attributes); + return $this; + } + /** * @inheritDoc */ @@ -365,7 +384,7 @@ public function addEvent(string $name, iterable $attributes = [], int $timestamp { if (!$this->hasEnded()) { $this->span->events[] = new SpanEvent( - $name, + $name, $attributes, $timestamp ?? (int)(microtime(true) * 1e9) ); @@ -522,7 +541,7 @@ private function updateSpanEvents() { $datadogSpanEvents = $this->span->events; $this->span->meta["events"] = count($this->events); - + $otel = []; foreach ($datadogSpanEvents as $datadogSpanEvent) { $exceptionAttributes = []; @@ -539,7 +558,7 @@ private function updateSpanEvents() $event = new Event( $datadogSpanEvent->name, (int)$datadogSpanEvent->timestamp, - Attributes::create(array_merge($exceptionAttributes, \is_array($datadogSpanEvent->attributes) ? $datadogSpanEvent->attributes : iterator_to_array($datadogSpanEvent->attributes))) + Attributes::create(array_merge($exceptionAttributes, \is_array($datadogSpanEvent->attributes) ? $datadogSpanEvent->attributes : iterator_to_array($datadogSpanEvent->attributes))) ); // Save the event @@ -547,9 +566,27 @@ private function updateSpanEvents() } $otel[] = $event; } - + // Update the events $this->events = $otel; $this->totalRecordedEvents = count($otel); } + + private function createAndSaveSpanLink(SpanContextInterface $context, iterable $attributes = [], LinkInterface $link = null) + { + $spanLink = new SpanLink(); + $spanLink->traceId = $context->getTraceId(); + $spanLink->spanId = $context->getSpanId(); + $spanLink->traceState = (string)$context->getTraceState(); // __toString() + $spanLink->attributes = $attributes; + $spanLink->droppedAttributesCount = 0; // Attributes limit aren't supported/meaningful in DD + + // Save the link + if (is_null($link)) { + $link = new Link($context, Attributes::create($attributes)); + } + ObjectKVStore::put($spanLink, "link", $link); + + return $spanLink; + } } diff --git a/tests/OpenTelemetry/Integration/Context/FiberTest.php b/tests/OpenTelemetry/Integration/Context/Fiber/FiberTest.php similarity index 75% rename from tests/OpenTelemetry/Integration/Context/FiberTest.php rename to tests/OpenTelemetry/Integration/Context/Fiber/FiberTest.php index 55697a32fb..cda6c178a4 100644 --- a/tests/OpenTelemetry/Integration/Context/FiberTest.php +++ b/tests/OpenTelemetry/Integration/Context/Fiber/FiberTest.php @@ -13,7 +13,7 @@ use OpenTelemetry\API\Trace\Span; use OpenTelemetry\API\Trace\SpanKind; use OpenTelemetry\Context\Context; -use OpenTelemetry\Context\ContextStorage; +use OpenTelemetry\Context\ExecutionContextAwareInterface; use OpenTelemetry\SDK\Trace\TracerProvider; use function DDTrace\close_span; use function DDTrace\start_span; @@ -31,78 +31,6 @@ public function ddSetUp(): void public function ddTearDown() { parent::ddTearDown(); - Context::setStorage(new ContextStorage()); // Reset OpenTelemetry context - } - - /** - * @throws \Throwable - */ - public function test_context_switching_ffi_observer() - { - $key = Context::createKey('-'); - $scope = Context::getCurrent() - ->with($key, 'main') - ->activate(); - - $fiber = new Fiber(function () use ($key) { - $scope = Context::getCurrent() - ->with($key, 'fiber') - ->activate(); - - $this->assertSame('fiber:fiber', 'fiber:' . Context::getCurrent()->get($key)); - - Fiber::suspend(); - - $this->assertSame('fiber:fiber', 'fiber:' . Context::getCurrent()->get($key)); - - $scope->detach(); - }); - - $this->assertSame('main:main', 'main:' . Context::getCurrent()->get($key)); - - $fiber->start(); - - $this->assertSame('main:main', 'main:' . Context::getCurrent()->get($key)); - - $fiber->resume(); - - $this->assertSame('main:main', 'main:' . Context::getCurrent()->get($key)); - - $scope->detach(); - } - - public function test_context_switching_ffi_observer_registered_on_startup() - { - $key = Context::createKey('-'); - - $fiber = new Fiber(function () use ($key) { - $scope = Context::getCurrent() - ->with($key, 'fiber') - ->activate(); - - $this->assertSame('fiber:fiber', 'fiber:' . Context::getCurrent()->get($key)); - - Fiber::suspend(); - - $this->assertSame('fiber:fiber', 'fiber:' . Context::getCurrent()->get($key)); - - $scope->detach(); - }); - - - $fiber->start(); - - $this->assertSame('main:', 'main:' . Context::getCurrent()->get($key)); - - $scope = Context::getCurrent() - ->with($key, 'main') - ->activate(); - - $fiber->resume(); - - $this->assertSame('main:main', 'main:' . Context::getCurrent()->get($key)); - - $scope->detach(); } public function testFiberInteroperabilityStackSwitch() diff --git a/tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer.phpt b/tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer.phpt new file mode 100644 index 0000000000..7caf00f006 --- /dev/null +++ b/tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer.phpt @@ -0,0 +1,46 @@ +--TEST-- +Context switches on execution context switch. +--SKIPIF-- + +--ENV-- +OTEL_PHP_FIBERS_ENABLED=1 +--FILE-- +with($key, 'main') + ->activate(); + +$fiber = new Fiber(function() use ($key) { + $scope = Context::getCurrent() + ->with($key, 'fiber') + ->activate(); + + echo 'fiber:' . Context::getCurrent()->get($key), PHP_EOL; + + Fiber::suspend(); + echo 'fiber:' . Context::getCurrent()->get($key), PHP_EOL; + + $scope->detach(); +}); + +echo 'main:' . Context::getCurrent()->get($key), PHP_EOL; + +$fiber->start(); +echo 'main:' . Context::getCurrent()->get($key), PHP_EOL; + +$fiber->resume(); +echo 'main:' . Context::getCurrent()->get($key), PHP_EOL; + +$scope->detach(); +?> +--EXPECT-- +main:main +fiber:fiber +main:main +fiber:fiber +main:main diff --git a/tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer_registered_on_startup.phpt b/tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer_registered_on_startup.phpt new file mode 100644 index 0000000000..4de8eb1db9 --- /dev/null +++ b/tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer_registered_on_startup.phpt @@ -0,0 +1,45 @@ +--TEST-- +Fiber handler has to be loaded before fibers are used. +--SKIPIF-- + +--ENV-- +OTEL_PHP_FIBERS_ENABLED=1 +--FILE-- +with($key, 'fiber') + ->activate(); + + echo 'fiber:' . Context::getCurrent()->get($key), PHP_EOL; + + Fiber::suspend(); + echo 'fiber:' . Context::getCurrent()->get($key), PHP_EOL; + + $scope->detach(); +}); + +$fiber->start(); +echo 'main:' . Context::getCurrent()->get($key), PHP_EOL; + +$scope = Context::getCurrent() + ->with($key, 'main') + ->activate(); + +$fiber->resume(); +echo 'main:' . Context::getCurrent()->get($key), PHP_EOL; + +$scope->detach(); + +?> +--EXPECT-- +fiber:fiber +main: +fiber:fiber +main:main diff --git a/tests/OpenTelemetry/Integration/SDK/SpanBuilderTest.php b/tests/OpenTelemetry/Integration/SDK/SpanBuilderTest.php index b849ce7e45..5d8a7406b7 100644 --- a/tests/OpenTelemetry/Integration/SDK/SpanBuilderTest.php +++ b/tests/OpenTelemetry/Integration/SDK/SpanBuilderTest.php @@ -109,6 +109,22 @@ public function test_add_link(): void $this->assertCount(2, $span->toSpanData()->getLinks()); } + /** + * @group trace-compliance + */ + public function test_add_link_after_span_creation(): void + { + /** @var Span $span */ + $span = $this + ->tracer + ->spanBuilder(self::SPAN_NAME) + ->addLink($this->sampledSpanContext) + ->startSpan() + ->addLink($this->sampledSpanContext); + + $this->assertCount(2, $span->toSpanData()->getLinks()); + } + public function test_add_link_invalid(): void { /** @var Span $span */ diff --git a/tests/OpenTelemetry/composer-1.json b/tests/OpenTelemetry/composer-1.json new file mode 100644 index 0000000000..3cd307c3b1 --- /dev/null +++ b/tests/OpenTelemetry/composer-1.json @@ -0,0 +1,9 @@ +{ + "name": "datadog/dd-trace-tests", + "require": { + "open-telemetry/sdk": "1.0.*", + "open-telemetry/extension-propagator-b3": "1.0.*", + "open-telemetry/opentelemetry-logger-monolog": "1.0.*" + }, + "minimum-stability": "stable" +} diff --git a/tests/OpenTelemetry/composer-beta.json b/tests/OpenTelemetry/composer-beta.json new file mode 100644 index 0000000000..17891594b8 --- /dev/null +++ b/tests/OpenTelemetry/composer-beta.json @@ -0,0 +1,9 @@ +{ + "name": "datadog/dd-trace-tests", + "require": { + "open-telemetry/sdk": "@beta", + "open-telemetry/extension-propagator-b3": "@beta", + "open-telemetry/opentelemetry-logger-monolog": "@beta" + }, + "minimum-stability": "beta" +} diff --git a/tests/OpenTelemetry/composer.json b/tests/OpenTelemetry/composer.json deleted file mode 100644 index 607a2888d4..0000000000 --- a/tests/OpenTelemetry/composer.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "name": "datadog/dd-trace-tests", - "require": { - "open-telemetry/sdk": "@stable", - "open-telemetry/extension-propagator-b3": "@stable", - "open-telemetry/opentelemetry-logger-monolog": "@stable" - } -} diff --git a/tests/phpunit.xml b/tests/phpunit.xml index fd3902de3a..4e36afbfc6 100644 --- a/tests/phpunit.xml +++ b/tests/phpunit.xml @@ -74,6 +74,7 @@ ./Integrations/Laravel/Octane + ./OpenTelemetry/Integration/Context/Fiber ./OpenTelemetry/Unit/API ./OpenTelemetry/Unit/Context ./OpenTelemetry/Unit/Propagation