Skip to content

Commit 53e20b5

Browse files
authored
fix(trace-stats)!: add grpc_method to aggregation key (#2151)
# What does this PR do? Removes \`grpc_method\` / \`rpc.method\` entirely from the OTLP trace metrics pipeline. Previously \`grpc_method\` was a sidecar on \`GroupedStats\` — present in the OTLP metric payload as an explicit attribute, but not part of the aggregation key. This PR removes it from both: - **\`FixedAggregationKey\`** — it is no longer an aggregation dimension. - **\`OtlpExactGroup\`** — it is no longer carried in the per-cell sidecar. - **\`build_attributes()\`** — \`rpc.method\` is no longer emitted on OTLP histogram data points. ## Rationale For gRPC spans, \`span.resource\` already carries the full method path (e.g. \`/package.Service/Method\`). Emitting \`rpc.method\` as a separate OTLP attribute is redundant — \`span.resource\` (mapped to \`span.name\` on the data point) is the authoritative carrier of gRPC method identity and is already an aggregation dimension. Adding \`rpc.method\` as a separate dimension would also increase cardinality for gRPC services without providing any information not already present on the metric. Note: \`rpc.response.status_code\` (from \`grpc.status.code\`) is **kept** as a first-class aggregation dimension — it is not redundant with any existing field. # Motivation Correctness and cleanliness for the OTLP trace metrics feature landed in #2067. # Risk **Breaking changes (major semver):** - \`FixedAggregationKey<T>\` loses its \`grpc_method: T\` field. Callers constructing the struct by name must remove the field. - \`OtlpExactGroup\` loses its \`grpc_method\` field. - \`build_attributes()\` loses the \`grpc_method\` parameter. **Blast radius is limited** because: 1. The OTLP trace metrics feature (#2067) landed on \`main\` after the v36 release (June 19). No released version is affected — this only exists in unreleased code. 2. The OTLP metrics path is opt-in: the sidecar only activates it when \`set_otlp_metrics_endpoint\` is configured. No SDK enables this today. 3. Agent \`/v0.6/stats\` protobuf wire format is unchanged — \`ClientGroupedStats\` has no \`grpc_method\` field. **Urgency: low.** No live production systems are affected. This must land before the next release that ships the OTLP metrics feature. # How to test the change? ``` cargo nextest run -p libdd-trace-stats -p datadog-ipc -p libdd-data-pipeline ``` Co-authored-by: munir.abdinur <munir.abdinur@datadoghq.com>
1 parent c2751ef commit 53e20b5

4 files changed

Lines changed: 7 additions & 60 deletions

File tree

datadog-ipc/src/shm_stats.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ use libdd_trace_stats::span_concentrator::{FixedAggregationKey, FlushableConcent
6060

6161
use crate::platform::{FileBackedHandle, MappedMem, NamedShmHandle};
6262

63-
const SHM_VERSION: u32 = 1;
63+
const SHM_VERSION: u32 = 2;
6464

6565
/// Maximum peer-tag (key, value) pairs per aggregation slot.
6666
pub const MAX_PEER_TAGS: usize = 16;

libdd-data-pipeline/src/otlp/metrics.rs

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ pub fn map_stats_to_otlp_metrics(
5858
continue;
5959
};
6060
data_points.push(json!({
61-
"attributes": build_attributes(group, &exact.grpc_method, is_error, resource_info, otel_trace_semantics_enabled),
61+
"attributes": build_attributes(group, is_error, resource_info, otel_trace_semantics_enabled),
6262
"startTimeUnixNano": b.bucket.start.to_string(),
6363
"timeUnixNano": end.to_string(),
6464
"count": cell.count.to_string(),
@@ -120,7 +120,6 @@ fn sketch_bucket_counts(sketch: &DDSketch) -> Vec<String> {
120120

121121
fn build_attributes(
122122
group: &pb::ClientGroupedStats,
123-
grpc_method: &str,
124123
is_error: bool,
125124
resource_info: &OtlpResourceInfo,
126125
otel_trace_semantics_enabled: bool,
@@ -146,7 +145,6 @@ fn build_attributes(
146145
push("span.kind", &group.span_kind);
147146
push("http.request.method", &group.http_method);
148147
push("http.route", &group.http_endpoint);
149-
push("rpc.method", grpc_method);
150148
push("rpc.response.status_code", &group.grpc_status_code);
151149
for tag in &group.peer_tags {
152150
if let Some((k, v)) = tag.split_once(':') {
@@ -347,22 +345,10 @@ mod tests {
347345
OtlpExactGroup {
348346
ok: cell(ok_ns),
349347
error: cell(err_ns),
350-
grpc_method: String::new(),
351348
},
352349
)
353350
}
354351

355-
fn group_pair_with_grpc(
356-
ok_ns: &[u64],
357-
err_ns: &[u64],
358-
grpc_method: &str,
359-
customize: impl FnOnce(&mut pb::ClientGroupedStats),
360-
) -> (pb::ClientGroupedStats, OtlpExactGroup) {
361-
let (g, mut e) = group_with_exact(ok_ns, err_ns, customize);
362-
e.grpc_method = grpc_method.into();
363-
(g, e)
364-
}
365-
366352
fn buckets(groups: Vec<(pb::ClientGroupedStats, OtlpExactGroup)>) -> Vec<OtlpStatsBucket> {
367353
let (stats, exact): (Vec<_>, Vec<_>) = groups.into_iter().unzip();
368354
vec![OtlpStatsBucket {
@@ -444,7 +430,7 @@ mod tests {
444430

445431
#[test]
446432
fn data_point_attributes_and_otel_strip() {
447-
let g_pair = group_pair_with_grpc(&[1_000_000_000], &[], "/pkg.Svc/Method", |g| {
433+
let g_pair = group_with_exact(&[1_000_000_000], &[], |g| {
448434
g.http_status_code = 404;
449435
g.http_method = "POST".into();
450436
g.http_endpoint = "/users/:id".into();
@@ -467,7 +453,6 @@ mod tests {
467453
assert_eq!(str_at(a, "http.request.method"), Some("POST"));
468454
assert_eq!(str_at(a, "http.route"), Some("/users/:id"));
469455
assert!(a.iter().any(|kv| kv["key"] == "http.response.status_code"));
470-
assert_eq!(str_at(a, "rpc.method"), Some("/pkg.Svc/Method"));
471456
assert_eq!(str_at(a, "datadog.operation.name"), Some("test.op"));
472457
assert_eq!(str_at(a, "datadog.span.type"), Some("web"));
473458
assert_eq!(str_at(a, "datadog.origin"), Some("synthetics"));

libdd-trace-stats/src/span_concentrator/aggregation.rs

Lines changed: 3 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ const GRPC_STATUS_CODE_FIELD: &[&str] = &[
2424
"rpc.grpc.status.code",
2525
"grpc.status.code",
2626
];
27-
const GRPC_METHOD_FIELD: &[&str] = &["grpc.method.name", "rpc.method"];
2827

2928
/// Aggregation key fields shared across all concentrator implementations — everything
3029
/// **except** peer tags.
@@ -153,17 +152,6 @@ fn get_grpc_status_code<'a>(span: &'a impl StatSpan<'a>) -> Option<u8> {
153152
None
154153
}
155154

156-
pub(super) fn get_grpc_method<'a>(span: &'a impl StatSpan<'a>) -> &'a str {
157-
for key in GRPC_METHOD_FIELD {
158-
if let Some(val) = span.get_meta(key) {
159-
if !val.is_empty() {
160-
return val;
161-
}
162-
}
163-
}
164-
""
165-
}
166-
167155
fn grpc_status_str_to_int_value(v: &str) -> Option<u8> {
168156
if let Ok(status) = v.parse() {
169157
return Some(status);
@@ -262,7 +250,6 @@ impl<'a> BorrowedAggregationKey<'a> {
262250
};
263251

264252
let grpc_status_code = get_grpc_status_code(span);
265-
266253
let service_source = span.get_meta(TAG_SVC_SRC).unwrap_or_default();
267254

268255
Self {
@@ -344,9 +331,6 @@ pub(super) struct GroupedStats {
344331
error_duration: u64,
345332
error_min: u64,
346333
error_max: u64,
347-
// gRPC method for OTLP export only; not part of the aggregation key so agent stats are
348-
// unaffected.
349-
pub(super) grpc_method: String,
350334
}
351335

352336
impl GroupedStats {
@@ -390,14 +374,11 @@ pub struct OtlpExactCell {
390374
}
391375

392376
/// Exact OK/ERROR cells for one aggregation group, in the same order as the `stats` vector
393-
/// of the accompanying [`pb::ClientStatsBucket`]. `grpc_method` is the group's gRPC method (DD
394-
/// schema `grpc.method.name`) carried out-of-band so it does not appear in the agent stats
395-
/// protobuf wire format.
377+
/// of the accompanying [`pb::ClientStatsBucket`].
396378
#[derive(Debug, Clone, Default)]
397379
pub struct OtlpExactGroup {
398380
pub ok: OtlpExactCell,
399381
pub error: OtlpExactCell,
400-
pub grpc_method: String,
401382
}
402383

403384
/// A bucket flushed for the OTLP trace-metrics path. `exact[i]` is the exact-scalar sidecar
@@ -433,14 +414,10 @@ impl StatsBucket {
433414
duration: i64,
434415
is_error: bool,
435416
is_top_level: bool,
436-
grpc_method: &str,
437417
) {
438418
self.data
439419
.entry_ref(&key)
440-
.or_insert_with(|| GroupedStats {
441-
grpc_method: grpc_method.to_owned(),
442-
..Default::default()
443-
})
420+
.or_default()
444421
.insert(duration, is_error, is_top_level);
445422
}
446423

@@ -455,8 +432,7 @@ impl StatsBucket {
455432
pub(super) fn flush_with_otlp_exact(self, bucket_duration: u64) -> OtlpStatsBucket {
456433
let mut stats = Vec::with_capacity(self.data.len());
457434
let mut exact = Vec::with_capacity(self.data.len());
458-
for (k, mut g) in self.data {
459-
let grpc_method = std::mem::take(&mut g.grpc_method);
435+
for (k, g) in self.data {
460436
exact.push(OtlpExactGroup {
461437
ok: OtlpExactCell {
462438
count: g.hits.saturating_sub(g.errors),
@@ -470,7 +446,6 @@ impl StatsBucket {
470446
min_ns: g.error_min,
471447
max_ns: g.error_max,
472448
},
473-
grpc_method,
474449
});
475450
stats.push(encode_grouped_stats(k, g));
476451
}
@@ -834,18 +809,6 @@ mod tests {
834809
}
835810
.into_key(),
836811
),
837-
// grpc.method.name is carried in GroupedStats (for OTLP), not in the aggregation key.
838-
(
839-
SpanBytes {
840-
meta: vec![("grpc.method.name".into(), "/pkg.Svc/Method".into())].into(),
841-
..Default::default()
842-
},
843-
FixedAggregationKey {
844-
is_trace_root: true,
845-
..Default::default()
846-
}
847-
.into_key(),
848-
),
849812
// Span with grpc status from meta as numeric string
850813
(
851814
SpanBytes {

libdd-trace-stats/src/span_concentrator/mod.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use libdd_trace_protobuf::pb;
99
use aggregation::StatsBucket;
1010

1111
mod aggregation;
12-
use aggregation::{get_grpc_method, BorrowedAggregationKey};
12+
use aggregation::BorrowedAggregationKey;
1313
pub use aggregation::{FixedAggregationKey, OtlpExactCell, OtlpExactGroup, OtlpStatsBucket};
1414

1515
pub mod stat_span;
@@ -184,7 +184,6 @@ impl SpanConcentrator {
184184
span.duration(),
185185
span.is_error(),
186186
span.has_top_level(),
187-
get_grpc_method(span),
188187
);
189188
}
190189

0 commit comments

Comments
 (0)