From 4568c413e6cd3fb9ba1cbb79e1fc5f3fcbfb50fd Mon Sep 17 00:00:00 2001 From: Alexandre Choura <42672104+PROFeNoM@users.noreply.github.com> Date: Wed, 10 Jan 2024 14:39:18 +0100 Subject: [PATCH] Add span links capabilities to the OTel API (#2451) * feat: Basic OTel Span Links Functionalities * test: Add Basic Interoperability Tests * feat: Partially handle on-the-fly addition of span links * feat: Handle Span Links Removal (Interoperability) * test: Duplicate Instances Addition * fix: Prevent creation of useless instances * Remove unused instance variable Co-authored-by: Bob Weinand --------- Co-authored-by: Bob Weinand --- src/DDTrace/OpenTelemetry/Context.php | 17 +- src/DDTrace/OpenTelemetry/Span.php | 76 +++++- src/DDTrace/OpenTelemetry/SpanBuilder.php | 17 +- .../Integration/InteroperabilityTest.php | 246 ++++++++++++++++++ .../Integration/SDK/SpanBuilderTest.php | 7 +- 5 files changed, 351 insertions(+), 12 deletions(-) diff --git a/src/DDTrace/OpenTelemetry/Context.php b/src/DDTrace/OpenTelemetry/Context.php index dcea348ffe..0e28b8486d 100644 --- a/src/DDTrace/OpenTelemetry/Context.php +++ b/src/DDTrace/OpenTelemetry/Context.php @@ -8,6 +8,7 @@ use DDTrace\Tag; use DDTrace\Util\ObjectKVStore; use OpenTelemetry\API\Trace as API; +use OpenTelemetry\SDK\Common\Attribute\Attributes; use OpenTelemetry\SDK\Common\Attribute\AttributesFactory; use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeFactory; use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeInterface; @@ -176,6 +177,18 @@ private static function activateParent(?SpanData $currentSpan): ContextInterface : null; $traceState = new API\TraceState($traceContext['tracestate'] ?? null); + // Check for span links + $links = []; + foreach ($currentSpan->links as $spanLink) { + $linkSpanContext = API\SpanContext::create( + $spanLink->traceId, + $spanLink->spanId, + API\TraceFlags::DEFAULT, + new API\TraceState($spanLink->traceState ?? null), + ); + $links[] = new SDK\Link($linkSpanContext, Attributes::create($spanLink->attributes)); + } + $OTelCurrentSpan = SDK\Span::startSpan( $currentSpan, API\SpanContext::create($currentTraceId, $currentSpanId, $traceFlags, $traceState), // $context @@ -186,8 +199,8 @@ private static function activateParent(?SpanData $currentSpan): ContextInterface NoopSpanProcessor::getInstance(), // $spanProcessor ResourceInfoFactory::defaultResource(), // $resource (new AttributesFactory())->builder(), // $attributesBuilder - [], // TODO: Handle Span Links - 0, // TODO: Handle Span Links + $links, // $links + count($links), // $totalRecordedLinks false // The span was created using the DD Api ); ObjectKVStore::put($currentSpan, 'otel_span', $OTelCurrentSpan); diff --git a/src/DDTrace/OpenTelemetry/Span.php b/src/DDTrace/OpenTelemetry/Span.php index ae823a71c3..9f1b5d8461 100644 --- a/src/DDTrace/OpenTelemetry/Span.php +++ b/src/DDTrace/OpenTelemetry/Span.php @@ -5,6 +5,7 @@ namespace OpenTelemetry\SDK\Trace; use DDTrace\SpanData; +use DDTrace\SpanLink; use DDTrace\Tag; use DDTrace\Util\Convention; use DDTrace\Util\ObjectKVStore; @@ -35,6 +36,16 @@ final class Span extends API\Span implements ReadWriteSpanInterface /** @readonly */ private SpanProcessorInterface $spanProcessor; + /** + * @readonly + * + * @var list + */ + private array $links; + + /** @readonly */ + private int $totalRecordedLinks; + /** @readonly */ private int $kind; @@ -56,6 +67,8 @@ private function __construct( API\SpanContextInterface $parentSpanContext, SpanProcessorInterface $spanProcessor, ResourceInfo $resource, + array $links = [], + int $totalRecordedLinks = 0, bool $isRemapped = true ) { $this->span = $span; @@ -65,6 +78,8 @@ private function __construct( $this->parentSpanContext = $parentSpanContext; $this->spanProcessor = $spanProcessor; $this->resource = $resource; + $this->links = $links; + $this->totalRecordedLinks = $totalRecordedLinks; $this->status = StatusData::unset(); @@ -75,6 +90,27 @@ private function __construct( // done in serializer.c:ddtrace_set_root_span_properties (as of v0.92.0) $span->name = $this->operationNameConvention = Convention::defaultOperationName($span); } + + // Set the span links + if ($isRemapped) { + // At initialization time (now), only set the links if the span was created using the OTel API + // Otherwise, the links were already set in DD's OpenTelemetry\Context\Context + 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; + } + } } /** @@ -116,6 +152,8 @@ public static function startSpan( $parentSpan->getContext(), $spanProcessor, $resource, + $links, + $totalRecordedLinks, $isRemapped ); @@ -162,10 +200,12 @@ public function toSpanData(): SpanDataInterface { $hasEnded = $this->hasEnded(); + $this->updateSpanLinks(); + return new ImmutableSpan( $this, $this->getName(), - [], // TODO: Handle Span Links + $this->links, [], // TODO: Handle Span Events Attributes::create(array_merge($this->span->meta, $this->span->metrics)), 0, @@ -206,7 +246,7 @@ public function getStartEpochNanos(): int public function getTotalRecordedLinks(): int { - return 0; + return $this->totalRecordedLinks; } public function getTotalRecordedEvents(): int @@ -400,4 +440,36 @@ public function getDDSpan(): SpanData { return $this->span; } + + private function updateSpanLinks() + { + // Important: Span Links are supposed immutable + $datadogSpanLinks = $this->span->links; + + $otel = []; + foreach ($datadogSpanLinks as $datadogSpanLink) { + // Check if the link relationship exists + $link = ObjectKVStore::get($datadogSpanLink, "link"); + if ($link === null) { + // Create the link + $link = new Link( + API\SpanContext::create( + $datadogSpanLink->traceId, + $datadogSpanLink->spanId, + $this->context->getTraceFlags(), + new API\TraceState($datadogSpanLink->traceState ?? null) + ), + Attributes::create($datadogSpanLink->attributes ?? []) + ); + + // Save the link + ObjectKVStore::put($datadogSpanLink, "link", $link); + } + $otel[] = $link; + } + + // Update the links + $this->links = $otel; + $this->totalRecordedLinks = count($otel); + } } diff --git a/src/DDTrace/OpenTelemetry/SpanBuilder.php b/src/DDTrace/OpenTelemetry/SpanBuilder.php index af96184adf..2d4f85c41b 100644 --- a/src/DDTrace/OpenTelemetry/SpanBuilder.php +++ b/src/DDTrace/OpenTelemetry/SpanBuilder.php @@ -69,7 +69,20 @@ public function setParent($context): API\SpanBuilderInterface public function addLink(SpanContextInterface $context, iterable $attributes = []): SpanBuilderInterface { - // TODO: Span Links are future works + if (!$context->isValid()) { + return $this; + } + + $this->totalNumberOfLinksAdded++; + + $this->links[] = new Link( + $context, + $this->tracerSharedState + ->getSpanLimits() + ->getLinkAttributesFactory() + ->builder($attributes) + ->build(), + ); return $this; } @@ -186,7 +199,7 @@ public function startSpan(): SpanInterface $this->tracerSharedState->getResource(), $attributesBuilder, $this->links, - 0, + $this->totalNumberOfLinksAdded, ); } diff --git a/tests/OpenTelemetry/Integration/InteroperabilityTest.php b/tests/OpenTelemetry/Integration/InteroperabilityTest.php index 46348090b7..e1b34b76bc 100644 --- a/tests/OpenTelemetry/Integration/InteroperabilityTest.php +++ b/tests/OpenTelemetry/Integration/InteroperabilityTest.php @@ -2,6 +2,7 @@ namespace DDTrace\Tests\OpenTelemetry\Integration; +use DDTrace\SpanLink; use DDTrace\Tag; use DDTrace\Tests\Common\BaseTestCase; use DDTrace\Tests\Common\SpanAssertion; @@ -14,6 +15,8 @@ use OpenTelemetry\API\Trace\SpanContext; use OpenTelemetry\API\Trace\SpanContextValidator; use OpenTelemetry\API\Trace\SpanKind; +use OpenTelemetry\API\Trace\TraceFlags; +use OpenTelemetry\API\Trace\TraceState; use OpenTelemetry\Context\Context; use OpenTelemetry\Context\ContextStorage; use OpenTelemetry\Extension\Propagator\B3\B3Propagator; @@ -907,5 +910,248 @@ public function testAttributesInteroperability() $this->assertEquals(2, $traces[0][0]['metrics']['m2']); } + public function testSpanLinksInteroperabilityFromDatadogSpan() + { + $traces = $this->isolateTracer(function () { + $span = start_span(); + $span->name = "dd.span"; + + $spanLink = new SpanLink(); + $spanLink->traceId = "ff0000000000051791e0000000000041"; + $spanLink->spanId = "ff00000000000517"; + $spanLink->traceState = "dd=t.dm:-0"; + $spanLink->attributes = [ + 'arg1' => 'value1', + 'arg2' => 'value2', + ]; + $span->links[] = $spanLink; + + /** @var \OpenTelemetry\SDK\Trace\Span $OTelSpan */ + $OTelSpan = Span::getCurrent(); + $OTelSpanLink = $OTelSpan->toSpanData()->getLinks()[0]; + $OTelSpanLinkContext = $OTelSpanLink->getSpanContext(); + + $this->assertSame('ff0000000000051791e0000000000041', $OTelSpanLinkContext->getTraceId()); + $this->assertSame('ff00000000000517', $OTelSpanLinkContext->getSpanId()); + $this->assertSame('dd=t.dm:-0', (string) $OTelSpanLinkContext->getTraceState()); + + $this->assertSame([ + 'arg1' => 'value1', + 'arg2' => 'value2', + ], $OTelSpanLink->getAttributes()->toArray()); + + close_span(); + }); + + $this->assertCount(1, $traces[0]); + $this->assertSame("[{\"trace_id\":\"ff0000000000051791e0000000000041\",\"span_id\":\"ff00000000000517\",\"trace_state\":\"dd=t.dm:-0\",\"attributes\":{\"arg1\":\"value1\",\"arg2\":\"value2\"}}]", $traces[0][0]['meta']['_dd.span_links']); + } + + public function testSpanLinksInteroperabilityFromOpenTelemetrySpan() + { + $sampledSpanContext = SpanContext::create( + '12345678876543211234567887654321', + '8765432112345678', + TraceFlags::SAMPLED, + new TraceState('dd=t.dm:-0') + ); + + $traces = $this->isolateTracer(function () use ($sampledSpanContext) { + $otelSpan = self::getTracer()->spanBuilder("otel.span") + ->addLink($sampledSpanContext, ['arg1' => 'value1']) + ->startSpan(); + + $activeSpan = active_span(); + $spanLink = $activeSpan->links[0]; + $this->assertSame('12345678876543211234567887654321', $spanLink->traceId); + $this->assertSame('8765432112345678', $spanLink->spanId); + $this->assertSame('dd=t.dm:-0', $spanLink->traceState); + $this->assertSame(['arg1' => 'value1'], $spanLink->attributes); + $this->assertEquals(0, $spanLink->droppedAttributesCount); + + $otelSpan->end(); + }); + + $this->assertCount(1, $traces[0]); + $this->assertSame("[{\"trace_id\":\"12345678876543211234567887654321\",\"span_id\":\"8765432112345678\",\"trace_state\":\"dd=t.dm:-0\",\"attributes\":{\"arg1\":\"value1\"},\"dropped_attributes_count\":0}]", $traces[0][0]['meta']['_dd.span_links']); + } + + public function testSpanLinksInteroperabilityBothTypes() + { + $sampledSpanContext = SpanContext::create( + '12345678876543211234567887654321', + '8765432112345678', + TraceFlags::SAMPLED, + new TraceState('dd=t.dm:-0') + ); + + $traces = $this->isolateTracer(function () use ($sampledSpanContext) { + // Add 1 span link using the OTel API + $otelSpan = self::getTracer()->spanBuilder("otel.span") + ->addLink($sampledSpanContext, ['arg1' => 'value1']) + ->startSpan(); + + // Add 1 span link using the DD API + $newSpanLink = new SpanLink(); + $newSpanLink->traceId = "ff0000000000051791e0000000000041"; + $newSpanLink->spanId = "ff00000000000517"; + active_span()->links[] = $newSpanLink; + + // Verify the span links from DD's POV + $datadogSpanLinks = active_span()->links; + $this->assertCount(2, $datadogSpanLinks); + + $this->assertSame('12345678876543211234567887654321', $datadogSpanLinks[0]->traceId); + $this->assertSame('8765432112345678', $datadogSpanLinks[0]->spanId); + $this->assertSame('dd=t.dm:-0', $datadogSpanLinks[0]->traceState); + $this->assertSame(['arg1' => 'value1'], $datadogSpanLinks[0]->attributes); + $this->assertEquals(0, $datadogSpanLinks[0]->droppedAttributesCount); + + $this->assertSame('ff0000000000051791e0000000000041', $datadogSpanLinks[1]->traceId); + $this->assertSame('ff00000000000517', $datadogSpanLinks[1]->spanId); + + // Verify the span links from OTel's POV + $otelSpanLinks = $otelSpan->toSpanData()->getLinks(); + + $firstSpanLinkContext = $otelSpanLinks[0]->getSpanContext(); + $this->assertSame('12345678876543211234567887654321', $firstSpanLinkContext->getTraceId()); + $this->assertSame('8765432112345678', $firstSpanLinkContext->getSpanId()); + $this->assertSame('dd=t.dm:-0', (string) $firstSpanLinkContext->getTraceState()); + $this->assertSame(['arg1' => 'value1'], $otelSpanLinks[0]->getAttributes()->toArray()); + $secondSpanLinkContext = $otelSpanLinks[1]->getSpanContext(); + $this->assertSame('ff0000000000051791e0000000000041', $secondSpanLinkContext->getTraceId()); + $this->assertSame('ff00000000000517', $secondSpanLinkContext->getSpanId()); + + + $otelSpan->end(); + }); + + $this->assertCount(1, $traces[0]); + $this->assertSame("[{\"trace_id\":\"12345678876543211234567887654321\",\"span_id\":\"8765432112345678\",\"trace_state\":\"dd=t.dm:-0\",\"attributes\":{\"arg1\":\"value1\"},\"dropped_attributes_count\":0},{\"trace_id\":\"ff0000000000051791e0000000000041\",\"span_id\":\"ff00000000000517\"}]", $traces[0][0]['meta']['_dd.span_links']); + } + + public function testSpanLinksInteroperabilityRemoval() + { + $sampledSpanContext = SpanContext::create( + '12345678876543211234567887654321', + '8765432112345678', + TraceFlags::SAMPLED, + new TraceState('dd=t.dm:-0') + ); + + $traces = $this->isolateTracer(function () use ($sampledSpanContext) { + // Add 1 span link using the OTel API + $otelSpan = self::getTracer()->spanBuilder("otel.span") + ->addLink($sampledSpanContext, ['arg1' => 'value1']) + ->startSpan(); + + // Remove the span link using the DD API + unset(active_span()->links[0]); + + // Add a span link using the DD API + $newSpanLink = new SpanLink(); + $newSpanLink->traceId = "ff0000000000051791e0000000000041"; + $newSpanLink->spanId = "ff00000000000517"; + $newSpanLink->traceState = "dd=t.dm:-1"; + $newSpanLink->attributes = [ + 'arg3' => 'value3', + 'arg4' => 'value4', + ]; + active_span()->links[] = $newSpanLink; + + // Verify that there is only 1 span link from OTel's POV + $otelSpanLinks = $otelSpan->toSpanData()->getLinks(); + $this->assertCount(1, $otelSpanLinks); + $spanLinkContext = $otelSpanLinks[0]->getSpanContext(); + $this->assertSame('ff0000000000051791e0000000000041', $spanLinkContext->getTraceId()); + $this->assertSame('ff00000000000517', $spanLinkContext->getSpanId()); + $this->assertSame('dd=t.dm:-1', (string) $spanLinkContext->getTraceState()); + $this->assertSame(['arg3' => 'value3', 'arg4' => 'value4'], $otelSpanLinks[0]->getAttributes()->toArray()); + + $otelSpan->end(); + }); + + $this->assertCount(1, $traces[0]); + } + + public function testSpanLinksInteroperabilityRemovalDuplicates() + { + $traces = $this->isolateTracer(function () { + $context1 = SpanContext::create('00000000000000000000000000000001', '0000000000000001'); + $context2 = SpanContext::create('00000000000000000000000000000002', '0000000000000002'); + $context3 = SpanContext::create('00000000000000000000000000000003', '0000000000000003'); + + // Otel -> [Link(T1, S1), Link(T2, S2), Link(T2, S2), Link(T3, S3)] + $otelSpan = self::getTracer()->spanBuilder("otel.span") + ->addLink($context1) + ->addLink($context2) + ->addLink($context2) // Duplicate + ->addLink($context3) + ->startSpan(); + $initialOtelSpanLinks = $otelSpan->toSpanData()->getLinks(); + + // Modify to [Link(T1, S1), Link(T2, S2), Link(T4, S4)] + unset(active_span()->links[3]); // Remove Link(T3, S3) + unset(active_span()->links[1]); // Remove Link(T2, S2) + $newSpanLink = new SpanLink(); + $newSpanLink->traceId = "00000000000000000000000000000004"; + $newSpanLink->spanId = "0000000000000004"; + active_span()->links[] = $newSpanLink; // Add Link(T4, S4) + + // Verify the spans links from OTel's POV + $otelSpanLinks = $otelSpan->toSpanData()->getLinks(); + $this->assertCount(3, $otelSpanLinks); + + // Verify Link(T1, S1) + Instance + $spanLinkContext = $otelSpanLinks[0]->getSpanContext(); + $this->assertSame('00000000000000000000000000000001', $spanLinkContext->getTraceId()); + $this->assertSame('0000000000000001', $spanLinkContext->getSpanId()); + $this->assertSame($initialOtelSpanLinks[0], $otelSpanLinks[0]); + + // Verify Link(T2, S2) + Instance + $spanLinkContext = $otelSpanLinks[1]->getSpanContext(); + $this->assertSame('00000000000000000000000000000002', $spanLinkContext->getTraceId()); + $this->assertSame('0000000000000002', $spanLinkContext->getSpanId()); + $this->assertSame($initialOtelSpanLinks[2], $otelSpanLinks[1]); + + // Verify Link(T4, S4) + $spanLinkContext = $otelSpanLinks[2]->getSpanContext(); + $this->assertSame('00000000000000000000000000000004', $spanLinkContext->getTraceId()); + $this->assertSame('0000000000000004', $spanLinkContext->getSpanId()); + }); + } + + public function testSpanLinksInteroperabilityAddDuplicates() + { + $traces = $this->isolateTracer(function () { + $context1 = SpanContext::create('00000000000000000000000000000001', '0000000000000001'); + $context2 = SpanContext::create('00000000000000000000000000000002', '0000000000000002'); + + $otelSpan = self::getTracer()->spanBuilder("otel.span") + ->addLink($context1) + ->addLink($context2) + ->startSpan(); + + $spanLink2 = active_span()->links[1]; + active_span()->links[] = $spanLink2; // Duplicate, same instance + + $otelSpanLinks = $otelSpan->toSpanData()->getLinks(); + + $this->assertCount(3, $otelSpanLinks); + // Verify that the duplicate is the same instance + $this->assertSame($otelSpanLinks[1], $otelSpanLinks[2]); + + // Create a new span link with the same trace id and span id + $newSpanLink = new SpanLink(); + $newSpanLink->traceId = $spanLink2->traceId; + $newSpanLink->spanId = $spanLink2->spanId; + active_span()->links[] = $newSpanLink; // Duplicate, but different instance + + $otelSpanLinks = $otelSpan->toSpanData()->getLinks(); + $this->assertCount(4, $otelSpanLinks); + // Verify that the duplicate is not the same instance + $this->assertNotSame($otelSpanLinks[1], $otelSpanLinks[3]); + }); + } } diff --git a/tests/OpenTelemetry/Integration/SDK/SpanBuilderTest.php b/tests/OpenTelemetry/Integration/SDK/SpanBuilderTest.php index d024e1a1fa..b849ce7e45 100644 --- a/tests/OpenTelemetry/Integration/SDK/SpanBuilderTest.php +++ b/tests/OpenTelemetry/Integration/SDK/SpanBuilderTest.php @@ -98,7 +98,6 @@ public function test_set_parent_invalid_context(): void */ public function test_add_link(): void { - $this->markTestSkipped("Span Links aren't yet supported"); /** @var Span $span */ $span = $this ->tracer @@ -112,7 +111,6 @@ public function test_add_link(): void public function test_add_link_invalid(): void { - $this->markTestIncomplete("Span Links aren't yet supported"); /** @var Span $span */ $span = $this ->tracer @@ -127,7 +125,7 @@ public function test_add_link_invalid(): void public function test_add_link_dropping_links(): void { - $this->markTestSkipped("Span Links aren't yet supported"); + $this->markTestSkipped("Irrelevant: Span Links Limits aren't supported"); $maxNumberOfLinks = 8; $tracerProvider = new TracerProvider([], null, null, (new SpanLimitsBuilder())->setLinkCountLimit($maxNumberOfLinks)->build()); $spanBuilder = $tracerProvider @@ -156,7 +154,6 @@ public function test_add_link_dropping_links(): void public function test_add_link_truncate_link_attributes(): void { - $this->markTestSkipped("Span Links aren't yet supported"); $tracerProvider = new TracerProvider([], null, null, (new SpanLimitsBuilder())->setAttributePerLinkCountLimit(1)->build()); /** @var Span $span */ $span = $tracerProvider @@ -178,7 +175,6 @@ public function test_add_link_truncate_link_attributes(): void public function test_add_link_truncate_link_attribute_value(): void { - $this->markTestSkipped("Spans Links aren't yet supported"); $maxLength = 25; $strVal = str_repeat('a', $maxLength); @@ -218,7 +214,6 @@ public function test_add_link_truncate_link_attribute_value(): void */ public function test_add_link_no_effect_after_start_span(): void { - $this->markTestSkipped("Span Links aren't yet supported"); $spanBuilder = $this->tracer->spanBuilder(self::SPAN_NAME); /** @var Span $span */