Skip to content

Commit 7490016

Browse files
jerrytfleungjerrytfleung
authored
feat: Implement onEnding interface of Span Processor in Trace SDK (#1785)
* Updated * Fixed * nits * Backport-ed the 2.x Span::end() implementation #1483
1 parent f94a880 commit 7490016

File tree

10 files changed

+257
-15
lines changed

10 files changed

+257
-15
lines changed

‎composer.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@
8787
}
8888
},
8989
"require-dev": {
90+
"amphp/amp": "^3",
9091
"ext-grpc": "*",
9192
"bamarni/composer-bin-plugin": "^1.8",
9293
"dg/bypass-finals": "^1.4",

‎src/SDK/Trace/ExtendedSpanProcessorInterface.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace OpenTelemetry\SDK\Trace;
6+
7+
/**
8+
* @experimental
9+
*/
10+
interface ExtendedSpanProcessorInterface extends SpanProcessorInterface
11+
{
12+
/**
13+
* @see https://github.com/open-telemetry/opentelemetry-specification/blob/v1.51.0/specification/trace/sdk.md#onending
14+
*/
15+
public function onEnding(ReadWriteSpanInterface $span): void;
16+
}

‎src/SDK/Trace/Span.php‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ final class Span extends API\Span implements ReadWriteSpanInterface
2626
private array $events = [];
2727
private int $totalRecordedEvents = 0;
2828
private StatusDataInterface $status;
29-
private int $endEpochNanos = 0;
29+
private ?int $endEpochNanos = null;
3030
private bool $hasEnded = false;
3131

3232
/**
@@ -268,16 +268,20 @@ public function setStatus(string $code, ?string $description = null): self
268268
#[\Override]
269269
public function end(?int $endEpochNanos = null): void
270270
{
271-
if ($this->hasEnded) {
271+
if ($this->endEpochNanos !== null) {
272272
return;
273273
}
274274

275275
$this->endEpochNanos = $endEpochNanos ?? Clock::getDefault()->now();
276-
$this->hasEnded = true;
277-
278-
$this->checkForDroppedElements();
276+
$span = clone $this;
277+
$this->hasEnded = true; // prevent further modifications to the span by async code
278+
if ($this->spanProcessor instanceof ExtendedSpanProcessorInterface) {
279+
$this->spanProcessor->onEnding($span);
280+
}
281+
$span->hasEnded = true;
279282

280-
$this->spanProcessor->onEnd($this);
283+
$this->spanProcessor->onEnd($span);
284+
$span->checkForDroppedElements();
281285
}
282286

283287
/** @inheritDoc */
@@ -317,7 +321,7 @@ public function toSpanData(): SpanDataInterface
317321
$this->totalRecordedLinks,
318322
$this->totalRecordedEvents,
319323
$this->status,
320-
$this->endEpochNanos,
324+
$this->endEpochNanos ?? 0,
321325
$this->hasEnded
322326
);
323327
}

‎src/SDK/Trace/SpanProcessor/MultiSpanProcessor.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use OpenTelemetry\Context\ContextInterface;
88
use OpenTelemetry\SDK\Common\Future\CancellationInterface;
9+
use OpenTelemetry\SDK\Trace\ExtendedSpanProcessorInterface;
910
use OpenTelemetry\SDK\Trace\ReadableSpanInterface;
1011
use OpenTelemetry\SDK\Trace\ReadWriteSpanInterface;
1112
use OpenTelemetry\SDK\Trace\SpanProcessorInterface;
@@ -14,7 +15,7 @@
1415
* Class SpanMultiProcessor is a SpanProcessor that forwards all events to an
1516
* array of SpanProcessors.
1617
*/
17-
final class MultiSpanProcessor implements SpanProcessorInterface
18+
final class MultiSpanProcessor implements ExtendedSpanProcessorInterface
1819
{
1920
/** @var list<SpanProcessorInterface> */
2021
private array $processors = [];
@@ -46,6 +47,17 @@ public function onStart(ReadWriteSpanInterface $span, ContextInterface $parentCo
4647
}
4748
}
4849

50+
/** @inheritDoc */
51+
#[\Override]
52+
public function onEnding(ReadWriteSpanInterface $span): void
53+
{
54+
foreach ($this->processors as $processor) {
55+
if ($processor instanceof ExtendedSpanProcessorInterface) {
56+
$processor->onEnding($span);
57+
}
58+
}
59+
}
60+
4961
/** @inheritDoc */
5062
#[\Override]
5163
public function onEnd(ReadableSpanInterface $span): void

‎tests/Integration/SDK/Trace/test_async_cannot_modify_span_during_onending.phpt‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
--TEST--
2+
Test that async code cannot update span during OnEnding
3+
--DESCRIPTION--
4+
The SDK MUST guarantee that the span can no longer be modified by any other thread before invoking OnEnding of the first SpanProcessor.
5+
--FILE--
6+
<?php
7+
require_once 'vendor/autoload.php';
8+
9+
use OpenTelemetry\SDK\Trace\TracerProviderBuilder;
10+
use OpenTelemetry\SDK\Trace\ReadWriteSpanInterface;
11+
use OpenTelemetry\SDK\Trace\ReadableSpanInterface;
12+
use OpenTelemetry\SDK\Common\Future\CancellationInterface;
13+
use OpenTelemetry\SDK\Trace\ExtendedSpanProcessorInterface;
14+
use OpenTelemetry\Context\ContextInterface;
15+
16+
$tracerProvider = (new TracerProviderBuilder())
17+
->addSpanProcessor(new class implements ExtendedSpanProcessorInterface {
18+
#[\Override]
19+
public function onEnding(ReadWriteSpanInterface $span): void {
20+
$spanName = $span->getName();
21+
22+
Amp\delay(0);
23+
assert($span->getName() === $spanName);
24+
}
25+
#[\Override]
26+
public function onStart(ReadWriteSpanInterface $span, ContextInterface $parentContext): void {}
27+
#[\Override]
28+
public function onEnd(ReadableSpanInterface $span): void {}
29+
#[\Override]
30+
public function forceFlush(?CancellationInterface $cancellation = null): bool { return true; }
31+
#[\Override]
32+
public function shutdown(?CancellationInterface $cancellation = null): bool { return true; }
33+
})
34+
->build();
35+
36+
$span = $tracerProvider->getTracer('test')->spanBuilder('span')->startSpan();
37+
Amp\async($span->updateName(...), 'should-not-update');
38+
$span->end();
39+
?>
40+
--EXPECT--

‎tests/Integration/SDK/Trace/test_end_span_in_onending.phpt‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
--TEST--
2+
Test ending a span during OnEnding
3+
--FILE--
4+
<?php
5+
require_once 'vendor/autoload.php';
6+
7+
use OpenTelemetry\SDK\Trace\TracerProviderBuilder;
8+
use OpenTelemetry\SDK\Trace\ReadWriteSpanInterface;
9+
use OpenTelemetry\SDK\Trace\ReadableSpanInterface;
10+
use OpenTelemetry\SDK\Common\Future\CancellationInterface;
11+
use OpenTelemetry\SDK\Trace\ExtendedSpanProcessorInterface;
12+
use OpenTelemetry\Context\ContextInterface;
13+
14+
$tracerProvider = (new TracerProviderBuilder())
15+
->addSpanProcessor(new class implements ExtendedSpanProcessorInterface {
16+
#[\Override]
17+
public function onEnding(ReadWriteSpanInterface $span): void {
18+
$span->end();
19+
}
20+
#[\Override]
21+
public function onStart(ReadWriteSpanInterface $span, ContextInterface $parentContext): void {}
22+
#[\Override]
23+
public function onEnd(ReadableSpanInterface $span): void {}
24+
#[\Override]
25+
public function forceFlush(?CancellationInterface $cancellation = null): bool { return true; }
26+
#[\Override]
27+
public function shutdown(?CancellationInterface $cancellation = null): bool { return true; }
28+
})
29+
->build();
30+
31+
$span = $tracerProvider->getTracer('test')->spanBuilder('span')->startSpan();
32+
$span->end();
33+
?>
34+
--EXPECT--

‎tests/Integration/SDK/Trace/test_mutate_span_in_onending.phpt‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
--TEST--
2+
Test ending a span during OnEnding
3+
--FILE--
4+
<?php
5+
require_once 'vendor/autoload.php';
6+
7+
use OpenTelemetry\SDK\Trace\TracerProviderBuilder;
8+
use OpenTelemetry\SDK\Trace\ReadWriteSpanInterface;
9+
use OpenTelemetry\SDK\Trace\ReadableSpanInterface;
10+
use OpenTelemetry\SDK\Common\Future\CancellationInterface;
11+
use OpenTelemetry\SDK\Trace\ExtendedSpanProcessorInterface;
12+
use OpenTelemetry\Context\ContextInterface;
13+
14+
$exporter = (new \OpenTelemetry\SDK\Trace\SpanExporter\ConsoleSpanExporterFactory())->create();
15+
16+
$processor = new class($exporter) extends ExtendedSpanProcessorInterface {
17+
#[\Override]
18+
public function onEnding(ReadWriteSpanInterface $span): void {
19+
$span->updateName('updated');
20+
$span->setAttribute('new-key', 'new-value');
21+
$span->addEvent('new-event');
22+
}
23+
};
24+
25+
$tracerProvider = (new TracerProviderBuilder())
26+
->addSpanProcessor($processor)
27+
->build();
28+
29+
$span = $tracerProvider->getTracer('test')->spanBuilder('span')->startSpan();
30+
$span->end();
31+
?>
32+
--EXPECTF--
33+
[
34+
{
35+
"name": "updated",
36+
"context": {
37+
"trace_id": "%s",
38+
"span_id": "%s",
39+
"trace_state": "",
40+
"trace_flags": 1
41+
},
42+
"resource": {
43+
"telemetry.sdk.name": "opentelemetry",
44+
"telemetry.sdk.language": "php",
45+
"telemetry.sdk.version": "%s",
46+
"telemetry.distro.name": "opentelemetry-php-instrumentation",
47+
"telemetry.distro.version": "%s",
48+
"service.name": "%s"
49+
},
50+
"parent_span_id": "",
51+
"kind": "KIND_INTERNAL",
52+
"start": %d,
53+
"end": %d,
54+
"attributes": {
55+
"new-key": "new-value"
56+
},
57+
"status": {
58+
"code": "Unset",
59+
"description": ""
60+
},
61+
"events": [
62+
{
63+
"name": "new-event",
64+
"timestamp": %d,
65+
"attributes": []
66+
}
67+
],
68+
"links": [],
69+
"schema_url": ""
70+
}
71+
]'

‎tests/Integration/SDK/TracerTest.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,14 @@
1010
use OpenTelemetry\API\Trace\TraceState;
1111
use OpenTelemetry\Context\Context;
1212
use OpenTelemetry\SDK\Common\Configuration\Variables;
13+
use OpenTelemetry\SDK\Trace\ExtendedSpanProcessorInterface;
14+
use OpenTelemetry\SDK\Trace\ReadWriteSpanInterface;
1315
use OpenTelemetry\SDK\Trace\Sampler\AlwaysOffSampler;
1416
use OpenTelemetry\SDK\Trace\SamplerInterface;
1517
use OpenTelemetry\SDK\Trace\SamplingResult;
1618
use OpenTelemetry\SDK\Trace\Span;
1719
use OpenTelemetry\SDK\Trace\SpanBuilder;
20+
use OpenTelemetry\SDK\Trace\SpanProcessor\MultiSpanProcessor;
1821
use OpenTelemetry\SDK\Trace\SpanProcessorInterface;
1922
use OpenTelemetry\SDK\Trace\TracerProvider;
2023
use OpenTelemetry\SDK\Trace\TracerProviderFactory;
@@ -105,4 +108,29 @@ public function test_factory_returns_noop_tracer_when_sdk_disabled(): void
105108
$tracer = $tracerProvider->getTracer('foo');
106109
$this->assertInstanceOf(API\NoopTracer::class, $tracer);
107110
}
111+
112+
#[Group('trace-compliance')]
113+
/**
114+
* The Span object MUST still be mutable (i.e., SetAttribute, AddLink, AddEvent can be called) while OnEnding is called.
115+
*/
116+
public function test_span_processor_onending_can_mutate_span(): void
117+
{
118+
$one = $this->createMock(ExtendedSpanProcessorInterface::class);
119+
$one->expects($this->once())->method('onEnding')->willReturnCallback(function (ReadWriteSpanInterface $span) {
120+
$span->setAttribute('foo', 'bar');
121+
$this->assertCount(0, $span->toSpanData()->getEvents());
122+
$span->addEvent('new-event', ['baz' => 'bat']);
123+
$span->updateName('updated');
124+
});
125+
$two = $this->createMock(ExtendedSpanProcessorInterface::class);
126+
$two->expects($this->once())->method('onEnding')->willReturnCallback(function (ReadWriteSpanInterface $span) {
127+
$this->assertSame('updated', $span->getName());
128+
$this->assertCount(1, $span->toSpanData()->getEvents());
129+
$this->assertSame('bar', $span->getAttribute('foo'));
130+
});
131+
$multi = new MultiSpanProcessor($one, $two);
132+
$tracerProvider = new TracerProvider($multi);
133+
$span = $tracerProvider->getTracer('test')->spanBuilder('test')->startSpan();
134+
$span->end();
135+
}
108136
}

‎tests/Unit/SDK/Trace/SpanProcessor/MultiSpanProcessorTest.php‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace OpenTelemetry\Tests\Unit\SDK\Trace\SpanProcessor;
66

77
use OpenTelemetry\Context\Context;
8+
use OpenTelemetry\SDK\Trace\ExtendedSpanProcessorInterface;
89
use OpenTelemetry\SDK\Trace\ReadableSpanInterface;
910
use OpenTelemetry\SDK\Trace\ReadWriteSpanInterface;
1011
use OpenTelemetry\SDK\Trace\SpanProcessor\MultiSpanProcessor;
@@ -31,9 +32,12 @@ public function test_add_span_processor(): void
3132
$multiProcessor = $this->createMultiSpanProcessor();
3233
$processor = $this->createMock(SpanProcessorInterface::class);
3334
$multiProcessor->addSpanProcessor($processor);
35+
$extendedProcessor = $this->createMock(ExtendedSpanProcessorInterface::class);
36+
$multiProcessor->addSpanProcessor($extendedProcessor);
3437
$processors = array_merge(
38+
$this->getSpanProcessors(),
3539
[$this->createMock(SpanProcessorInterface::class)],
36-
$this->getSpanProcessors()
40+
[$this->createMock(ExtendedSpanProcessorInterface::class)],
3741
);
3842

3943
$this->assertEquals(
@@ -57,6 +61,22 @@ public function test_on_start(): void
5761
);
5862
}
5963

64+
public function test_on_ending(): void
65+
{
66+
/** @var MockObject $processor */
67+
foreach ($this->getSpanProcessors() as $processor) {
68+
if ($processor instanceof ExtendedSpanProcessorInterface) {
69+
$processor->expects($this->once())
70+
->method('onEnding');
71+
}
72+
}
73+
74+
$this->createMultiSpanProcessor()
75+
->onEnding(
76+
$this->createMock(ReadWriteSpanInterface::class)
77+
);
78+
}
79+
6080
public function test_on_end(): void
6181
{
6282
/** @var MockObject $processor */
@@ -144,7 +164,7 @@ private function getSpanProcessors(): array
144164
return $this->spanProcessors === []
145165
? $this->spanProcessors = [
146166
$this->createMock(SpanProcessorInterface::class),
147-
$this->createMock(SpanProcessorInterface::class),
167+
$this->createMock(ExtendedSpanProcessorInterface::class),
148168
]
149169
: $this->spanProcessors;
150170
}

0 commit comments

Comments
 (0)