Skip to content

Commit 4cb3001

Browse files
rayztobz
authored andcommitted
fix(metrics): encode units in V3 series payloads (#1761)
## Summary <!-- Please provide a brief summary about what this PR does. This should help the reviewers give feedback faster and with higher quality. --> This pr adds metric units encoding into V3 series payloads to match the Datadog Agent V3 serializer. This change keeps sketch behavior unchanged: V3 sketches do not encode units, matching the Agent sketch path. ## Change Type - [x] Bug fix - [ ] New feature - [ ] Non-functional (chore, refactoring, docs) - [ ] Performance ## How did you test this PR? <!-- Please how you tested these changes here --> unit tests / ci ## References <!-- Please list any issues closed by this PR. --> <!-- - Closes: <issue link> --> <!-- Any other issues or PRs relevant to this PR? Feel free to list them here. -->
1 parent 0fc6858 commit 4cb3001

3 files changed

Lines changed: 152 additions & 3 deletions

File tree

lib/saluki-components/src/encoders/datadog/metrics/mod.rs

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -979,6 +979,12 @@ fn write_metric_to_v3(writer: &mut v3::V3Writer, metric: &Metric, additional_tag
979979
}
980980
}
981981

982+
if metric_type != v3::V3MetricType::Sketch {
983+
if let Some(unit) = metric.metadata().unit() {
984+
builder.set_unit(unit);
985+
}
986+
}
987+
982988
// Points based on metric type
983989
match metric.values() {
984990
MetricValues::Counter(points) | MetricValues::Gauge(points) => {
@@ -1102,7 +1108,12 @@ async fn create_v3_request(
11021108

11031109
#[cfg(test)]
11041110
mod tests {
1105-
use saluki_core::data_model::{event::Event, payload::Payload};
1111+
use saluki_context::Context;
1112+
use saluki_core::data_model::{
1113+
event::{metric::MetricMetadata, Event},
1114+
payload::Payload,
1115+
};
1116+
use stringtheory::MetaString;
11061117
use tokio::time::timeout;
11071118

11081119
use super::*;
@@ -1153,6 +1164,65 @@ serializer_experimental_use_v3_api:
11531164
assert_eq!("/api/intake/metrics/custom/series", request.uri());
11541165
}
11551166

1167+
#[test]
1168+
fn v3_series_metric_unit_refs_are_encoded_sparsely() {
1169+
let context = Context::from_static_parts("my.timer.avg", &[]);
1170+
let metadata = MetricMetadata::default().with_unit(MetaString::from_static("millisecond"));
1171+
let gauge = Metric::from_parts(context, MetricValues::gauge([1.0_f64]), metadata);
1172+
let context = Context::from_static_parts("my.counter", &[]);
1173+
let no_unit = Metric::from_parts(context, MetricValues::gauge([2.0_f64]), MetricMetadata::default());
1174+
let context = Context::from_static_parts("my.timer.max", &[]);
1175+
let metadata = MetricMetadata::default().with_unit(MetaString::from_static("millisecond"));
1176+
let same_unit = Metric::from_parts(context, MetricValues::gauge([3.0_f64]), metadata);
1177+
1178+
let payload = encode_v3_metrics_batch(&[gauge, no_unit, same_unit], &SharedTagSet::default())
1179+
.expect("V3 metric should encode successfully");
1180+
1181+
let expected_unit_dict = [
1182+
0xca, 0x01, // field 25, length-delimited.
1183+
0x0c, // field payload length: varint string length + string bytes.
1184+
0x0b, b'm', b'i', b'l', b'l', b'i', b's', b'e', b'c', b'o', b'n', b'd',
1185+
];
1186+
assert!(
1187+
payload
1188+
.windows(expected_unit_dict.len())
1189+
.any(|window| window == expected_unit_dict),
1190+
"V3 payload should contain DictUnitStr field for 'millisecond', got bytes: {:?}",
1191+
payload
1192+
);
1193+
1194+
let expected_unit_ref = [
1195+
0xd2, 0x01, // field 26, length-delimited.
1196+
0x02, // packed field payload length.
1197+
0x02, 0x00, // sparse unit refs for metrics 1 and 3 only: refs [1, 1] -> deltas [1, 0].
1198+
];
1199+
assert!(
1200+
payload
1201+
.windows(expected_unit_ref.len())
1202+
.any(|window| window == expected_unit_ref),
1203+
"V3 payload should contain UnitRef field for 'millisecond', got bytes: {:?}",
1204+
payload
1205+
);
1206+
}
1207+
1208+
#[test]
1209+
fn v3_sketch_metric_unit_not_encoded() {
1210+
let context = Context::from_static_parts("my.histogram", &[]);
1211+
let metadata = MetricMetadata::default().with_unit(MetaString::from_static("millisecond"));
1212+
let histogram = Metric::from_parts(context, MetricValues::histogram([1.0_f64]), metadata);
1213+
1214+
let payload = encode_v3_metrics_batch(&[histogram], &SharedTagSet::default())
1215+
.expect("V3 sketch metric should encode successfully");
1216+
1217+
assert!(
1218+
!payload
1219+
.windows(b"millisecond".len())
1220+
.any(|window| window == b"millisecond"),
1221+
"V3 sketch payload should not contain unit bytes, matching the Agent V3 sketch builder: {:?}",
1222+
payload
1223+
);
1224+
}
1225+
11561226
#[tokio::test]
11571227
async fn validation_split_flush_assigns_batch_id_to_carried_metric() {
11581228
let v2_endpoint_config = EndpointConfiguration::new(CompressionScheme::noop(), 1, None);

lib/saluki-components/src/encoders/datadog/metrics/v3/constants.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,5 @@ pub const SKETCH_BIN_KEYS_FIELD_NUMBER: u32 = 21;
2525
pub const SKETCH_BIN_CNTS_FIELD_NUMBER: u32 = 22;
2626
pub const SOURCE_TYPE_NAME_FIELD_NUMBER: u32 = 23;
2727
pub const ORIGIN_INFO_FIELD_NUMBER: u32 = 24;
28+
pub const DICT_UNIT_STR_FIELD_NUMBER: u32 = 25;
29+
pub const UNIT_REFS_FIELD_NUMBER: u32 = 26;

lib/saluki-components/src/encoders/datadog/metrics/v3/writer.rs

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use super::interner::Interner;
1111
use super::types::{value_type_for_values, V3MetricType, V3ValueType};
1212

1313
const FLAG_NO_INDEX: u64 = 0x100;
14+
const FLAG_HAS_UNIT: u64 = 0x200;
1415

1516
/// Encoded V3 payload data ready for protobuf serialization.
1617
///
@@ -27,8 +28,9 @@ struct V3EncodedData {
2728
pub dict_resource_name: Vec<i64>,
2829
pub dict_source_type_bytes: Vec<u8>,
2930
pub dict_origin_info: Vec<i32>,
31+
pub dict_unit_bytes: Vec<u8>,
3032

31-
// Per-metric columns (one entry per metric)
33+
// Per-metric columns (one entry per metric, except conditional columns)
3234
pub types: Vec<u64>,
3335
pub names: Vec<i64>,
3436
pub tags: Vec<i64>,
@@ -37,6 +39,7 @@ struct V3EncodedData {
3739
pub num_points: Vec<u64>,
3840
pub source_type_names: Vec<i64>,
3941
pub origin_infos: Vec<i64>,
42+
pub unit_refs: Vec<i64>, // Present only for metrics with FLAG_HAS_UNIT set.
4043

4144
// Point data (varies per metric based on num_points)
4245
pub timestamps: Vec<i64>,
@@ -65,6 +68,7 @@ pub struct V3Writer {
6568
resource_interner: Interner<Vec<(i64, i64)>>,
6669
source_type_interner: Interner<String>,
6770
origin_interner: Interner<(i32, i32, i32)>,
71+
unit_interner: Interner<String>,
6872

6973
// Dictionary encoded bytes
7074
dict_name_bytes: Vec<u8>,
@@ -76,8 +80,9 @@ pub struct V3Writer {
7680
dict_resource_name: Vec<i64>,
7781
dict_source_type_bytes: Vec<u8>,
7882
dict_origin_info: Vec<i32>,
83+
dict_unit_bytes: Vec<u8>,
7984

80-
// Per-metric columns
85+
// Per-metric columns (one entry per metric, except conditional columns)
8186
types: Vec<u64>,
8287
names: Vec<i64>,
8388
tags: Vec<i64>,
@@ -86,6 +91,7 @@ pub struct V3Writer {
8691
num_points: Vec<u64>,
8792
source_type_names: Vec<i64>,
8893
origin_infos: Vec<i64>,
94+
unit_refs: Vec<i64>, // Present only for metrics with FLAG_HAS_UNIT set.
8995

9096
// Point data
9197
timestamps: Vec<i64>,
@@ -134,6 +140,7 @@ impl V3Writer {
134140
point_start_idx,
135141
sint64_start_idx,
136142
metric_idx,
143+
unit_ref_idx: None,
137144
}
138145
}
139146

@@ -144,6 +151,7 @@ impl V3Writer {
144151
delta_encode(&mut self.resources);
145152
delta_encode(&mut self.source_type_names);
146153
delta_encode(&mut self.origin_infos);
154+
delta_encode(&mut self.unit_refs);
147155
delta_encode(&mut self.timestamps);
148156

149157
V3EncodedData {
@@ -156,6 +164,7 @@ impl V3Writer {
156164
dict_resource_name: self.dict_resource_name,
157165
dict_source_type_bytes: self.dict_source_type_bytes,
158166
dict_origin_info: self.dict_origin_info,
167+
dict_unit_bytes: self.dict_unit_bytes,
159168
types: self.types,
160169
names: self.names,
161170
tags: self.tags,
@@ -164,6 +173,7 @@ impl V3Writer {
164173
num_points: self.num_points,
165174
source_type_names: self.source_type_names,
166175
origin_infos: self.origin_infos,
176+
unit_refs: self.unit_refs,
167177
timestamps: self.timestamps,
168178
vals_sint64: self.vals_sint64,
169179
vals_float32: self.vals_float32,
@@ -207,6 +217,9 @@ impl V3Writer {
207217
}
208218

209219
os.write_repeated_packed_int32(DICT_ORIGIN_INFO_FIELD_NUMBER, &data.dict_origin_info)?;
220+
if !data.dict_unit_bytes.is_empty() {
221+
os.write_bytes(DICT_UNIT_STR_FIELD_NUMBER, &data.dict_unit_bytes)?;
222+
}
210223

211224
// Per-metric columns
212225
os.write_repeated_packed_uint64(TYPES_FIELD_NUMBER, &data.types)?;
@@ -217,6 +230,7 @@ impl V3Writer {
217230
os.write_repeated_packed_uint64(NUM_POINTS_FIELD_NUMBER, &data.num_points)?;
218231
os.write_repeated_packed_sint64(SOURCE_TYPE_NAME_FIELD_NUMBER, &data.source_type_names)?;
219232
os.write_repeated_packed_sint64(ORIGIN_INFO_FIELD_NUMBER, &data.origin_infos)?;
233+
os.write_repeated_packed_sint64(UNIT_REFS_FIELD_NUMBER, &data.unit_refs)?;
220234

221235
// Point data
222236
os.write_repeated_packed_sint64(TIMESTAMPS_FIELD_NUMBER, &data.timestamps)?;
@@ -363,6 +377,17 @@ impl V3Writer {
363377
}
364378
id
365379
}
380+
381+
fn intern_unit(&mut self, unit: &str) -> i64 {
382+
if unit.is_empty() {
383+
return 0;
384+
}
385+
let (id, is_new) = self.unit_interner.get_or_insert(unit);
386+
if is_new {
387+
append_len_str(&mut self.dict_unit_bytes, unit);
388+
}
389+
id
390+
}
366391
}
367392

368393
/// Builder for a single metric within a V3 payload.
@@ -374,6 +399,7 @@ pub struct V3MetricBuilder<'a> {
374399
point_start_idx: usize,
375400
sint64_start_idx: usize,
376401
metric_idx: usize,
402+
unit_ref_idx: Option<usize>,
377403
}
378404

379405
impl<'a> V3MetricBuilder<'a> {
@@ -423,6 +449,26 @@ impl<'a> V3MetricBuilder<'a> {
423449
}
424450
}
425451

452+
/// Sets the unit for this metric.
453+
pub fn set_unit(&mut self, unit: &str) {
454+
if unit.is_empty() {
455+
self.writer.types[self.metric_idx] &= !FLAG_HAS_UNIT;
456+
if let Some(unit_ref_idx) = self.unit_ref_idx.take() {
457+
self.writer.unit_refs.remove(unit_ref_idx);
458+
}
459+
return;
460+
}
461+
462+
let id = self.writer.intern_unit(unit);
463+
if let Some(unit_ref_idx) = self.unit_ref_idx {
464+
self.writer.unit_refs[unit_ref_idx] = id;
465+
} else {
466+
self.unit_ref_idx = Some(self.writer.unit_refs.len());
467+
self.writer.unit_refs.push(id);
468+
}
469+
self.writer.types[self.metric_idx] |= FLAG_HAS_UNIT;
470+
}
471+
426472
/// Adds a data point to this metric.
427473
pub fn add_point(&mut self, timestamp: i64, value: f64) {
428474
self.writer.timestamps.push(timestamp);
@@ -608,6 +654,37 @@ mod tests {
608654
assert_eq!(data.timestamps.len(), 2);
609655
}
610656

657+
#[test]
658+
fn test_writer_unit() {
659+
let mut writer = V3Writer::new();
660+
661+
{
662+
let mut metric = writer.write(V3MetricType::Gauge, "has.unit");
663+
metric.set_unit("millisecond");
664+
metric.add_point(1000, 42.0);
665+
metric.close();
666+
}
667+
{
668+
let mut metric = writer.write(V3MetricType::Gauge, "no.unit");
669+
metric.add_point(1000, 43.0);
670+
metric.close();
671+
}
672+
{
673+
let mut metric = writer.write(V3MetricType::Gauge, "same.unit");
674+
metric.set_unit("millisecond");
675+
metric.add_point(1000, 44.0);
676+
metric.close();
677+
}
678+
679+
let data = writer.finalize_inner();
680+
681+
assert_eq!(data.unit_refs, vec![1, 0]);
682+
assert_eq!(data.dict_unit_bytes, b"\x0bmillisecond");
683+
assert_eq!(data.types[0] & FLAG_HAS_UNIT, FLAG_HAS_UNIT);
684+
assert_eq!(data.types[1] & FLAG_HAS_UNIT, 0);
685+
assert_eq!(data.types[2] & FLAG_HAS_UNIT, FLAG_HAS_UNIT);
686+
}
687+
611688
#[test]
612689
fn test_writer_multiple_metrics() {
613690
let mut writer = V3Writer::new();

0 commit comments

Comments
 (0)