Skip to content

Commit 4c0dcf6

Browse files
mabdinurbwoebi
mabdinur
and
bwoebi
authored
w3c phase 2: add last parent_id to tracestate (#2549)
* app p to php * Finish implementation of _dd.parent_id Signed-off-by: Bob Weinand <[email protected]> * ignore tracestate in field in snapshots * fix tests * handle errors * fix testGetSpanContextWithRemoteParent * Take the proper span context into account for OTel tracecontext generation Signed-off-by: Bob Weinand <[email protected]> --------- Signed-off-by: Bob Weinand <[email protected]> Co-authored-by: Bob Weinand <[email protected]>
1 parent 7c36868 commit 4c0dcf6

12 files changed

Lines changed: 72 additions & 29 deletions

‎docker-compose.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ services:
223223
- SNAPSHOTS_DIR=/snapshots
224224
- SNAPSHOT_CI=0
225225
- SNAPSHOT_REMOVED_ATTRS=start,duration,metrics.php.compilation.total_time_ms,metrics.process_id
226+
- SNAPSHOT_IGNORED_ATTRS=meta._dd.parent_id,meta.tracestate
226227
- ENABLED_CHECKS=trace_stall,trace_peer_service,trace_dd_service
227228

228229

‎ext/ddtrace.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,7 @@ ZEND_METHOD(DDTrace_SpanLink, fromHeaders) {
642642
zend_hash_copy(Z_ARR(link->property_attributes), &result.meta_tags, NULL);
643643

644644
zend_string *propagated_tags = ddtrace_format_propagated_tags(&result.propagated_tags, &result.meta_tags);
645-
zend_string *full_tracestate = ddtrace_format_tracestate(result.tracestate, result.origin, result.priority_sampling, propagated_tags, &result.tracestate_unknown_dd_keys);
645+
zend_string *full_tracestate = ddtrace_format_tracestate(result.tracestate, 0, result.origin, result.priority_sampling, propagated_tags, &result.tracestate_unknown_dd_keys);
646646
zend_string_release(propagated_tags);
647647
if (full_tracestate) {
648648
ZVAL_STR(&link->property_trace_state, full_tracestate);

‎ext/distributed_tracing_headers.c‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,9 @@ static ddtrace_distributed_tracing_result ddtrace_read_distributed_tracing_ids_t
227227
result.parent_id = parent_id;
228228
result.priority_sampling = (tracedata->trace_flags[1] & 1) == (tracedata->trace_flags[1] <= '9'); // ('a' & 1) == 1
229229

230-
// header format: "[*,]dd=s:1;o:rum;t.dm:-4;t.usr.id:12345[,*]"
230+
zend_string *span_parent_key = zend_string_init("_dd.parent_id", strlen("_dd.parent_id"), 0);
231+
232+
// header format: "[*,]dd=p:0000000000000111;s:1;o:rum;t.dm:-4;t.usr.id:12345[,*]"
231233
if (read_header((zai_str)ZAI_STRL("TRACESTATE"), "tracestate", &tracestate, data)) {
232234
bool last_comma = true;
233235
result.tracestate = zend_string_alloc(ZSTR_LEN(tracestate), 0);
@@ -260,7 +262,13 @@ static ddtrace_distributed_tracing_result ddtrace_read_distributed_tracing_ids_t
260262
}
261263
size_t valuelen = valueend - valuestart;
262264

263-
if (keylen == 1 && keystart[0] == 's') {
265+
if (keylen == 1 && keystart[0] == 'p') {
266+
zval zv;
267+
ZVAL_STRINGL(&zv, valuestart, valuelen);
268+
zend_hash_update(&result.meta_tags, span_parent_key, &zv);
269+
zend_string_release(span_parent_key);
270+
span_parent_key = NULL;
271+
} else if (keylen == 1 && keystart[0] == 's') {
264272
int extraced_priority = strtol(valuestart, NULL, 10);
265273
if ((result.priority_sampling > 0) == (extraced_priority > 0)) {
266274
result.priority_sampling = extraced_priority;
@@ -316,6 +324,13 @@ static ddtrace_distributed_tracing_result ddtrace_read_distributed_tracing_ids_t
316324
zend_string_release(tracestate);
317325
}
318326

327+
if (span_parent_key) {
328+
zval zv;
329+
ZVAL_STRING(&zv, "0000000000000000");
330+
zend_hash_update(&result.meta_tags, span_parent_key, &zv);
331+
zend_string_release(span_parent_key);
332+
}
333+
319334
dd_check_tid(&result);
320335
}
321336

‎ext/handlers_http.h‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,17 @@
77

88
ZEND_EXTERN_MODULE_GLOBALS(ddtrace);
99

10-
static inline zend_string *ddtrace_format_tracestate(zend_string *tracestate, zend_string *origin, zend_long sampling_priority, zend_string *propagated_tags, zend_array *tracestate_unknown_dd_keys) {
10+
static inline zend_string *ddtrace_format_tracestate(zend_string *tracestate, uint64_t span_id, zend_string *origin, zend_long sampling_priority, zend_string *propagated_tags, zend_array *tracestate_unknown_dd_keys) {
1111
smart_str str = {0};
1212

13+
if (span_id) {
14+
smart_str_append_printf(&str, "p:%016" PRIx64, span_id);
15+
}
16+
1317
if (origin) {
18+
if (str.s) {
19+
smart_str_appendc(&str, ';');
20+
}
1421
smart_str_appends(&str, "o:");
1522
signed char *cur = (signed char *)ZSTR_VAL(str.s) + ZSTR_LEN(str.s);
1623
smart_str_append(&str, origin);
@@ -178,7 +185,15 @@ static inline void ddtrace_inject_distributed_headers_config(zend_array *array,
178185
span_id,
179186
sampling_priority > 0);
180187

181-
zend_string *full_tracestate = ddtrace_format_tracestate(tracestate, origin, sampling_priority, propagated_tags, tracestate_unknown_dd_keys);
188+
uint64_t propagated_span_id = 0;
189+
zval *old_parent_id;
190+
if (root) {
191+
propagated_span_id = span_id;
192+
} else if ((old_parent_id = zend_hash_str_find(&DDTRACE_G(root_span_tags_preset), ZEND_STRL("_dd.parent_id")))) {
193+
propagated_span_id = ddtrace_parse_hex_span_id(old_parent_id);
194+
}
195+
196+
zend_string *full_tracestate = ddtrace_format_tracestate(tracestate, propagated_span_id, origin, sampling_priority, propagated_tags, tracestate_unknown_dd_keys);
182197
if (full_tracestate) {
183198
ADD_HEADER("tracestate", "%.*s", (int)ZSTR_LEN(full_tracestate), ZSTR_VAL(full_tracestate));
184199
zend_string_release(full_tracestate);

‎src/DDTrace/OpenTelemetry/SpanContext.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,14 @@ public function getSpanIdBinary(): string
7474

7575
public function getTraceState(): ?TraceStateInterface
7676
{
77-
$traceContext = generate_distributed_tracing_headers(['tracecontext']);
77+
$current = \DDTrace\active_stack();
78+
if ($current !== $this->span->stack) {
79+
\DDTrace\switch_stack($this->span);
80+
$traceContext = generate_distributed_tracing_headers(['tracecontext']);
81+
\DDTrace\switch_stack($current);
82+
} else {
83+
$traceContext = generate_distributed_tracing_headers(['tracecontext']);
84+
}
7885
return new TraceState($traceContext['tracestate'] ?? null);
7986
}
8087

‎tests/OpenTelemetry/Integration/API/TracerTest.php‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -543,9 +543,9 @@ public function testGetSpanContextWithMultipleTraceStates()
543543
'_dd.p.congo' => 't61rcWkgMzE',
544544
'_dd.p.some_val' => 'tehehe'
545545
]);
546-
$this->assertRegularExpression('/^dd=t.dm:-0;t.congo:t61rcWkgMzE;t.some_val:tehehe$/', (string)$span->getContext()->getTraceState());
546+
$this->assertRegularExpression('/^dd=p:[0-9a-f]{16};t.congo:t61rcWkgMzE;t.some_val:tehehe;t.dm:-0$/', (string)$span->getContext()->getTraceState());
547547
$span->end();
548-
$this->assertRegularExpression('/^dd=t.dm:-0;t.congo:t61rcWkgMzE;t.some_val:tehehe$/', (string)$span->getContext()->getTraceState());
548+
$this->assertRegularExpression('/^dd=p:[0-9a-f]{16};t.congo:t61rcWkgMzE;t.some_val:tehehe;t.dm:-0$/', (string)$span->getContext()->getTraceState());
549549
});
550550

551551
$span = $traces[0][0];
@@ -589,7 +589,7 @@ public function testGetSpanContextWithRemoteParent(int $traceFlags, ?TraceState
589589
$this->assertSame($remoteContext->getSpanId(), $child->getParentContext()->getSpanId());
590590
$this->assertFalse($childContext->isRemote()); // "When creating children from remote spans, their IsRemote flag MUST be set to false."
591591
$this->assertEquals(1, $childContext->getTraceFlags()); // RECORD_AND_SAMPLED ==> 01 (AlwaysOn sampler)
592-
$this->assertSame("dd=t.dm:-0" . ($traceState ? ",$traceState" : ""), (string)$childContext->getTraceState());
592+
$this->assertSame("dd=p:" . $child->getContext()->getSpanID() . ";t.dm:-0" . ($traceState ? ",$traceState" : ""), (string)$childContext->getTraceState());
593593
});
594594

595595
$span = $traces[0][0];
@@ -664,11 +664,11 @@ public function getDescription(): string
664664
)))->getTracer('OpenTelemetry.TracerTest');
665665
$parent = $tracer->spanBuilder("parent")->startSpan(); // root sampler will be used
666666
$scope = $parent->activate();
667-
$this->assertRegularExpression('/^dd=t.dm:-0,root=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE$/', (string)$parent->getContext()->getTraceState());
667+
$this->assertRegularExpression('/^dd=p:[0-9a-f]{16};t.dm:-0,root=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE$/', (string)$parent->getContext()->getTraceState());
668668
$parent->setAttributes([
669669
'_dd.p.some_val' => 'tehehe'
670670
]);
671-
$this->assertRegularExpression('/^dd=t.dm:-0;t.some_val:tehehe,root=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE$/', (string)$parent->getContext()->getTraceState());
671+
$this->assertRegularExpression('/^dd=p:[0-9a-f]{16};t.dm:-0;t.some_val:tehehe,root=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE$/', (string)$parent->getContext()->getTraceState());
672672
try {
673673
$child = $tracer->spanBuilder("child")->startSpan(); // local parent sampler will be used
674674

@@ -677,8 +677,8 @@ public function getDescription(): string
677677
$this->assertSame($parent->getContext()->getSpanId(), $child->getParentContext()->getSpanId());
678678
$this->assertFalse($childContext->isRemote()); // "When creating children from remote spans, their IsRemote flag MUST be set to false."
679679
$this->assertEquals(1, $childContext->getTraceFlags()); // RECORD_AND_SAMPLED ==> 01 (AlwaysOn sampler)
680-
$this->assertRegularExpression('/^dd=t.dm:-0,localparent=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE$/', (string)$childContext->getTraceState());
681-
$this->assertRegularExpression('/^dd=t.dm:-0,localparent=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE$/', (string)$parent->getContext()->getTraceState());
680+
$this->assertRegularExpression('/^dd=p:[0-9a-f]{16};t.dm:-0,localparent=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE$/', (string)$childContext->getTraceState());
681+
$this->assertRegularExpression('/^dd=p:[0-9a-f]{16};t.dm:-0;t.some_val:tehehe,root=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE$/', (string)$parent->getContext()->getTraceState());
682682

683683
$grandChild = $tracer->spanBuilder("grandChild")
684684
->setParent(Context::getCurrent()->withContextValue($child))
@@ -690,14 +690,14 @@ public function getDescription(): string
690690
$this->assertSame($child->getContext()->getSpanId(), $grandChild->getParentContext()->getSpanId());
691691
$this->assertFalse($grandChildContext->isRemote()); // "When creating children from remote spans, their IsRemote flag MUST be set to false."
692692
$this->assertEquals(1, $grandChildContext->getTraceFlags()); // RECORD_AND_SAMPLED ==> 01 (AlwaysOn sampler)
693-
$this->assertRegularExpression('/^dd=t.dm:-0,localparent=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE$/', (string)$grandChildContext->getTraceState());
693+
$this->assertRegularExpression('/^dd=p:[0-9a-f]{16};t.dm:-0,localparent=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE$/', (string)$grandChildContext->getTraceState());
694694

695695
$grandChildScope->detach();
696696
$grandChild->end();
697697

698698
$child->end();
699699
} finally {
700-
$this->assertRegularExpression('/^dd=t.dm:-0;t.some_val:tehehe,root=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE$/', (string)$parent->getContext()->getTraceState());
700+
$this->assertRegularExpression('/^dd=p:[0-9a-f]{16};t.dm:-0;t.some_val:tehehe,root=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE$/', (string)$parent->getContext()->getTraceState());
701701
$scope->detach();
702702
$parent->end();
703703
}
@@ -806,22 +806,25 @@ public function getDescription(): string
806806
$this->assertSame($remoteContext->getSpanId(), $child->getParentContext()->getSpanId());
807807
$this->assertFalse($childContext->isRemote()); // "When creating children from remote spans, their IsRemote flag MUST be set to false."
808808
$this->assertEquals(1, $childContext->getTraceFlags()); // RECORD_AND_SAMPLED ==> 01 (AlwaysOn sampler)
809-
//$this->assertSame("dd=t.dm:-0,remoteparent=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE", (string)$childContext->getTraceState());
809+
//$this->assertSame("dd=p:[0-9a-f]{16};t.dm:-0,remoteparent=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE", (string)$childContext->getTraceState());
810810

811811
$tracer = self::getTracer();
812812
$grandChild = $tracer->spanBuilder("grandChild")
813813
->setParent(Context::getCurrent()->withContextValue($child))
814814
->startSpan();
815-
$this->assertSame("dd=t.dm:-0,remoteparent=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE", (string)$child->getContext()->getTraceState());
815+
$expected_tracestate = "dd=p:" . $childContext->getSpanId() . ";t.dm:-0,remoteparent=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE";
816+
$this->assertSame($expected_tracestate, (string)$child->getContext()->getTraceState());
816817
$grandChildScope = $grandChild->activate();
817818

818819
$grandChildContext = $grandChild->getContext();
819820
$this->assertSame($remoteContext->getTraceId(), $grandChildContext->getTraceId());
820821
$this->assertSame($child->getContext()->getSpanId(), $grandChild->getParentContext()->getSpanId());
821822
$this->assertFalse($grandChildContext->isRemote()); // "When creating children from remote spans, their IsRemote flag MUST be set to false."
822823
$this->assertEquals(1, $grandChildContext->getTraceFlags()); // RECORD_AND_SAMPLED ==> 01 (AlwaysOn sampler)
823-
$this->assertSame("dd=t.dm:-0,remoteparent=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE", (string)$child->getContext()->getTraceState());
824-
$this->assertSame("dd=t.dm:-0,remoteparent=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE", (string)$grandChildContext->getTraceState());
824+
$expected_tracestate = "dd=p:" . $childContext->getSpanId() . ";t.dm:-0,remoteparent=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE";
825+
$this->assertSame($expected_tracestate, (string)$child->getContext()->getTraceState());
826+
$expected_tracestate = "dd=p:" . $grandChildContext->getSpanId(). ";t.dm:-0,remoteparent=yes,rojo=00f067aa0ba902b7,congo=t61rcWkgMzE";
827+
$this->assertSame($expected_tracestate, (string)$grandChildContext->getTraceState());
825828

826829
$grandChildScope->detach();
827830
$grandChild->end();
@@ -853,7 +856,7 @@ public function testAddItemToTracestate()
853856
'_dd.p.congo' => 't61rcWkgMzE',
854857
]);
855858

856-
$this->assertRegularExpression('/^dd=t.dm:-0;t.congo:t61rcWkgMzE$/', (string)$span->getContext()->getTraceState());
859+
$this->assertRegularExpression('/^dd=p:[0-9a-f]{16};t.congo:t61rcWkgMzE;t.dm:-0$/', (string)$span->getContext()->getTraceState());
857860

858861
$traceState = $span->getContext()->getTraceState()->with('rojo', '00f067aa0ba902b7');
859862
$context = SpanContext::create(
@@ -867,7 +870,7 @@ public function testAddItemToTracestate()
867870
->setParent(Context::getCurrent()->withContextValue(Span::wrap($context)))
868871
->startSpan();
869872

870-
$this->assertRegularExpression('/^dd=t.dm:-0;t.congo:t61rcWkgMzE,rojo=00f067aa0ba902b7$/', (string)$child->getContext()->getTraceState());
873+
$this->assertRegularExpression('/^dd=p:[0-9a-f]{16};t.congo:t61rcWkgMzE;t.dm:-0,rojo=00f067aa0ba902b7$/', (string)$child->getContext()->getTraceState());
871874

872875
$child->end();
873876
$span->end();

‎tests/OpenTelemetry/Integration/InteroperabilityTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ public function testW3CInteroperability()
646646
$OTelRootSpan->end();
647647

648648
$this->assertSame("00-ff0000000000051791e0000000000041-$DDChildSpanId-01", $carrier[TraceContextPropagator::TRACEPARENT]);
649-
$this->assertSame('dd=t.dm:-0', $carrier[TraceContextPropagator::TRACESTATE]); // ff00000000000517 is the high 64-bit part of the 128-bit trace id
649+
$this->assertSame("dd=p:$DDChildSpanId;t.dm:-0", $carrier[TraceContextPropagator::TRACESTATE]); // ff00000000000517 is the high 64-bit part of the 128-bit trace id
650650
});
651651

652652
$this->assertSame('10511401530282737729', $traces[0][0]['trace_id']);

‎tests/OpenTelemetry/Integration/SDK/TracerTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public function test_sampler_may_override_parents_trace_state(): void
7979
$span = $tracer->spanBuilder('test.span')->setParent($parentContext)->startSpan();
8080

8181
$this->assertNotEquals($parentTraceState, $span->getContext()->getTraceState());
82-
$this->assertEquals('dd=t.dm:-0,new-key=new_value', (string)$span->getContext()->getTraceState());
82+
$this->assertEquals("dd=p:" . $span->getContext()->getSpanID() . ";t.dm:-0,new-key=new_value", (string)$span->getContext()->getTraceState());
8383
});
8484
}
8585

‎tests/ext/distributed_tracestate_consumption.phpt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED=1
77
<?php
88

99
// This is sampled, hence the mechanism must be retained
10-
$rawTracestate = 'rojo=00f067aa0ba902b7,dd=t.dm:-1;t.congo:t61rcWkgMzE';
10+
$rawTracestate = 'rojo=00f067aa0ba902b7,dd=t.dm:-1;p:0123456789abcdef;t.congo:t61rcWkgMzE';
1111

1212
$span = \DDTrace\start_span();
1313

@@ -22,12 +22,14 @@ $traceParent = "00-$traceId-$parentId-$traceFlags";
2222
]);
2323

2424
var_dump(\DDTrace\generate_distributed_tracing_headers(['tracecontext']));
25+
var_dump(\DDTrace\root_span()->meta["_dd.parent_id"]);
2526

2627
?>
2728
--EXPECTF--
2829
array(2) {
2930
["traceparent"]=>
3031
string(55) "00-%sc151df7d6ee5e2d6-a3978fb9b92502a8-01"
3132
["tracestate"]=>
32-
string(52) "dd=t.dm:-1;t.congo:t61rcWkgMzE,rojo=00f067aa0ba902b7"
33+
string(71) "dd=p:a3978fb9b92502a8;t.dm:-1;t.congo:t61rcWkgMzE,rojo=00f067aa0ba902b7"
3334
}
35+
string(16) "0123456789abcdef"

‎tests/ext/integrations/curl/distributed_tracing_curl.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ echo 'Done.' . PHP_EOL;
5757
[ddtrace] [error] The to be propagated tag '_dd.p.very=looooooooooooooooong' is too long and exceeds the maximum limit of 25 characters and is thus dropped.
5858
b3: %s-%s-1
5959
traceparent: 00-%s-%s
60-
tracestate: dd=o:phpt-test
60+
tracestate: dd=p:%s;o:phpt-test
6161
x-b3-spanid: %s
6262
x-b3-traceid: %s
6363
x-datadog-origin: phpt-test

0 commit comments

Comments
 (0)