From 19041196979e3228cc1395c82cea7740797cf2c3 Mon Sep 17 00:00:00 2001 From: Alexandre Choura Date: Thu, 12 Sep 2024 11:59:55 +0200 Subject: [PATCH 01/10] fix(otel): Missing method and Fiber handling --- src/DDTrace/OpenTelemetry/Context.php | 7 +- src/DDTrace/OpenTelemetry/Span.php | 84 ++++++++++++------- .../Context/{ => Fiber}/FiberTest.php | 74 +--------------- .../test_context_switching_ffi_observer.phpt | 46 ++++++++++ ...ng_ffi_observer_registered_on_startup.phpt | 45 ++++++++++ .../Integration/SDK/SpanBuilderTest.php | 16 ++++ tests/OpenTelemetry/composer.json | 9 +- tests/phpunit.xml | 1 + 8 files changed, 175 insertions(+), 107 deletions(-) rename tests/OpenTelemetry/Integration/Context/{ => Fiber}/FiberTest.php (75%) create mode 100644 tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer.phpt create mode 100644 tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer_registered_on_startup.phpt diff --git a/src/DDTrace/OpenTelemetry/Context.php b/src/DDTrace/OpenTelemetry/Context.php index 44910695e9f..62cdaf938f5 100644 --- a/src/DDTrace/OpenTelemetry/Context.php +++ b/src/DDTrace/OpenTelemetry/Context.php @@ -58,8 +58,11 @@ public static function setStorage(ContextStorageInterface $storage): void */ public static function storage(): ContextStorageInterface { - /** @psalm-suppress RedundantPropertyInitializationCheck */ - return self::$storage ??= new ContextStorage(); + if (class_exists('\OpenTelemetry\Context\FiberBoundContextStorageExecutionAwareBC')) { + return self::$storage ??= new FiberBoundContextStorageExecutionAwareBC(); + } else { + return self::$storage ??= new ContextStorage(); + } } /** diff --git a/src/DDTrace/OpenTelemetry/Span.php b/src/DDTrace/OpenTelemetry/Span.php index 52779a8775e..c60af54d37e 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,24 @@ 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() - ); + $spanData = [ + 'span' => $this, + 'name' => $this->getName(), + 'links' => $this->links, + 'events' => $this->events, + 'attributes' => Attributes::create(array_merge($this->span->meta, $this->span->metrics)), + 'totalRecordedEvents' => $this->totalRecordedEvents, + 'status' => StatusData::create($this->status->getCode(), $this->status->getDescription()), + 'endEpochNanos' => $hasEnded ? $this->span->getStartTime() + $this->span->getDuration() : 0, + 'hasEnded' => $this->hasEnded(), + ]; + + // Workaround for a breaking change introduced in open-telemetry/api 1.1.0 + if (in_array('addLink', get_class_methods(SpanInterface::class))) { + $spanData['totalRecordedLinks'] = $this->totalRecordedLinks; + } + + return new ImmutableSpan(...$spanData); } /** @@ -358,6 +355,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 +375,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 +532,7 @@ private function updateSpanEvents() { $datadogSpanEvents = $this->span->events; $this->span->meta["events"] = count($this->events); - + $otel = []; foreach ($datadogSpanEvents as $datadogSpanEvent) { $exceptionAttributes = []; @@ -539,7 +549,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 +557,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 55697a32fb1..cda6c178a47 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 00000000000..c86d01dd8f2 --- /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 00000000000..0d66a7e761d --- /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 b849ce7e45f..5d8a7406b71 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.json b/tests/OpenTelemetry/composer.json index 607a2888d48..17891594b80 100644 --- a/tests/OpenTelemetry/composer.json +++ b/tests/OpenTelemetry/composer.json @@ -1,8 +1,9 @@ { "name": "datadog/dd-trace-tests", "require": { - "open-telemetry/sdk": "@stable", - "open-telemetry/extension-propagator-b3": "@stable", - "open-telemetry/opentelemetry-logger-monolog": "@stable" - } + "open-telemetry/sdk": "@beta", + "open-telemetry/extension-propagator-b3": "@beta", + "open-telemetry/opentelemetry-logger-monolog": "@beta" + }, + "minimum-stability": "beta" } diff --git a/tests/phpunit.xml b/tests/phpunit.xml index fd3902de3a4..4e36afbfc62 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 From 2ce47beeaa8f43098f0be4e1b3eb758cc7614f14 Mon Sep 17 00:00:00 2001 From: Alexandre Choura Date: Fri, 13 Sep 2024 10:20:26 +0200 Subject: [PATCH 02/10] tests: `test_opentelemetry_{1|beta}` --- Makefile | 27 ++++++++++++++++++- tests/OpenTelemetry/composer-1.json | 9 +++++++ .../{composer.json => composer-beta.json} | 0 3 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 tests/OpenTelemetry/composer-1.json rename tests/OpenTelemetry/{composer.json => composer-beta.json} (100%) diff --git a/Makefile b/Makefile index 94ca7022a29..742589d84f9 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,32 @@ 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 cleanup_opentelemetry + rm tests/OpenTelemetry/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=) + $(call cleanup_opentelemetry) +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/tests/OpenTelemetry/composer-1.json b/tests/OpenTelemetry/composer-1.json new file mode 100644 index 00000000000..0c9aaa689e5 --- /dev/null +++ b/tests/OpenTelemetry/composer-1.json @@ -0,0 +1,9 @@ +{ + "name": "datadog/dd-trace-tests", + "require": { + "open-telemetry/sdk": "@stable", + "open-telemetry/extension-propagator-b3": "@stable", + "open-telemetry/opentelemetry-logger-monolog": "@stable" + }, + "minimum-stability": "stable" +} diff --git a/tests/OpenTelemetry/composer.json b/tests/OpenTelemetry/composer-beta.json similarity index 100% rename from tests/OpenTelemetry/composer.json rename to tests/OpenTelemetry/composer-beta.json From c50c2f0948097d52e0dfa03738018da766584992 Mon Sep 17 00:00:00 2001 From: Alexandre Choura Date: Fri, 13 Sep 2024 10:34:11 +0200 Subject: [PATCH 03/10] tests: Properly set composer to 1.0.* --- tests/OpenTelemetry/composer-1.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/OpenTelemetry/composer-1.json b/tests/OpenTelemetry/composer-1.json index 0c9aaa689e5..3cd307c3b1c 100644 --- a/tests/OpenTelemetry/composer-1.json +++ b/tests/OpenTelemetry/composer-1.json @@ -1,9 +1,9 @@ { "name": "datadog/dd-trace-tests", "require": { - "open-telemetry/sdk": "@stable", - "open-telemetry/extension-propagator-b3": "@stable", - "open-telemetry/opentelemetry-logger-monolog": "@stable" + "open-telemetry/sdk": "1.0.*", + "open-telemetry/extension-propagator-b3": "1.0.*", + "open-telemetry/opentelemetry-logger-monolog": "1.0.*" }, "minimum-stability": "stable" } From ae66da96be0b797cabc6be3beecaab360a9bd641 Mon Sep 17 00:00:00 2001 From: Alexandre Choura Date: Fri, 13 Sep 2024 10:37:24 +0200 Subject: [PATCH 04/10] fix: remove `cleanup_opentelemetry` --- Makefile | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Makefile b/Makefile index 742589d84f9..ef9dfec03fa 100644 --- a/Makefile +++ b/Makefile @@ -1140,15 +1140,10 @@ define setup_opentelemetry cp $(1) $(dir $(1))/composer.json endef -define cleanup_opentelemetry - rm tests/OpenTelemetry/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=) - $(call cleanup_opentelemetry) endef _test_opentelemetry_beta_setup: global_test_run_dependencies From 22a291342ed0368328665e99d437a79283436d58 Mon Sep 17 00:00:00 2001 From: Alexandre Choura Date: Fri, 13 Sep 2024 11:15:19 +0200 Subject: [PATCH 05/10] fix: php 7.4 unpacking issue --- src/DDTrace/OpenTelemetry/Span.php | 40 ++++++++++++++++++------------ 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/src/DDTrace/OpenTelemetry/Span.php b/src/DDTrace/OpenTelemetry/Span.php index c60af54d37e..d1396d328b7 100644 --- a/src/DDTrace/OpenTelemetry/Span.php +++ b/src/DDTrace/OpenTelemetry/Span.php @@ -225,24 +225,32 @@ public function toSpanData(): SpanDataInterface $this->updateSpanLinks(); $this->updateSpanEvents(); - $spanData = [ - 'span' => $this, - 'name' => $this->getName(), - 'links' => $this->links, - 'events' => $this->events, - 'attributes' => Attributes::create(array_merge($this->span->meta, $this->span->metrics)), - 'totalRecordedEvents' => $this->totalRecordedEvents, - 'status' => StatusData::create($this->status->getCode(), $this->status->getDescription()), - 'endEpochNanos' => $hasEnded ? $this->span->getStartTime() + $this->span->getDuration() : 0, - 'hasEnded' => $this->hasEnded(), - ]; - - // Workaround for a breaking change introduced in open-telemetry/api 1.1.0 if (in_array('addLink', get_class_methods(SpanInterface::class))) { - $spanData['totalRecordedLinks'] = $this->totalRecordedLinks; + 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(), + ); } - - return new ImmutableSpan(...$spanData); } /** From 1be1c34e9d98f103bb9b0614dc7cff358490cc70 Mon Sep 17 00:00:00 2001 From: Alexandre Choura Date: Fri, 13 Sep 2024 11:26:01 +0200 Subject: [PATCH 06/10] tests: skip fiber test on coverage --- .../Context/Fiber/test_context_switching_ffi_observer.phpt | 4 +++- ...context_switching_ffi_observer_registered_on_startup.phpt | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) 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 index c86d01dd8f2..80ae42f2a7f 100644 --- a/tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer.phpt +++ b/tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer.phpt @@ -1,7 +1,8 @@ --TEST-- Context switches on execution context switch. --SKIPIF-- - +# Skip if env PHPUNIT_COVERAGE is enabled + --ENV-- OTEL_PHP_FIBERS_ENABLED=1 --FILE-- @@ -9,6 +10,7 @@ OTEL_PHP_FIBERS_ENABLED=1 use OpenTelemetry\Context\Context; require_once './tests/OpenTelemetry/vendor/autoload.php'; +require_once './tests/vendor/autoload.php'; $key = Context::createKey('-'); $scope = Context::getCurrent() 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 index 0d66a7e761d..8a9262ce9ac 100644 --- 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 @@ -1,14 +1,17 @@ --TEST-- Fiber handler has to be loaded before fibers are used. --SKIPIF-- - + --ENV-- OTEL_PHP_FIBERS_ENABLED=1 +--INI-- +zend_extension=xdebug-3.2.2.so --FILE-- Date: Fri, 13 Sep 2024 11:26:25 +0200 Subject: [PATCH 07/10] style: remove useless require --- .../Context/Fiber/test_context_switching_ffi_observer.phpt | 1 - ...t_context_switching_ffi_observer_registered_on_startup.phpt | 3 --- 2 files changed, 4 deletions(-) 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 index 80ae42f2a7f..56123ac32b7 100644 --- a/tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer.phpt +++ b/tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer.phpt @@ -10,7 +10,6 @@ OTEL_PHP_FIBERS_ENABLED=1 use OpenTelemetry\Context\Context; require_once './tests/OpenTelemetry/vendor/autoload.php'; -require_once './tests/vendor/autoload.php'; $key = Context::createKey('-'); $scope = Context::getCurrent() 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 index 8a9262ce9ac..4de8eb1db98 100644 --- 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 @@ -4,14 +4,11 @@ Fiber handler has to be loaded before fibers are used. --ENV-- OTEL_PHP_FIBERS_ENABLED=1 ---INI-- -zend_extension=xdebug-3.2.2.so --FILE-- Date: Fri, 13 Sep 2024 15:15:02 +0200 Subject: [PATCH 08/10] remove comment --- .../Context/Fiber/test_context_switching_ffi_observer.phpt | 1 - 1 file changed, 1 deletion(-) 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 index 56123ac32b7..7caf00f0062 100644 --- a/tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer.phpt +++ b/tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer.phpt @@ -1,7 +1,6 @@ --TEST-- Context switches on execution context switch. --SKIPIF-- -# Skip if env PHPUNIT_COVERAGE is enabled --ENV-- OTEL_PHP_FIBERS_ENABLED=1 From 1c14c3bcb572a93492209819efd850db943116e3 Mon Sep 17 00:00:00 2001 From: Alexandre Choura Date: Mon, 16 Sep 2024 11:17:56 +0200 Subject: [PATCH 09/10] perf: condition check --- src/DDTrace/OpenTelemetry/Context.php | 13 +++++++++---- src/DDTrace/OpenTelemetry/Span.php | 3 ++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/DDTrace/OpenTelemetry/Context.php b/src/DDTrace/OpenTelemetry/Context.php index 62cdaf938f5..f8fddd5c445 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,11 +61,13 @@ public static function setStorage(ContextStorageInterface $storage): void */ public static function storage(): ContextStorageInterface { - if (class_exists('\OpenTelemetry\Context\FiberBoundContextStorageExecutionAwareBC')) { - return self::$storage ??= new FiberBoundContextStorageExecutionAwareBC(); - } else { - 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 d1396d328b7..c6aa4ae95e9 100644 --- a/src/DDTrace/OpenTelemetry/Span.php +++ b/src/DDTrace/OpenTelemetry/Span.php @@ -225,7 +225,8 @@ public function toSpanData(): SpanDataInterface $this->updateSpanLinks(); $this->updateSpanEvents(); - if (in_array('addLink', get_class_methods(SpanInterface::class))) { + if (method_exists(SpanInterface::class, 'addLink')) { + // v1.1 backward compatibility: totalRecordedLinks parameter added return new ImmutableSpan( $this, $this->getName(), From e44f4e456c07253aff91cad77cbc68921fadb820 Mon Sep 17 00:00:00 2001 From: Alexandre Choura <42672104+PROFeNoM@users.noreply.github.com> Date: Mon, 16 Sep 2024 13:17:58 +0200 Subject: [PATCH 10/10] perf: Class lookup optimization Co-authored-by: Bob Weinand --- src/DDTrace/OpenTelemetry/Context.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/DDTrace/OpenTelemetry/Context.php b/src/DDTrace/OpenTelemetry/Context.php index f8fddd5c445..a05f6b1445c 100644 --- a/src/DDTrace/OpenTelemetry/Context.php +++ b/src/DDTrace/OpenTelemetry/Context.php @@ -62,9 +62,9 @@ public static function setStorage(ContextStorageInterface $storage): void public static function storage(): ContextStorageInterface { if (self::$storageClass === '') { - self::$storageClass = class_exists('\OpenTelemetry\Context\FiberBoundContextStorageExecutionAwareBC') - ? '\OpenTelemetry\Context\FiberBoundContextStorageExecutionAwareBC' // v1.1+ - : '\OpenTelemetry\Context\ContextStorage'; + self::$storageClass = class_exists('OpenTelemetry\Context\FiberBoundContextStorageExecutionAwareBC') + ? 'OpenTelemetry\Context\FiberBoundContextStorageExecutionAwareBC' // v1.1+ + : 'OpenTelemetry\Context\ContextStorage'; } return self::$storage ??= new self::$storageClass();