Skip to content

Commit 17b3c7a

Browse files
PROFeNoMbwoebi
PROFeNoM
and
bwoebi
authored
fix(otel): Missing addLink method and Fiber handling (#2849)
* fix(otel): Missing method and Fiber handling * tests: `test_opentelemetry_{1|beta}` * tests: Properly set composer to 1.0.* * fix: remove `cleanup_opentelemetry` * fix: php 7.4 unpacking issue * tests: skip fiber test on coverage * style: remove useless require * remove comment * perf: condition check * perf: Class lookup optimization Co-authored-by: Bob Weinand <[email protected]> --------- Co-authored-by: Bob Weinand <[email protected]>
1 parent 501f386 commit 17b3c7a

11 files changed

Lines changed: 223 additions & 112 deletions

File tree

‎Makefile‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -846,6 +846,7 @@ TEST_INTEGRATIONS_81 := \
846846
test_integrations_mysqli \
847847
test_integrations_openai \
848848
test_opentelemetry_1 \
849+
test_opentelemetry_beta \
849850
test_integrations_guzzle7 \
850851
test_integrations_pcntl \
851852
test_integrations_pdo \
@@ -898,6 +899,7 @@ TEST_INTEGRATIONS_82 := \
898899
test_integrations_mysqli \
899900
test_integrations_openai \
900901
test_opentelemetry_1 \
902+
test_opentelemetry_beta \
901903
test_integrations_guzzle7 \
902904
test_integrations_pcntl \
903905
test_integrations_pdo \
@@ -958,6 +960,7 @@ TEST_INTEGRATIONS_83 := \
958960
test_integrations_mysqli \
959961
test_integrations_openai \
960962
test_opentelemetry_1 \
963+
test_opentelemetry_beta \
961964
test_integrations_guzzle7 \
962965
test_integrations_pcntl \
963966
test_integrations_pdo \
@@ -1133,10 +1136,27 @@ benchmarks: benchmarks_run_dependencies call_benchmarks
11331136

11341137
benchmarks_opcache: benchmarks_run_dependencies call_benchmarks_opcache
11351138

1136-
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)
1139+
define setup_opentelemetry
1140+
cp $(1) $(dir $(1))/composer.json
1141+
endef
1142+
1143+
define run_opentelemetry_tests
11371144
$(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)
11381145
$(call run_tests,--testsuite=opentelemetry1 $(TESTS))
11391146
$(eval TEST_EXTRA_ENV=)
1147+
endef
1148+
1149+
_test_opentelemetry_beta_setup: global_test_run_dependencies
1150+
$(call setup_opentelemetry,tests/OpenTelemetry/composer-beta.json)
1151+
1152+
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)
1153+
$(call run_opentelemetry_tests)
1154+
1155+
_test_opentelemetry_1_setup: global_test_run_dependencies
1156+
$(call setup_opentelemetry,tests/OpenTelemetry/composer-1.json)
1157+
1158+
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)
1159+
$(call run_opentelemetry_tests)
11401160

11411161
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)
11421162
$(call run_tests,tests/OpenTracer1Unit)

‎src/DDTrace/OpenTelemetry/Context.php‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ final class Context implements ContextInterface
2727
/** @var ContextStorageInterface&ExecutionContextAwareInterface */
2828
private static ContextStorageInterface $storage;
2929

30+
/** @var string $storageClass */
31+
private static string $storageClass = '';
32+
3033
// Optimization for spans to avoid copying the context array.
3134
private static ContextKeyInterface $spanContextKey;
3235
private ?object $span = null;
@@ -58,8 +61,13 @@ public static function setStorage(ContextStorageInterface $storage): void
5861
*/
5962
public static function storage(): ContextStorageInterface
6063
{
61-
/** @psalm-suppress RedundantPropertyInitializationCheck */
62-
return self::$storage ??= new ContextStorage();
64+
if (self::$storageClass === '') {
65+
self::$storageClass = class_exists('OpenTelemetry\Context\FiberBoundContextStorageExecutionAwareBC')
66+
? 'OpenTelemetry\Context\FiberBoundContextStorageExecutionAwareBC' // v1.1+
67+
: 'OpenTelemetry\Context\ContextStorage';
68+
}
69+
70+
return self::$storage ??= new self::$storageClass();
6371
}
6472

6573
/**

‎src/DDTrace/OpenTelemetry/Span.php‎

Lines changed: 65 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -112,28 +112,18 @@ private function __construct(
112112
foreach ($links as $link) {
113113
/** @var LinkInterface $link */
114114
$linkContext = $link->getSpanContext();
115-
116-
$spanLink = new SpanLink();
117-
$spanLink->traceId = $linkContext->getTraceId();
118-
$spanLink->spanId = $linkContext->getSpanId();
119-
$spanLink->traceState = (string)$linkContext->getTraceState(); // __toString()
120-
$spanLink->attributes = $link->getAttributes()->toArray();
121-
$spanLink->droppedAttributesCount = 0; // Attributes limit aren't supported/meaningful in DD
122-
123-
// Save the link
124-
ObjectKVStore::put($spanLink, "link", $link);
125-
$span->links[] = $spanLink;
115+
$span->links[] = $this->createAndSaveSpanLink($linkContext, $link->getAttributes()->toArray(), $link);
126116
}
127117

128118
foreach ($events as $event) {
129119
/** @var EventInterface $event */
130120

131121
$spanEvent = new SpanEvent(
132-
$event->getName(),
122+
$event->getName(),
133123
$event->getAttributes()->toArray(),
134124
$event->getEpochNanos()
135125
);
136-
126+
137127
// Save the event
138128
ObjectKVStore::put($spanEvent, "event", $event);
139129
$span->events[] = $spanEvent;
@@ -235,17 +225,33 @@ public function toSpanData(): SpanDataInterface
235225
$this->updateSpanLinks();
236226
$this->updateSpanEvents();
237227

238-
return new ImmutableSpan(
239-
$this,
240-
$this->getName(),
241-
$this->links,
242-
$this->events,
243-
Attributes::create(array_merge($this->span->meta, $this->span->metrics)),
244-
0,
245-
StatusData::create($this->status->getCode(), $this->status->getDescription()),
246-
$hasEnded ? $this->span->getStartTime() + $this->span->getDuration() : 0,
247-
$this->hasEnded()
248-
);
228+
if (method_exists(SpanInterface::class, 'addLink')) {
229+
// v1.1 backward compatibility: totalRecordedLinks parameter added
230+
return new ImmutableSpan(
231+
$this,
232+
$this->getName(),
233+
$this->links,
234+
$this->events,
235+
Attributes::create(array_merge($this->span->meta, $this->span->metrics)),
236+
$this->totalRecordedEvents,
237+
$this->totalRecordedLinks,
238+
StatusData::create($this->status->getCode(), $this->status->getDescription()),
239+
$hasEnded ? $this->span->getStartTime() + $this->span->getDuration() : 0,
240+
$this->hasEnded(),
241+
);
242+
} else {
243+
return new ImmutableSpan(
244+
$this,
245+
$this->getName(),
246+
$this->links,
247+
$this->events,
248+
Attributes::create(array_merge($this->span->meta, $this->span->metrics)),
249+
$this->totalRecordedEvents,
250+
StatusData::create($this->status->getCode(), $this->status->getDescription()),
251+
$hasEnded ? $this->span->getStartTime() + $this->span->getDuration() : 0,
252+
$this->hasEnded(),
253+
);
254+
}
249255
}
250256

251257
/**
@@ -358,14 +364,27 @@ public function setAttributes(iterable $attributes): SpanInterface
358364
return $this;
359365
}
360366

367+
/**
368+
* @inheritDoc
369+
*/
370+
public function addLink(SpanContextInterface $context, iterable $attributes = []): SpanInterface
371+
{
372+
if ($this->hasEnded() || !$context->isValid()) {
373+
return $this;
374+
}
375+
376+
$this->span->links[] = $this->createAndSaveSpanLink($context, $attributes);
377+
return $this;
378+
}
379+
361380
/**
362381
* @inheritDoc
363382
*/
364383
public function addEvent(string $name, iterable $attributes = [], int $timestamp = null): SpanInterface
365384
{
366385
if (!$this->hasEnded()) {
367386
$this->span->events[] = new SpanEvent(
368-
$name,
387+
$name,
369388
$attributes,
370389
$timestamp ?? (int)(microtime(true) * 1e9)
371390
);
@@ -522,7 +541,7 @@ private function updateSpanEvents()
522541
{
523542
$datadogSpanEvents = $this->span->events;
524543
$this->span->meta["events"] = count($this->events);
525-
544+
526545
$otel = [];
527546
foreach ($datadogSpanEvents as $datadogSpanEvent) {
528547
$exceptionAttributes = [];
@@ -539,17 +558,35 @@ private function updateSpanEvents()
539558
$event = new Event(
540559
$datadogSpanEvent->name,
541560
(int)$datadogSpanEvent->timestamp,
542-
Attributes::create(array_merge($exceptionAttributes, \is_array($datadogSpanEvent->attributes) ? $datadogSpanEvent->attributes : iterator_to_array($datadogSpanEvent->attributes)))
561+
Attributes::create(array_merge($exceptionAttributes, \is_array($datadogSpanEvent->attributes) ? $datadogSpanEvent->attributes : iterator_to_array($datadogSpanEvent->attributes)))
543562
);
544563

545564
// Save the event
546565
ObjectKVStore::put($datadogSpanEvent, "event", $event);
547566
}
548567
$otel[] = $event;
549568
}
550-
569+
551570
// Update the events
552571
$this->events = $otel;
553572
$this->totalRecordedEvents = count($otel);
554573
}
574+
575+
private function createAndSaveSpanLink(SpanContextInterface $context, iterable $attributes = [], LinkInterface $link = null)
576+
{
577+
$spanLink = new SpanLink();
578+
$spanLink->traceId = $context->getTraceId();
579+
$spanLink->spanId = $context->getSpanId();
580+
$spanLink->traceState = (string)$context->getTraceState(); // __toString()
581+
$spanLink->attributes = $attributes;
582+
$spanLink->droppedAttributesCount = 0; // Attributes limit aren't supported/meaningful in DD
583+
584+
// Save the link
585+
if (is_null($link)) {
586+
$link = new Link($context, Attributes::create($attributes));
587+
}
588+
ObjectKVStore::put($spanLink, "link", $link);
589+
590+
return $spanLink;
591+
}
555592
}

tests/OpenTelemetry/Integration/Context/FiberTest.php renamed to tests/OpenTelemetry/Integration/Context/Fiber/FiberTest.php

Lines changed: 1 addition & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
use OpenTelemetry\API\Trace\Span;
1414
use OpenTelemetry\API\Trace\SpanKind;
1515
use OpenTelemetry\Context\Context;
16-
use OpenTelemetry\Context\ContextStorage;
16+
use OpenTelemetry\Context\ExecutionContextAwareInterface;
1717
use OpenTelemetry\SDK\Trace\TracerProvider;
1818
use function DDTrace\close_span;
1919
use function DDTrace\start_span;
@@ -31,78 +31,6 @@ public function ddSetUp(): void
3131
public function ddTearDown()
3232
{
3333
parent::ddTearDown();
34-
Context::setStorage(new ContextStorage()); // Reset OpenTelemetry context
35-
}
36-
37-
/**
38-
* @throws \Throwable
39-
*/
40-
public function test_context_switching_ffi_observer()
41-
{
42-
$key = Context::createKey('-');
43-
$scope = Context::getCurrent()
44-
->with($key, 'main')
45-
->activate();
46-
47-
$fiber = new Fiber(function () use ($key) {
48-
$scope = Context::getCurrent()
49-
->with($key, 'fiber')
50-
->activate();
51-
52-
$this->assertSame('fiber:fiber', 'fiber:' . Context::getCurrent()->get($key));
53-
54-
Fiber::suspend();
55-
56-
$this->assertSame('fiber:fiber', 'fiber:' . Context::getCurrent()->get($key));
57-
58-
$scope->detach();
59-
});
60-
61-
$this->assertSame('main:main', 'main:' . Context::getCurrent()->get($key));
62-
63-
$fiber->start();
64-
65-
$this->assertSame('main:main', 'main:' . Context::getCurrent()->get($key));
66-
67-
$fiber->resume();
68-
69-
$this->assertSame('main:main', 'main:' . Context::getCurrent()->get($key));
70-
71-
$scope->detach();
72-
}
73-
74-
public function test_context_switching_ffi_observer_registered_on_startup()
75-
{
76-
$key = Context::createKey('-');
77-
78-
$fiber = new Fiber(function () use ($key) {
79-
$scope = Context::getCurrent()
80-
->with($key, 'fiber')
81-
->activate();
82-
83-
$this->assertSame('fiber:fiber', 'fiber:' . Context::getCurrent()->get($key));
84-
85-
Fiber::suspend();
86-
87-
$this->assertSame('fiber:fiber', 'fiber:' . Context::getCurrent()->get($key));
88-
89-
$scope->detach();
90-
});
91-
92-
93-
$fiber->start();
94-
95-
$this->assertSame('main:', 'main:' . Context::getCurrent()->get($key));
96-
97-
$scope = Context::getCurrent()
98-
->with($key, 'main')
99-
->activate();
100-
101-
$fiber->resume();
102-
103-
$this->assertSame('main:main', 'main:' . Context::getCurrent()->get($key));
104-
105-
$scope->detach();
10634
}
10735

10836
public function testFiberInteroperabilityStackSwitch()

‎tests/OpenTelemetry/Integration/Context/Fiber/test_context_switching_ffi_observer.phpt‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
--TEST--
2+
Context switches on execution context switch.
3+
--SKIPIF--
4+
<?php if (PHP_VERSION_ID < 80100 || !extension_loaded('ffi') || getenv('PHPUNIT_COVERAGE')) die('skip requires PHP8.1 and FFI'); ?>
5+
--ENV--
6+
OTEL_PHP_FIBERS_ENABLED=1
7+
--FILE--
8+
<?php
9+
use OpenTelemetry\Context\Context;
10+
11+
require_once './tests/OpenTelemetry/vendor/autoload.php';
12+
13+
$key = Context::createKey('-');
14+
$scope = Context::getCurrent()
15+
->with($key, 'main')
16+
->activate();
17+
18+
$fiber = new Fiber(function() use ($key) {
19+
$scope = Context::getCurrent()
20+
->with($key, 'fiber')
21+
->activate();
22+
23+
echo 'fiber:' . Context::getCurrent()->get($key), PHP_EOL;
24+
25+
Fiber::suspend();
26+
echo 'fiber:' . Context::getCurrent()->get($key), PHP_EOL;
27+
28+
$scope->detach();
29+
});
30+
31+
echo 'main:' . Context::getCurrent()->get($key), PHP_EOL;
32+
33+
$fiber->start();
34+
echo 'main:' . Context::getCurrent()->get($key), PHP_EOL;
35+
36+
$fiber->resume();
37+
echo 'main:' . Context::getCurrent()->get($key), PHP_EOL;
38+
39+
$scope->detach();
40+
?>
41+
--EXPECT--
42+
main:main
43+
fiber:fiber
44+
main:main
45+
fiber:fiber
46+
main:main

0 commit comments

Comments
 (0)