Skip to content

Commit 3ef1113

Browse files
authored
fix(stats): scan all peer tag keys sent by the agent (#4102)
DDTRACE_MAX_PEER_TAGS bounds how many peer tags a single span collects, but it was also truncating the key list from /info before lookup. The agent's list is sorted alphabetically and already longer than the cap, so keys sorting last (out.host, peer.hostname, peer.service, server.address) were silently ignored.
1 parent 194efce commit 3ef1113

2 files changed

Lines changed: 20 additions & 8 deletions

File tree

tests/ext/request-replayer/client_side_stats_peer_tags.phpt

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,30 @@
11
--TEST--
2-
Client-side SHM span stats include peer tags configured via agent info
2+
Client-side SHM span stats include peer tags configured via agent info, wherever they sit in the key list
33
--SKIPIF--
44
<?php include __DIR__ . '/../includes/skipif_no_dev_env.inc'; ?>
55
<?php
66
if (PHP_VERSION_ID < 70400) die("skip: Before PHP 7.4, the skip-task would cause the sidecar to fetch the info already.");
77
if (PHP_VERSION_ID >= 80100) {
88
echo "nocache\n";
99
}
10+
// Keys are looked up in the order the agent sent them. The real list is derived from the agent's
11+
// semantic registry, is sorted alphabetically and is currently 44 keys long, so pad the list here
12+
// and keep one matching key at each end: that catches any truncation of the key scan (a regression
13+
// that dropped, among others, network.destination.name and out.host).
14+
$peerTags = ['db.hostname'];
15+
for ($i = 0; $i < 40; $i++) {
16+
$peerTags[] = sprintf('peer.unused.%02d', $i);
17+
}
18+
$peerTags[] = 'out.host';
19+
1020
$ctx = stream_context_create([
1121
'http' => [
1222
'method' => 'PUT',
1323
'header' => [
1424
'Content-Type: application/json',
1525
'X-Datadog-Test-Session-Token: client_side_stats_peer_tags',
1626
],
17-
'content' => json_encode(['version' => '7.65.0', 'client_drop_p0s' => true, 'peer_tags' => ['db.hostname']]),
27+
'content' => json_encode(['version' => '7.65.0', 'client_drop_p0s' => true, 'peer_tags' => $peerTags]),
1828
]
1929
]);
2030
file_get_contents('http://request-replayer/set-agent-info', false, $ctx);
@@ -45,12 +55,15 @@ dd_trace_internal_fn('await_agent_info');
4555
// Now create the span whose stats we want to inspect. When this span is fed to the
4656
// concentrator, ddog_apply_agent_info_concentrator_config() is called first, picks up
4757
// the peer_tags update from the SHM, and the concentrator extracts db.hostname from meta.
58+
// db.hostname is the first key the agent sent and out.host the last one (index 41): both must end
59+
// up in the stats payload, no matter where they sit in the list.
4860
$root = \DDTrace\start_trace_span();
4961
$root->name = "web.request";
5062
$root->resource = "GET /db";
5163
$root->service = "stats-test-service";
5264
$root->meta['span.kind'] = 'client';
5365
$root->meta['db.hostname'] = 'my-db-host';
66+
$root->meta['out.host'] = 'my-remote-host';
5467
\DDTrace\close_span();
5568

5669
dd_trace_internal_fn('synchronous_flush');
@@ -90,4 +103,4 @@ if (!$found) {
90103

91104
?>
92105
--EXPECT--
93-
peer_tags: ["db.hostname:my-db-host"]
106+
peer_tags: ["db.hostname:my-db-host","out.host:my-remote-host"]

tracer/span_stats.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ static const size_t GRPC_META_KEY_LENS[] = {
3434
sizeof("grpc.status.code") - 1,
3535
};
3636

37-
// Maximum number of peer tags we handle per span (a hard cap to bound stack usage).
37+
// Maximum number of peer tags we collect per span (a hard cap to bound stack usage).
38+
// This bounds matched tags on a single span, not the number of peer tag keys configured by the
39+
// agent, which is larger and must be scanned in full.
3840
#define DDTRACE_MAX_PEER_TAGS 32
3941

4042
void ddtrace_precompute_span(ddtrace_span_data *span, ddtrace_span_precomputed *pre) {
@@ -346,11 +348,8 @@ static void ddtrace_span_concentrator_feed_cb(const ddog_SpanConcentrator *c, vo
346348
if (pre->span_kind && (zend_string_equals_literal(pre->span_kind, "client") || zend_string_equals_literal(pre->span_kind, "producer") || zend_string_equals_literal(pre->span_kind, "consumer"))) {
347349
size_t peer_tag_keys_count = 0;
348350
const ddog_CharSlice *peer_tag_keys = ddog_span_concentrator_peer_tag_keys(c, &peer_tag_keys_count);
349-
if (peer_tag_keys_count > DDTRACE_MAX_PEER_TAGS) {
350-
peer_tag_keys_count = DDTRACE_MAX_PEER_TAGS;
351-
}
352351
if (peer_tag_keys_count > 0 && peer_tag_keys) {
353-
for (size_t i = 0; i < peer_tag_keys_count; i++) {
352+
for (size_t i = 0; i < peer_tag_keys_count && actual_peer_tags < DDTRACE_MAX_PEER_TAGS; i++) {
354353
const ddog_CharSlice *k = &peer_tag_keys[i];
355354
zval *val = zend_hash_str_find(pre->meta, k->ptr, k->len);
356355
if (val && Z_TYPE_P(val) == IS_STRING) {

0 commit comments

Comments
 (0)