WIP: feat(tracer): align C span code with v1-compatible stub API (v2 groundwork) - #4046
WIP: feat(tracer): align C span code with v1-compatible stub API (v2 groundwork)#4046Leiyks wants to merge 20 commits into
Conversation
|
Benchmarks [ tracer ]Benchmark execution time: 2026-08-26 17:32:07 Comparing candidate commit 24cdec1 in PR branch Some scenarios are present only in baseline or only in candidate runs. If you didn't create or remove some scenarios in your branch, this maybe a sign of crashed benchmarks 💥💥💥 Scenarios present only in baseline:
Found 1 performance improvements and 26 performance regressions! Performance is the same for 163 metrics, 0 unstable metrics.
|
2531886 to
b29cd49
Compare
10c7edf to
84351df
Compare
84351df to
73c262e
Compare
Signed-off-by: Alexandre Rulleau <alexandre.rulleau@datadoghq.com>
Signed-off-by: Alexandre Rulleau <alexandre.rulleau@datadoghq.com>
Bring the tracer C code into consistency with the already-rewritten v1-compatible span stub (tracer/ddtrace.stub.php), keeping the v04 wire format byte-identical (libdatadog v1 FFI is not yet available). - Regenerate ddtrace_arginfo.h from the stub; reorder the ddtrace_span_properties / ddtrace_root_span_data / ddtrace_span_stack structs to match the new property declaration order (PHP property offsets are bound to these C struct fields). - Drop SpanEvent/SpanLink JsonSerializable + jsonSerialize(); relocate that logic into serializer.c so the _dd.span_links / events meta blobs are produced with identical bytes. - Make component/spanKind sourced from the new SpanData properties, translated back into meta (span.kind/component) at serialize time so the v04 wire is unchanged. Register the new SpanKind class. - Remove dead code for stub-removed functions and the DD_TRACE_WARN_LEGACY_DD_TRACE config key; complete the userland integration-analytics removal (Integration.php + call sites); delete tests orphaned by removed functions and rewrite the dd_trace_reset helper tests without gutting their coverage. Refs APMLP-1197.
…DD_TRACE_WARN_LEGACY_DD_TRACE The config key DD_TRACE_WARN_LEGACY_DD_TRACE was removed from tracer/configuration.h, but metadata/supported-configurations.json was not regenerated, causing the 'Configuration Consistency' CI job to fail. Ran tooling/generate-supported-configurations.sh to sync.
RootSpanData extends SpanData, which already declares $env and $version. The v1-stub alignment accidentally re-declared them on RootSpanData (they were never on RootSpanData on master, and the ddtrace_root_span_data C struct has no separate env/version slots -- they are inherited SpanData slots). On PHP < 8.1 this redundant child redeclaration corrupts the RootSpanData property table: env/version are var_dump'd twice and propagatedTags loses its default, breaking tests/ext/active_span.phpt and span_clone.phpt on 7.0-8.0 (they passed on 8.1+). Removing the redeclaration from the stub and arginfo aligns stub=arginfo=C struct and restores consistent output across all versions; env/version remain available on RootSpanData via inheritance.
…on PHP 7
The new `attributes` property (stub default `= []`) was not materialized at
span/stack creation. On PHP < 8.0 array-typed property defaults become null
(see the ZVAL_EMPTY_ARRAY shim in functions.c) and are only lazily turned into
arrays when touched; since `attributes` is never touched during a plain span
lifecycle it var_dump'd as NULL on 7.0-7.4 (array(0){} on 8.0+), breaking
tests/ext/active_span.phpt and span_clone.phpt. Force-materialize it in
ddtrace_init_span and dd_alloc_span_stack (guarded to PHP < 8.0) so it is a
consistent empty array on every supported version, matching its stub default.
The chore that dropped integration (App Analytics) auto-tagging removed Integration::addTraceAnalyticsIfEnabled and the per-integration DD_TRACE_<integration>_ANALYTICS_* config, so the _dd1.sr.eausr metric is no longer auto-added to integration spans. Align the tests: - Delete the 25 TraceSearchConfigTest.php files (Laravel, Lumen, Symfony, ZendFramework, Custom) whose sole subject was the removed per-integration trace-analytics config. - Drop the per-integration analytics setup + _dd1.sr.eausr metric assertions from the mixed PDO and SQLSRV integration tests, preserving all other span coverage. - Remove Curl's dedicated testTraceAnalytics method + its data provider (per-integration analytics config matrix) and the stale DD_CURL_ANALYTICS_ENABLED teardown-cleanup entries in Curl/Guzzle. The user-facing App Analytics API (Span::setMetric(Tag::ANALYTICS_KEY) -> TraceAnalyticsProcessor, Tag::ANALYTICS_KEY constant) is intentionally kept by this branch, so its tests (SpanTest, TraceAnalyticsProcessorTest, UserAvailableConstantsTest, OpenTelemetry, ext/test_special_attributes) are left intact.
Comment-only cleanup of the C span-consistency changes: trim the multi-line explanatory blocks in serializer.c, span.c, handlers_httpstreams.c and tracer_telemetry.c down to concise 1-2 line comments. No behavior change.
The user-facing App Analytics API is now a deprecated no-op: it remains callable (Tag::ANALYTICS_KEY, TraceAnalyticsProcessor, DD_TRACE_ANALYTICS_ENABLED stay defined) but no longer applies any behavior nor emits the _dd1.sr.eausr metric in-process or on the wire. - serializer.c: drop the DD_TRACE_ANALYTICS_ENABLED/web-analytics emission, stop converting the analytics.event meta key to the metric (still consumed), and skip _dd1.sr.eausr in the metrics serialization loop. - TraceAnalyticsProcessor::normalizeAnalyticsValue is now an empty no-op; Tag::ANALYTICS_KEY, the processor, and the api stubs are marked @deprecated. - Tests rewritten to assert the API is callable and emits no _dd1.sr.eausr.
Bump the libdatadog submodule to PR #2156's head (938c110), which brings in the v1 sidecar span encoder/decoder (#2145, #2174) via its main base. Regenerate components-rs/{common,sidecar}.h with cbindgen (exposes the new ddog_sidecar_send_trace_v1_shm / _bytes entrypoints and the ASM_RAW_RESPONSE_BODY remote-config capability) and mirror libdatadog's consolidated [workspace.dependencies] into the root Cargo.toml so the path-dependency crates resolve their { workspace = true } inheritance. Wire format is unchanged: v04 remains the default send path.
… V1 wire Add DD_TRACE_AGENT_PROTOCOL_VERSION (default "0.4"). When set to "1"/"1.0", assemble ddog_TracerMetadataV1 and call ddog_send_traces_to_sidecar_v1; otherwise keep the unchanged V0.4 sidecar send. Hard gate; no /info negotiation.
Regenerate components-rs/*.h and Cargo.lock against the rebased libdatadog submodule (v1 send FFI branch merged with libdatadog main). Picks up the RemoteConfig DEBUG product enum, the v1 FFI comment trims, the agent_info doc-comment tightening, and the zrip/ring lockfile additions.
73c262e to
0d3d03d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d3d03d6f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| static bool dd_v1_native_span_enabled(void) { | ||
| zend_string *pv = get_global_DD_TRACE_AGENT_PROTOCOL_VERSION(); | ||
| return zend_string_equals_literal(pv, "1") || zend_string_equals_literal(pv, "1.0"); |
There was a problem hiding this comment.
Gate native fields on the negotiated V1 path
When the protocol setting is 1/1.0 but agent info is unavailable or does not advertise /v1.0/traces, this helper still moves links and events exclusively into the native V1 fields, while auto_flush.c:75-100 deliberately falls back to the V0.4 sender. The V0.4 encoder expects the JSON values in _dd.span_links and events, so those spans silently lose all links and events during startup, with older agents, and whenever the non-sidecar sender is used. Base this decision on the same negotiated use_v1 condition as the sender, rather than the configuration value alone.
Useful? React with 👍 / 👎.
| zval *meta_env = pre->meta ? zend_hash_str_find(pre->meta, ZEND_STRL("env")) : NULL; | ||
| if (meta_env) { | ||
| pre->env_deprecated = true; | ||
| LOG(DEPRECATED, "Using \"env\" in meta is deprecated. Instead specify the env property directly on the span."); | ||
| zend_string *str = datadog_convert_to_str(meta_env); | ||
| pre->env_deprecated = false; | ||
| zval *prop_env = &span->property_env; |
There was a problem hiding this comment.
Preserve legacy env overrides during precomputation
When a span sets the supported legacy $span->meta['env'] override while its env property contains an inherited process environment, meta_env is now looked up but ignored. Consequently client-side stats are aggregated under the property environment via ddtrace_feed_span_to_concentrator() instead of the environment represented by the span, and the later property write in serializer.c:1914-1915 can replace the meta override on the wire. Restore the documented meta-first selection, even if it remains deprecated.
Useful? React with 👍 / 👎.
| #if PHP_VERSION_ID < 80000 | ||
| // PHP 7 array-typed properties default to null; materialize `attributes` to match its | ||
| // `= []` stub default (as on PHP 8). | ||
| ddtrace_property_array(&span->property_attributes); |
There was a problem hiding this comment.
Initialize attributes in the PHP 7 object creators
On PHP 7 this initialization only runs for spans allocated through the tracer's internal ddtrace_init_span() helper. Public direct construction such as new DDTrace\SpanData() or new DDTrace\RootSpanData() goes through the create_object callbacks in functions.c:244-269 instead, leaving the new attributes property as null because array defaults are deliberately rewritten to null on PHP 7. Directly constructed SpanStack objects have the same problem because their initialization was placed only in dd_alloc_span_stack(). Initialize these properties in the respective object creators so every construction path satisfies the declared array API.
Useful? React with 👍 / 👎.
Stages 3+4 of the v1-native migration. The sidecar sender now always builds a
native libdatadog v1 TracerPayload via the new builder FFI and sends it with
ddog_send_traces_to_sidecar_v1; the sidecar negotiates v1-vs-v0.4 with the agent
and downgrades as needed, so the tracer-side DD_TRACE_AGENT_PROTOCOL_VERSION gate
is removed.
- serializer.c: convert each fully-built v0.4 span into the v1 builder (fields
and meta/metrics/meta_struct via the v0.4 read getters, native links/events
from the still-alive PHP span). Promoted fields (env/version/component/
span.kind) use the dedicated setters and are excluded from the attribute map;
chunk-level fields (sampling priority/origin/mechanism/128-bit trace-id/
dropped) are routed to the chunk. Array/object attribute values, which have no
native v1 attribute variant, are preserved as a JSON string. The v0.4 build
stays byte-identical for the in-process sender and functions.c introspection.
- auto_flush.c: sidecar path always builds the v1 builder and sends v1; shrunk
ddog_TracerMetadataV1 {hostname,env,app_version,runtime_id,git_commit_sha}. The
in-process sender (PHP <= 8.2) stays on v0.4, unchanged.
- Remove the now-dead DD_TRACE_AGENT_PROTOCOL_VERSION config key and the
ddog_agent_info_has_endpoint getter; regenerate datadog.h and
supported-configurations.json.
- Bump libdatadog to the v1 send FFI (317c98d28) and regenerate cbindgen
headers. Add a hand-written prototypes header (sidecar_v1_macro_ffi.h) for the
macro-generated v1 setters that cbindgen cannot emit.
- Update the request-replayer span-event tests to assert native span events.
libdatadog now defines the v1 builder setters as explicit fns instead of declarative-macro-generated ones, so cbindgen emits them into the generated components-rs/sidecar.h. Bump the libdatadog submodule, regenerate sidecar.h (now carries ddog_v1_set_span_* / ddog_v1_add_span_attr_* / ddog_v1_add_event_attr_*), and remove the interim components-rs/sidecar_v1_macro_ffi.h and its include in serializer.c.
THIS IS STILL WORK IS PROGRESS
What
Makes the PHP tracer build and send native V1 traces on the sidecar path,
using libdatadog's new index-based V1 span-builder FFI. The sidecar handles
protocol negotiation and downgrade, so the tracer always emits v1 on that path
and older agents keep working transparently.
Depends on libdatadog #2311
(pinned via the submodule bump in this PR).
Details
ddog_TracerPayloadV1Builder: strings go through the interned string table(with a per-
zend_string*id cache for the fast path), promoted fields(service / name / resource / type / env / version / component / ids / times /
error / kind) are set directly, attributes are emitted as typed values
(string / int / double / bool / bytes), and span links / span events are built
natively rather than folded into meta.
the sidecar negotiates against the agent
/infoand downgrades to v0.4 whenthe agent lacks v1 support (see [Bug]: PHPRedis infinite flushing loop #2311). In-process (PHP ≤ 8.2) stays v0.4.
DD_TRACE_AGENT_PROTOCOL_VERSIONgate — protocol selection isno longer a tracer-side config knob; the sidecar decides.
explicit fns), so the interim hand-written
components-rs/sidecar_v1_macro_ffi.his gone.
Known follow-ups
AnyValue-array FFI yet).process_tagsstill travel via span meta./v1.0/traceswire is CI-verified.