diff --git a/.github/labeller.yml b/.github/labeller.yml index 10f89df8c9..d49c4e6a8e 100644 --- a/.github/labeller.yml +++ b/.github/labeller.yml @@ -22,3 +22,6 @@ labels: - name: "tracing" allFilesIn: "(.*\/ext\/.*|.*\/src\/.*|.*\/tests\/.*|.*\/zend_abstract_interface\/.*|docker-compose.yml|.*\/.circleci\/.*)" + + - name: "cat:opentelemetry" + allFilesIn: "(.*\/OpenTelemetry\/.*)" diff --git a/ext/ddtrace.stub.php b/ext/ddtrace.stub.php index 01b33fd366..92192ec5ad 100644 --- a/ext/ddtrace.stub.php +++ b/ext/ddtrace.stub.php @@ -120,6 +120,11 @@ class SpanData { */ public array $peerServiceSources = []; + /** + * @var \Closure|string|null $endCallback A callback to be called when the span is finished, if any. + */ + public \Closure|string|null $endCallback = null; + /** * @var SpanData|null The parent span, or 'null' if there is none */ @@ -424,7 +429,7 @@ function update_span_duration(SpanData $span, float $finishTime = 0): null {} * @param float $startTime Start time of the span in seconds. * @return SpanData The newly created root span */ - function start_trace_span(float $startTime = 0): SpanData {} + function start_trace_span(float $startTime = 0): RootSpanData {} /** * Get the active stack diff --git a/ext/ddtrace_arginfo.h b/ext/ddtrace_arginfo.h index 0d0ed49079..38c3aaa096 100644 --- a/ext/ddtrace_arginfo.h +++ b/ext/ddtrace_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 1d07ca443c39ea7a0a831b062bb0efe4baf69b0f */ + * Stub hash: e001e68def98ce39cf3f6a11337dc109cca4868f */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_DDTrace_trace_method, 0, 3, _IS_BOOL, 0) ZEND_ARG_TYPE_INFO(0, className, IS_STRING, 0) @@ -61,7 +61,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_DDTrace_update_span_duration, 0, ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, finishTime, IS_DOUBLE, 0, "0") ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_DDTrace_start_trace_span, 0, 0, DDTrace\\SpanData, 0) +ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_DDTrace_start_trace_span, 0, 0, DDTrace\\RootSpanData, 0) ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, startTime, IS_DOUBLE, 0, "0") ZEND_END_ARG_INFO() @@ -569,6 +569,13 @@ static zend_class_entry *register_class_DDTrace_SpanData(void) zend_declare_typed_property(class_entry, property_peerServiceSources_name, &property_peerServiceSources_default_value, ZEND_ACC_PUBLIC, NULL, (zend_type) ZEND_TYPE_INIT_MASK(MAY_BE_ARRAY)); zend_string_release(property_peerServiceSources_name); + zval property_endCallback_default_value; + ZVAL_NULL(&property_endCallback_default_value); + zend_string *property_endCallback_name = zend_string_init("endCallback", sizeof("endCallback") - 1, 1); + zend_string *property_endCallback_class_Closure = zend_string_init("Closure", sizeof("Closure")-1, 1); + zend_declare_typed_property(class_entry, property_endCallback_name, &property_endCallback_default_value, ZEND_ACC_PUBLIC, NULL, (zend_type) ZEND_TYPE_INIT_CLASS(property_endCallback_class_Closure, 0, MAY_BE_STRING|MAY_BE_NULL)); + zend_string_release(property_endCallback_name); + zval property_parent_default_value; ZVAL_UNDEF(&property_parent_default_value); zend_string *property_parent_name = zend_string_init("parent", sizeof("parent") - 1, 1); diff --git a/ext/span.c b/ext/span.c index c419033f53..2926149f45 100644 --- a/ext/span.c +++ b/ext/span.c @@ -410,7 +410,65 @@ bool ddtrace_span_alter_root_span_config(zval *old_value, zval *new_value) { } } +bool execute_span_end_callback(ddtrace_span_data *span) { + zval *end_closure_zv = &span->property_end_closure; + + if (end_closure_zv && !ZVAL_IS_NULL(end_closure_zv)) { + uint8_t type = Z_TYPE_P(end_closure_zv); + bool success = false; + + if (type == IS_OBJECT && Z_OBJCE_P(end_closure_zv) == zend_ce_closure) { + zval rv; + zval span_zv; + ZVAL_OBJ(&span_zv, &span->std); + // Get the closure's scope + zend_class_entry *closure_called_scope; + zend_function *closure_func; + zend_object *closure_this; +#if PHP_VERSION_ID < 80000 + Z_OBJ_HANDLER_P(end_closure_zv, get_closure)(end_closure_zv, &closure_called_scope, &closure_func, &closure_this); +#else + Z_OBJ_HANDLER_P(end_closure_zv, get_closure)(Z_OBJ_P(end_closure_zv), &closure_called_scope, &closure_func, &closure_this, true); +#endif + if (closure_called_scope && closure_func->common.fn_flags & ZEND_ACC_STATIC) { + success = zai_symbol_call_static( + closure_called_scope, + zai_str_from_zstr(closure_func->common.function_name), + &rv, + 1, + &span_zv + ); + } else { + success = zai_symbol_call( + closure_called_scope ? ZAI_SYMBOL_SCOPE_CLASS : ZAI_SYMBOL_SCOPE_GLOBAL, + closure_called_scope ? closure_called_scope : NULL, + ZAI_SYMBOL_FUNCTION_CLOSURE, + end_closure_zv, + &rv, + 1, + &span_zv + ); + } + + zval_ptr_dtor(&rv); + } // TODO: string, etc? TBD, else refactor + + if (!success) { + zend_string *hex_trace_id = ddtrace_trace_id_as_hex_string(span->root->trace_id); + zend_string *hex_span_id = ddtrace_span_id_as_hex_string(span->span_id); + LOG(Warn, "Error while calling end closure on span %s [dd.trace_id=%s dd.span_id=%s]", Z_STRVAL_P(&span->property_name), ZSTR_VAL(hex_trace_id), ZSTR_VAL(hex_span_id)); + zend_string_release(hex_trace_id); + zend_string_release(hex_span_id); + } + + return success; + } else { + return true; + } +} + void dd_trace_stop_span_time(ddtrace_span_data *span) { + execute_span_end_callback(span); span->duration = _get_nanoseconds(USE_MONOTONIC_CLOCK) - span->duration_start; } diff --git a/ext/span.h b/ext/span.h index 3f53d56463..0e75c334c8 100644 --- a/ext/span.h +++ b/ext/span.h @@ -50,6 +50,7 @@ typedef union ddtrace_span_properties { }; zval property_links; zval property_peer_service_sources; + zval property_end_closure; union { union ddtrace_span_properties *parent; zval property_parent; diff --git a/src/DDTrace/OpenTelemetry/Context.php b/src/DDTrace/OpenTelemetry/Context.php index 0e28b8486d..0f07c638ce 100644 --- a/src/DDTrace/OpenTelemetry/Context.php +++ b/src/DDTrace/OpenTelemetry/Context.php @@ -4,6 +4,7 @@ namespace OpenTelemetry\Context; +use DDTrace\OpenTelemetry\API\Trace as DDTraceAPI; use DDTrace\SpanData; use DDTrace\Tag; use DDTrace\Util\ObjectKVStore; @@ -191,7 +192,7 @@ private static function activateParent(?SpanData $currentSpan): ContextInterface $OTelCurrentSpan = SDK\Span::startSpan( $currentSpan, - API\SpanContext::create($currentTraceId, $currentSpanId, $traceFlags, $traceState), // $context + DDTraceAPI\SpanContext::createFromLocalSpan($currentSpan, $traceFlags === API\TraceFlags::SAMPLED, $currentSpanId), self::getDDInstrumentationScope(), // $instrumentationScope isset($currentSpan->meta[Tag::SPAN_KIND]) ? self::convertDDSpanKindToOtel($currentSpan->meta[Tag::SPAN_KIND]) : API\SpanKind::KIND_INTERNAL, // $kind API\Span::fromContext($parentContext), // $parentSpan (TODO: Handle null parent span) ? diff --git a/src/DDTrace/OpenTelemetry/Span.php b/src/DDTrace/OpenTelemetry/Span.php index 9f1b5d8461..8a5b7c5945 100644 --- a/src/DDTrace/OpenTelemetry/Span.php +++ b/src/DDTrace/OpenTelemetry/Span.php @@ -111,6 +111,8 @@ private function __construct( $span->links[] = $spanLink; } } + + $span->endCallback = fn () => $this->endOTelSpan(); } /** @@ -401,6 +403,7 @@ public function end(int $endEpochNanos = null): void return; } + $this->span->endCallback = null; // No need for the callback $this->endOTelSpan($endEpochNanos); switch_stack($this->span); diff --git a/src/DDTrace/OpenTelemetry/SpanBuilder.php b/src/DDTrace/OpenTelemetry/SpanBuilder.php index 2d4f85c41b..992d95f8a2 100644 --- a/src/DDTrace/OpenTelemetry/SpanBuilder.php +++ b/src/DDTrace/OpenTelemetry/SpanBuilder.php @@ -138,8 +138,13 @@ public function startSpan(): SpanInterface $parentSpan = Span::fromContext($parentContext); $parentSpanContext = $parentSpan->getContext(); - $span = $parentSpanContext->isValid() ? null : \DDTrace\start_trace_span($this->startEpochNanos); - $traceId = $parentSpanContext->isValid() ? $parentSpanContext->getTraceId() : \DDTrace\root_span()->traceId; + if ($parentSpanContext->isValid()) { + $span = null; + $traceId = $parentSpanContext->getTraceId(); + } else { + $span = \DDTrace\start_trace_span($this->startEpochNanos); + $traceId = $span->traceId; + } $samplingResult = $this ->tracerSharedState @@ -174,7 +179,7 @@ public function startSpan(): SpanInterface } $hexSpanId = $span->hexId(); - $spanContext = DDTraceAPI\SpanContext::createFromLocalSpan($span, $sampled, $traceId, $hexSpanId); + $spanContext = DDTraceAPI\SpanContext::createFromLocalSpan($span, $sampled, $hexSpanId); if (!in_array($samplingDecision, [SamplingResult::RECORD_AND_SAMPLE, SamplingResult::RECORD_ONLY], true)) { return Span::wrap($spanContext); diff --git a/src/DDTrace/OpenTelemetry/SpanContext.php b/src/DDTrace/OpenTelemetry/SpanContext.php index 2baef1e215..175c2cb35b 100644 --- a/src/DDTrace/OpenTelemetry/SpanContext.php +++ b/src/DDTrace/OpenTelemetry/SpanContext.php @@ -5,6 +5,7 @@ namespace DDTrace\OpenTelemetry\API\Trace; use DDTrace\OpenTelemetry\SDK\Trace\Span; +use DDTrace\RootSpanData; use DDTrace\SpanData; use OpenTelemetry\API\Trace as API; use OpenTelemetry\API\Trace\SpanContextInterface; @@ -23,27 +24,22 @@ final class SpanContext implements SpanContextInterface private bool $remote; - private ?string $traceId; - private ?string $spanId; - private bool $isValid = true; + private static function getRootSpan(SpanData $span): RootSpanData + { + while (!($span instanceof RootSpanData) && $span->parent) { + $span = $span->parent; + } + return $span; + } - private function __construct(SpanData $span, bool $sampled, bool $remote, ?string $traceId = null, ?string $spanId = null) + private function __construct(SpanData $span, bool $sampled, bool $remote, ?string $spanId = null) { $this->span = $span; $this->sampled = $sampled; $this->remote = $remote; - $this->traceId = $traceId ?: \DDTrace\root_span()->traceId; $this->spanId = $spanId ?: $this->span->hexId(); - - // TraceId must be exactly 16 bytes (32 chars) and at least one non-zero byte - // SpanId must be exactly 8 bytes (16 chars) and at least one non-zero byte - if (!SpanContextValidator::isValidTraceId($this->traceId) || !SpanContextValidator::isValidSpanId($this->spanId)) { - $this->traceId = SpanContextValidator::INVALID_TRACE; - $this->spanId = SpanContextValidator::INVALID_SPAN; - $this->isValid = false; - } } /** @@ -51,7 +47,8 @@ private function __construct(SpanData $span, bool $sampled, bool $remote, ?strin */ public function getTraceId(): string { - return $this->traceId; + $rootSpan = self::getRootSpan($this->span); + return $rootSpan->traceId; } public function getTraceIdBinary(): string @@ -85,7 +82,7 @@ public function isSampled(): bool public function isValid(): bool { - return $this->isValid; + return SpanContextValidator::isValidTraceId(self::getRootSpan($this->span)->traceId) && SpanContextValidator::isValidSpanId($this->spanId); } public function isRemote(): bool @@ -116,13 +113,12 @@ public static function getInvalid(): SpanContextInterface return API\SpanContext::getInvalid(); } - public static function createFromLocalSpan(SpanData $span, bool $sampled, ?string $traceId = null, ?string $spanId = null) + public static function createFromLocalSpan(SpanData $span, bool $sampled, ?string $spanId = null) { return new self( $span, $sampled, false, - $traceId, $spanId ); } diff --git a/tests/OpenTelemetry/Integration/InteroperabilityTest.php b/tests/OpenTelemetry/Integration/InteroperabilityTest.php index e1b34b76bc..0a97c07f36 100644 --- a/tests/OpenTelemetry/Integration/InteroperabilityTest.php +++ b/tests/OpenTelemetry/Integration/InteroperabilityTest.php @@ -23,8 +23,11 @@ use OpenTelemetry\SDK\Trace\Sampler\AlwaysOnSampler; use OpenTelemetry\SDK\Trace\TracerProvider; use function DDTrace\active_span; +use function DDTrace\add_distributed_tag; use function DDTrace\close_span; use function DDTrace\close_spans_until; +use function DDTrace\generate_distributed_tracing_headers; +use function DDTrace\set_distributed_tracing_context; use function DDTrace\start_span; use function DDTrace\start_trace_span; @@ -910,6 +913,21 @@ public function testAttributesInteroperability() $this->assertEquals(2, $traces[0][0]['metrics']['m2']); } + public function testModifyTracestateAfterEnd() + { + $rootSpan = \DDTrace\start_span(); + + $ddSpan = start_span(); + + /** @var \OpenTelemetry\SDK\Trace\Span $remappedSpan */ + $remappedSpan = Span::getCurrent(); + + $preTraceState = generate_distributed_tracing_headers(['tracecontext'])['tracestate']; + + add_distributed_tag('key', 'value'); + $readOnlyCopy = $remappedSpan->toSpanData(); + $midTraceState = generate_distributed_tracing_headers(['tracecontext'])['tracestate']; + $remappedTraceState = (string) $readOnlyCopy->getContext()->getTraceState(); public function testSpanLinksInteroperabilityFromDatadogSpan() { $traces = $this->isolateTracer(function () { @@ -1001,6 +1019,55 @@ public function testSpanLinksInteroperabilityBothTypes() $datadogSpanLinks = active_span()->links; $this->assertCount(2, $datadogSpanLinks); + $this->assertSame($midTraceState, $remappedTraceState); + + close_span(); // Closes $ddSpan + + add_distributed_tag('my', 'tag'); // Shouldn't affect the closed span + $readOnlyCopy = $remappedSpan->toSpanData(); + $postTraceState = generate_distributed_tracing_headers(['tracecontext'])['tracestate']; + $remappedTraceState = (string) $readOnlyCopy->getContext()->getTraceState(); + + $this->assertSame($midTraceState, $remappedTraceState); + $this->assertNotSame($preTraceState, $postTraceState); + $this->assertNotSame($midTraceState, $postTraceState); + + close_span(); // Closes $rootSpan + } + + public function testModifyTraceIdAfterEnd() + { + $rootSpan = \DDTrace\start_trace_span(); + $ddSpan = start_span(); + + /** @var \OpenTelemetry\SDK\Trace\Span $remappedSpan */ + $remappedSpan = Span::getCurrent(); + + set_distributed_tracing_context("23475823097097752842117799874953798269", "42"); + + $midTraceId = $remappedSpan->getContext()->getTraceId(); + + $this->assertSame($midTraceId, $rootSpan->traceId); + $this->assertSame('11a94770f26bd5a0aafe31aee31ce27d', $midTraceId); + + close_span(); // Closes $ddSpan + + set_distributed_tracing_context("33475823097097752843117799874953798249", "42"); // Note the different trace id + + $this->assertSame('192f3581c8461c79b9d31f028a80e269', $rootSpan->traceId); + + $readOnlyCopy = $remappedSpan->toSpanData(); + + $this->assertNotSame($midTraceId, $rootSpan->traceId); + $this->assertSame('11a94770f26bd5a0aafe31aee31ce27d', $midTraceId); + + $remappedTraceId = $readOnlyCopy->getContext()->getTraceId(); + + $this->assertSame($midTraceId, $remappedTraceId); + $this->assertSame('11a94770f26bd5a0aafe31aee31ce27d', $remappedTraceId); + + close_span(); // Closes $rootSpan + } $this->assertSame('12345678876543211234567887654321', $datadogSpanLinks[0]->traceId); $this->assertSame('8765432112345678', $datadogSpanLinks[0]->spanId); $this->assertSame('dd=t.dm:-0', $datadogSpanLinks[0]->traceState); diff --git a/tests/ext/span_callback_basic.phpt b/tests/ext/span_callback_basic.phpt new file mode 100644 index 0000000000..497a0a6b50 --- /dev/null +++ b/tests/ext/span_callback_basic.phpt @@ -0,0 +1,59 @@ +--TEST-- +Basic end callback behavior +--ENV-- +DD_TRACE_DEBUG=1 +--FILE-- +name\n"; + echo "Passed span: $span->name\n"; + //$span->meta['counter.value'] = $myGlobalClosedSpansCounter; +}; + +$span = \DDTrace\start_span(); +$span->name = "mySpan"; +//\DDTrace\set_end_callback($span, $callback); +$span->endCallback = $callback; + +\DDTrace\close_span(); + +var_dump($myGlobalClosedSpansCounter); +//var_dump(dd_trace_serialize_closed_spans()); + +class mySpanWrapper { + private $mySpan; + + private function __construct(\DDTrace\SpanData $span) { + $this->mySpan = $span; + /* + // Doesn't crash + $span->endCallback = function (\DDTrace\SpanData $span) { + $this->end(); + };*/ + } + + public static function create(\DDTrace\SpanData $span) { + $spanWrapper = new mySpanWrapper($span); + // Crash + $span->endCallback = function (\DDTrace\SpanData $span) use ($spanWrapper) { + $spanWrapper->end(); + }; + } + + public function end() { + echo "This is the end of {$this->mySpan->name}\n"; + } +} + +$span = \DDTrace\start_span(); +$span->name = "mySpan"; +mySpanWrapper::create($span); +\DDTrace\close_span(); + +?> +--EXPECT-- diff --git a/tests/ext/span_callback_static.phpt b/tests/ext/span_callback_static.phpt new file mode 100644 index 0000000000..2a61872dd0 --- /dev/null +++ b/tests/ext/span_callback_static.phpt @@ -0,0 +1,28 @@ +--TEST-- +End callback using a static method +--ENV-- +DD_TRACE_DEBUG=1 +--FILE-- +endCallback = $closure; +\DDTrace\close_span(); + +var_dump($counter); + +?> +--EXPECT--