impl rfc "APM endpoint resource renaming in the tracers" #3415

cataphract · 2025-09-15T15:43:21Z

Description

Implements [RFC-1051] APM endpoint resource renaming. See also DataDog/libdatadog#1219

Reviewer checklist

Test coverage seems ok.
Appropriate labels assigned.

codecov-commenter · 2025-09-15T16:06:00Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.74%. Comparing base (b3136db) to head (98ead20).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3415      +/-   ##
==========================================
- Coverage   61.90%   61.74%   -0.17%     
==========================================
  Files         141      141              
  Lines       12481    12481              
  Branches     1630     1630              
==========================================
- Hits         7726     7706      -20     
- Misses       4033     4054      +21     
+ Partials      722      721       -1

see 3 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b3136db...98ead20. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

pr-commenter · 2025-09-15T17:41:56Z

Benchmarks [ tracer ]

Benchmark execution time: 2025-09-18 13:49:12

Comparing candidate commit 98ead20 in PR branch glopes/http-endpoint with baseline commit b3136db in branch master.

Found 1 performance improvements and 5 performance regressions! Performance is the same for 188 metrics, 0 unstable metrics.

scenario:ContextPropagationBench/benchExtractHeaders128Bit

🟥 execution_time [+18.458ns; +73.542ns] or [+2.173%; +8.659%]

scenario:ContextPropagationBench/benchInject64Bit-opcache

🟥 execution_time [+168.498ns; +367.502ns] or [+2.267%; +4.945%]

scenario:PDOBench/benchPDOOverhead

🟥 execution_time [+6.911µs; +11.229µs] or [+2.581%; +4.194%]

scenario:PDOBench/benchPDOOverheadWithDBM

🟥 execution_time [+7.005µs; +10.280µs] or [+2.612%; +3.833%]

scenario:PHPRedisBench/benchRedisOverhead

🟥 execution_time [+30.860µs; +61.471µs] or [+2.878%; +5.733%]

scenario:TraceSerializationBench/benchSerializeTrace

🟩 execution_time [-29.386µs; -13.014µs] or [-6.297%; -2.789%]

bwoebi · 2025-09-16T16:27:05Z

ext/span.c

+            uintptr_t offset = XtOffsetOf(ddtrace_root_span_data, span);
+            ddtrace_root_span_data *root_span = (ddtrace_root_span_data *)((char *)span - offset);
+            ddtrace_maybe_add_guessed_endpoint_tag(root_span);


Suggested change

uintptr_t offset = XtOffsetOf(ddtrace_root_span_data, span);

ddtrace_root_span_data *root_span = (ddtrace_root_span_data *)((char *)span - offset);

ddtrace_maybe_add_guessed_endpoint_tag(root_span);

ddtrace_maybe_add_guessed_endpoint_tag(ROOTSPANDATA(&span->std));

bwoebi · 2025-09-16T16:29:30Z

ext/span.c

+        if (get_DD_TRACE_RESOURCE_RENAMING_ENABLED() && get_DD_TRACE_SIDECAR_TRACE_SENDER()) {
+            // the purpose is client-computed stats for /v0.6/stats, so sidecar is required


Um, in case we don't manually compute, the agent will do that task and should?
Also we don't ever drop that tag when computing - Let's always include the tag, if only for consistency.

Suggested change

if (get_DD_TRACE_RESOURCE_RENAMING_ENABLED() && get_DD_TRACE_SIDECAR_TRACE_SENDER()) {

// the purpose is client-computed stats for /v0.6/stats, so sidecar is required

if (get_DD_TRACE_RESOURCE_RENAMING_ENABLED()) {

bwoebi · 2025-09-16T16:30:58Z

ext/span.c

 #include "span.h"

 #include <SAPI.h>
+#include <Zend/zend_portability.h>


Suggested change

#include <Zend/zend_portability.h>

bwoebi · 2025-09-16T16:34:02Z

ext/endpoint_guessing.c

+
+    if (zend_hash_str_add(meta, ZEND_STRL("http.endpoint"), &endpoint_zv) == NULL) {
+        zval_dtor(&endpoint_zv);
+        return;


Suggested change

return;

Nit: redundant

bwoebi · 2025-09-16T16:43:42Z

ext/endpoint_guessing.c

+    // handle the first char: is_int does not allow a leading 0
+    if (len > 0) {
+        char c = path[0];
+        found_special_char = found_special_char || is_str_special(c);
+        found_digit = found_digit || is_digit(c);
+
+        uint8_t digit_mask = bool_to_mask(is_digit(c)) & (COMPONENT_IS_INT_ID | COMPONENT_IS_HEX | COMPONENT_IS_HEX_ID);
+
+        // first char for is_int must be 1–9
+        uint8_t is_int_mask = bool_to_mask(is_nonzero_digit(c)) & COMPONENT_IS_INT;
+
+        uint8_t hex_alpha_mask = bool_to_mask(is_hex_alpha(c)) & (COMPONENT_IS_HEX | COMPONENT_IS_HEX_ID);
+
+        uint8_t delimiter_mask = bool_to_mask(is_delim(c)) & (COMPONENT_IS_INT_ID | COMPONENT_IS_HEX_ID);
+
+        viable_components &= (digit_mask | is_int_mask | hex_alpha_mask | delimiter_mask | COMPONENT_IS_STR);
+    }
+
+    // Process remaining characters
+    for (size_t i = 1; i < len; ++i) {


Suggested change

// handle the first char: is_int does not allow a leading 0

if (len > 0) {

char c = path[0];

found_special_char = found_special_char || is_str_special(c);

found_digit = found_digit || is_digit(c);

uint8_t digit_mask = bool_to_mask(is_digit(c)) & (COMPONENT_IS_INT_ID | COMPONENT_IS_HEX | COMPONENT_IS_HEX_ID);

// first char for is_int must be 1–9

uint8_t is_int_mask = bool_to_mask(is_nonzero_digit(c)) & COMPONENT_IS_INT;

uint8_t hex_alpha_mask = bool_to_mask(is_hex_alpha(c)) & (COMPONENT_IS_HEX | COMPONENT_IS_HEX_ID);

uint8_t delimiter_mask = bool_to_mask(is_delim(c)) & (COMPONENT_IS_INT_ID | COMPONENT_IS_HEX_ID);

viable_components &= (digit_mask | is_int_mask | hex_alpha_mask | delimiter_mask | COMPONENT_IS_STR);

}

// Process remaining characters

for (size_t i = 1; i < len; ++i) {

// first char for is_int must be 1–9

viable_components &= (path[0] != '0') * COMPONENT_IS_INT;

for (size_t i = 0; i < len; ++i) {

Let's avoid the duplication

ext/endpoint_guessing.c

bwoebi · 2025-09-16T16:48:41Z

ext/endpoint_guessing.c

+
+    zval endpoint_zv;
+
+    zval *url = zend_hash_str_find(meta, ZEND_STRL("http.url"));


Suggested change

zval *url = zend_hash_str_find(meta, ZEND_STRL("http.url"));

zval *url = zend_hash_str_find_deref(meta, ZEND_STRL("http.url"));

bwoebi · 2025-09-16T16:52:40Z

ext/endpoint_guessing.c

+    if (!url || Z_TYPE_P(url) != IS_STRING) {
+        // "In case the url is not available, a default value of / should be used for the endpoint tag."
+        ZVAL_STRING(&endpoint_zv, "/");
+    } else {
+        zend_string* endpoint = guess_endpoint(Z_STRVAL_P(url), Z_STRLEN_P(url));
+        ZVAL_STR(&endpoint_zv, endpoint);
+    }
+
+    if (zend_hash_str_add(meta, ZEND_STRL("http.endpoint"), &endpoint_zv) == NULL) {
+        zval_dtor(&endpoint_zv);


Suggested change

if (!url || Z_TYPE_P(url) != IS_STRING) {

// "In case the url is not available, a default value of / should be used for the endpoint tag."

ZVAL_STRING(&endpoint_zv, "/");

} else {

zend_string* endpoint = guess_endpoint(Z_STRVAL_P(url), Z_STRLEN_P(url));

ZVAL_STR(&endpoint_zv, endpoint);

}

if (zend_hash_str_add(meta, ZEND_STRL("http.endpoint"), &endpoint_zv) == NULL) {

zval_dtor(&endpoint_zv);

zval *endpoint, endpoint_zv;

if ((endpoint = zend_hash_str_add(meta, ZEND_STRL("http.endpoint"), &endpoint_zv))) {

if (!url || Z_TYPE_P(url) != IS_STRING) {

// "In case the url is not available, a default value of / should be used for the endpoint tag."

ZVAL_STRING(endpoint, "/");

} else {

ZVAL_STR(endpoint, guess_endpoint(Z_STRVAL_P(url), Z_STRLEN_P(url)));

}

Simple way to only compute when needed, rather than destroying later.

bwoebi · 2025-09-16T16:55:40Z

Looks good so far - test failure are due to the sidecar requirement, but I anyway would not make it dependent on the sidecar anyway.
Some code improvements suggested, but nothing blocking.

cataphract · 2025-09-17T15:06:24Z

Thanks @bwoebi I'll apply your changes. Do you think you could look at DataDog/libdatadog#1219 as well?

bwoebi · 2025-09-17T16:29:38Z

@cataphract That's rather something for the data-pipeline team to review than for me.

cataphract requested a review from a team as a code owner September 15, 2025 15:43

cataphract force-pushed the glopes/http-endpoint branch 3 times, most recently from 87e685b to 9222b89 Compare September 15, 2025 15:55

cataphract force-pushed the glopes/http-endpoint branch from 9222b89 to 3ae54d2 Compare September 15, 2025 16:19

impl rfc "APM endpoint resource renaming in the tracers"

f409ea3

cataphract force-pushed the glopes/http-endpoint branch from 3ae54d2 to f409ea3 Compare September 15, 2025 16:34

bwoebi reviewed Sep 16, 2025

View reviewed changes

ext/endpoint_guessing.c Show resolved Hide resolved

bwoebi reviewed Sep 16, 2025

View reviewed changes

Address PR comments

c2c40ce

cataphract requested review from a team as code owners September 18, 2025 11:20

Adapt to changes in libdatadog

98ead20

cataphract force-pushed the glopes/http-endpoint branch from fd548e6 to 98ead20 Compare September 18, 2025 12:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

impl rfc "APM endpoint resource renaming in the tracers" #3415

impl rfc "APM endpoint resource renaming in the tracers" #3415

Uh oh!

cataphract commented Sep 15, 2025

Uh oh!

codecov-commenter commented Sep 15, 2025 •

edited

Loading

Uh oh!

pr-commenter bot commented Sep 15, 2025 •

edited

Loading

Uh oh!

bwoebi Sep 16, 2025

Uh oh!

bwoebi Sep 16, 2025

Uh oh!

bwoebi Sep 16, 2025

Uh oh!

bwoebi Sep 16, 2025

Uh oh!

bwoebi Sep 16, 2025 •

edited

Loading

Uh oh!

Uh oh!

bwoebi Sep 16, 2025

Uh oh!

bwoebi Sep 16, 2025 •

edited

Loading

Uh oh!

bwoebi commented Sep 16, 2025

Uh oh!

cataphract commented Sep 17, 2025

Uh oh!

bwoebi commented Sep 17, 2025

Uh oh!

Uh oh!

		if (get_DD_TRACE_RESOURCE_RENAMING_ENABLED() && get_DD_TRACE_SIDECAR_TRACE_SENDER()) {
		// the purpose is client-computed stats for /v0.6/stats, so sidecar is required


		zval endpoint_zv;

		zval *url = zend_hash_str_find(meta, ZEND_STRL("http.url"));

	zval *url = zend_hash_str_find(meta, ZEND_STRL("http.url"));
	zval *url = zend_hash_str_find_deref(meta, ZEND_STRL("http.url"));

impl rfc "APM endpoint resource renaming in the tracers" #3415

Are you sure you want to change the base?

impl rfc "APM endpoint resource renaming in the tracers" #3415

Uh oh!

Conversation

cataphract commented Sep 15, 2025

Description

Reviewer checklist

Uh oh!

codecov-commenter commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pr-commenter bot commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks [ tracer ]

scenario:ContextPropagationBench/benchExtractHeaders128Bit

scenario:ContextPropagationBench/benchInject64Bit-opcache

scenario:PDOBench/benchPDOOverhead

scenario:PDOBench/benchPDOOverheadWithDBM

scenario:PHPRedisBench/benchRedisOverhead

scenario:TraceSerializationBench/benchSerializeTrace

Uh oh!

bwoebi Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

bwoebi Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

bwoebi Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

bwoebi Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

bwoebi Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bwoebi Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

bwoebi Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bwoebi commented Sep 16, 2025

Uh oh!

cataphract commented Sep 17, 2025

Uh oh!

bwoebi commented Sep 17, 2025

Uh oh!

Uh oh!

codecov-commenter commented Sep 15, 2025 •

edited

Loading

pr-commenter bot commented Sep 15, 2025 •

edited

Loading

bwoebi Sep 16, 2025 •

edited

Loading

bwoebi Sep 16, 2025 •

edited

Loading