Skip to content

Commit 9680835

Browse files
SergeyKleymanSergeyKleyman
authored
Fix IS_REMOTE flag (#1522)
* Fixed span's is_remote flag computation * Added testing for cases without parent and with local parent span
1 parent 08c8860 commit 9680835

File tree

2 files changed

+86
-14
lines changed

2 files changed

+86
-14
lines changed

‎src/Contrib/Otlp/SpanConverter.php‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ private function convertSpan(SpanDataInterface $span): Span
149149
$pSpan = new Span();
150150
$pSpan->setTraceId($this->serializer->serializeTraceId($span->getContext()->getTraceIdBinary()));
151151
$pSpan->setSpanId($this->serializer->serializeSpanId($span->getContext()->getSpanIdBinary()));
152-
$pSpan->setFlags(self::traceFlags($span->getContext()));
152+
$pSpan->setFlags(self::buildFlagsForSpan($span->getContext(), parentSpanContext: $span->getParentContext()));
153153
$pSpan->setTraceState((string) $span->getContext()->getTraceState());
154154
if ($span->getParentContext()->isValid()) {
155155
$pSpan->setParentSpanId($this->serializer->serializeSpanId($span->getParentContext()->getSpanIdBinary()));
@@ -174,7 +174,7 @@ private function convertSpan(SpanDataInterface $span): Span
174174
$pSpan->getLinks()[] = $pLink = new Link();
175175
$pLink->setTraceId($this->serializer->serializeTraceId($link->getSpanContext()->getTraceIdBinary()));
176176
$pLink->setSpanId($this->serializer->serializeSpanId($link->getSpanContext()->getSpanIdBinary()));
177-
$pLink->setFlags(self::traceFlags($link->getSpanContext()));
177+
$pLink->setFlags(self::buildFlagsForLink($link->getSpanContext()));
178178
$pLink->setTraceState((string) $link->getSpanContext()->getTraceState());
179179
$this->setAttributes($pLink, $link->getAttributes());
180180
}
@@ -188,14 +188,46 @@ private function convertSpan(SpanDataInterface $span): Span
188188
return $pSpan;
189189
}
190190

191-
private static function traceFlags(SpanContextInterface $spanContext): int
191+
private static function addRemoteFlags(SpanContextInterface $spanContext, int $baseFlags): int
192192
{
193-
$flags = $spanContext->getTraceFlags();
193+
$flags = $baseFlags;
194194
$flags |= SpanFlags::SPAN_FLAGS_CONTEXT_HAS_IS_REMOTE_MASK;
195195
if ($spanContext->isRemote()) {
196196
$flags |= SpanFlags::SPAN_FLAGS_CONTEXT_IS_REMOTE_MASK;
197197
}
198198

199199
return $flags;
200200
}
201+
202+
private static function buildFlagsForSpan(SpanContextInterface $spanContext, SpanContextInterface $parentSpanContext): int
203+
{
204+
$flags = $spanContext->getTraceFlags();
205+
206+
/**
207+
* @see https://github.com/open-telemetry/opentelemetry-proto/blob/v1.5.0/opentelemetry/proto/trace/v1/trace.proto#L122
208+
*
209+
* Bits 8 and 9 represent the 3 states of whether a span's parent is remote.
210+
* ^^^^^^
211+
* That is why we pass parent span's context.
212+
*/
213+
$flags = self::addRemoteFlags($parentSpanContext, $flags);
214+
215+
return $flags;
216+
}
217+
218+
private static function buildFlagsForLink(SpanContextInterface $linkSpanContext): int
219+
{
220+
$flags = $linkSpanContext->getTraceFlags();
221+
222+
/**
223+
* @see https://github.com/open-telemetry/opentelemetry-proto/blob/v1.5.0/opentelemetry/proto/trace/v1/trace.proto#L279
224+
*
225+
* Bits 8 and 9 represent the 3 states of whether the link is remote.
226+
* ^^^^
227+
* That is why we pass link span's context.
228+
*/
229+
$flags = self::addRemoteFlags($linkSpanContext, $flags);
230+
231+
return $flags;
232+
}
201233
}

‎tests/Unit/Contrib/Otlp/SpanConverterTest.php‎

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@
1616
use Opentelemetry\Proto\Trace\V1;
1717
use Opentelemetry\Proto\Trace\V1\ResourceSpans;
1818
use Opentelemetry\Proto\Trace\V1\ScopeSpans;
19+
use Opentelemetry\Proto\Trace\V1\Span as ProtoSpan;
20+
use Opentelemetry\Proto\Trace\V1\Span\Link as ProtoSpanLink;
1921
use Opentelemetry\Proto\Trace\V1\Span\SpanKind as ProtoSpanKind;
22+
use Opentelemetry\Proto\Trace\V1\SpanFlags;
2023
use OpenTelemetry\SDK\Common\Attribute\Attributes;
2124
use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScope;
2225
use OpenTelemetry\SDK\Resource\ResourceInfo;
@@ -64,18 +67,54 @@ public function test_convert_span_to_payload(): void
6467

6568
public function test_span_context_is_remote_flags(): void
6669
{
67-
$span = (new SpanData())
68-
->setContext(SpanContext::createFromRemoteParent('0000000000000001', '00000001'))
69-
->addLink(SpanContext::createFromRemoteParent('0000000000000001', '00000002'), Attributes::create([]))
70-
->addLink(SpanContext::createFromRemoteParent('0000000000000001', '00000003', TraceFlags::SAMPLED), Attributes::create([]));
70+
$isFlagSet = static function (int $flags, int $mask): bool {
71+
return ($flags & $mask) !== 0;
72+
};
7173

72-
$converter = new SpanConverter();
73-
/** @psalm-suppress InvalidArgument */
74-
$row = $converter->convert([$span])->getResourceSpans()[0]->getScopeSpans()[0]->getSpans()[0];
74+
$isRemote = static function (int $flags) use ($isFlagSet): ?bool {
75+
if (!$isFlagSet($flags, V1\SpanFlags::SPAN_FLAGS_CONTEXT_HAS_IS_REMOTE_MASK)) {
76+
return null;
77+
}
78+
79+
return $isFlagSet($flags, V1\SpanFlags::SPAN_FLAGS_CONTEXT_IS_REMOTE_MASK);
80+
};
81+
82+
$convertSpanData = static function (SpanData $spanData): ProtoSpan {
83+
$converter = new SpanConverter();
84+
85+
/** @psalm-suppress InvalidArgument */
86+
return $converter->convert([$spanData])->getResourceSpans()[0]->getScopeSpans()[0]->getSpans()[0];
87+
};
88+
89+
$getLink = static function (ProtoSpan $protoSpan, int $linkIndex): ProtoSpanLink {
90+
/** @psalm-suppress InvalidArgument */
91+
return $protoSpan->getLinks()[$linkIndex];
92+
};
93+
94+
// Span with remote parent
95+
$convertedSpan = $convertSpanData(
96+
(new SpanData())
97+
->setParentContext(SpanContext::createFromRemoteParent('0000000000000001', '00000001'))
98+
->setContext(SpanContext::create('0000000000000001', '00000002'))
99+
->addLink(SpanContext::createFromRemoteParent('0000000000000001', '00000003'), Attributes::create([]))
100+
->addLink(SpanContext::createFromRemoteParent('0000000000000001', '00000004', TraceFlags::SAMPLED), Attributes::create([]))
101+
);
102+
$this->assertTrue($isRemote($convertedSpan->getFlags()));
103+
$this->assertTrue($isRemote($getLink($convertedSpan, 0)->getFlags()));
104+
$this->assertTrue($isRemote($getLink($convertedSpan, 1)->getFlags()));
105+
$this->assertTrue($isFlagSet($getLink($convertedSpan, 1)->getFlags(), TraceFlags::SAMPLED));
106+
107+
// Span without parent
108+
$convertedSpan = $convertSpanData((new SpanData())->setContext(SpanContext::create('0000000000000001', '00000001')));
109+
$this->assertFalse($isRemote($convertedSpan->getFlags()));
75110

76-
$this->assertSame(V1\SpanFlags::SPAN_FLAGS_CONTEXT_HAS_IS_REMOTE_MASK | V1\SpanFlags::SPAN_FLAGS_CONTEXT_IS_REMOTE_MASK, $row->getFlags());
77-
$this->assertSame(V1\SpanFlags::SPAN_FLAGS_CONTEXT_HAS_IS_REMOTE_MASK | V1\SpanFlags::SPAN_FLAGS_CONTEXT_IS_REMOTE_MASK, $row->getLinks()[0]->getFlags());
78-
$this->assertSame(V1\SpanFlags::SPAN_FLAGS_CONTEXT_HAS_IS_REMOTE_MASK | V1\SpanFlags::SPAN_FLAGS_CONTEXT_IS_REMOTE_MASK | TraceFlags::SAMPLED, $row->getLinks()[1]->getFlags());
111+
// Span with local parent
112+
$convertedSpan = $convertSpanData(
113+
(new SpanData())
114+
->setParentContext(SpanContext::create('0000000000000001', '00000001'))
115+
->setContext(SpanContext::create('0000000000000001', '00000002'))
116+
);
117+
$this->assertFalse($isRemote($convertedSpan->getFlags()));
79118
}
80119

81120
#[DataProvider('attributeAreCoercedCorrectlyDataProvider')]
@@ -217,6 +256,7 @@ public function test_otlp_happy_path_span(): void
217256
'start_time_unix_nano' => $start_time,
218257
'end_time_unix_nano' => $end_time,
219258
'kind' => V1\Span\SpanKind::SPAN_KIND_INTERNAL,
259+
'flags' => SpanFlags::SPAN_FLAGS_CONTEXT_HAS_IS_REMOTE_MASK,
220260
'status' => new V1\Status([ 'code' => V1\Status\StatusCode::STATUS_CODE_OK ]),
221261
'attributes' => [
222262
new KeyValue([

0 commit comments

Comments
 (0)
⚡