Skip to content

Commit 5f36e23

Browse files
authored
feat(cogs): Track categories in COGS (#4496)
Makes it possible to split a COGS measurement into multiple categories. Also instruments the transaction processing with more categories. The initial plan was to instrument normalization in multiple smaller categories, I decided against that in this PR because that requires an API change with huge effects in tests. If we realize normalization takes up a significant amount, we can revisit this. Ref: getsentry/team-ingest#637
1 parent e13fd30 commit 5f36e23

File tree

11 files changed

+455
-105
lines changed

11 files changed

+455
-105
lines changed

Cargo.lock

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

relay-cogs/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,6 @@ autobenches = false
1212

1313
[lints]
1414
workspace = true
15+
16+
[dev-dependencies]
17+
insta = { workspace = true }

relay-cogs/src/cogs.rs

Lines changed: 207 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
use core::fmt;
1+
use crate::time::Instant;
22
use std::collections::BTreeMap;
3+
use std::fmt;
34
use std::num::NonZeroUsize;
45
use std::sync::Arc;
5-
use std::time::Instant;
66

7-
use crate::{AppFeature, ResourceId};
7+
use crate::{AppFeature, Measurements, ResourceId};
88
use crate::{CogsMeasurement, CogsRecorder, Value};
99

1010
/// COGS measurements collector.
@@ -64,8 +64,8 @@ impl Cogs {
6464
Token {
6565
resource,
6666
features: weights.into(),
67-
start: Instant::now(),
68-
recorder: Arc::clone(&self.recorder),
67+
measurements: Measurements::start(),
68+
recorder: Some(Arc::clone(&self.recorder)),
6969
}
7070
}
7171
}
@@ -77,17 +77,43 @@ impl Cogs {
7777
pub struct Token {
7878
resource: ResourceId,
7979
features: FeatureWeights,
80-
start: Instant,
81-
recorder: Arc<dyn CogsRecorder>,
80+
measurements: Measurements,
81+
recorder: Option<Arc<dyn CogsRecorder>>,
8282
}
8383

8484
impl Token {
85+
/// Creates a new no-op token, which records nothing.
86+
///
87+
/// This is primarily useful for testing.
88+
pub fn noop() -> Self {
89+
Self {
90+
resource: ResourceId::Relay,
91+
features: FeatureWeights::none(),
92+
measurements: Measurements::start(),
93+
recorder: None,
94+
}
95+
}
96+
8597
/// Cancels the COGS measurement.
8698
pub fn cancel(&mut self) {
8799
// No features -> nothing gets attributed.
88100
self.update(FeatureWeights::none());
89101
}
90102

103+
/// Starts a categorized measurement.
104+
///
105+
/// The measurement is finalized when the returned [`CategoryToken`] is dropped.
106+
///
107+
/// Instead of manually starting a categorized measurement, the [`crate::with`]
108+
/// macro can be used.
109+
pub fn start_category(&mut self, category: impl Category) -> CategoryToken<'_> {
110+
CategoryToken {
111+
parent: self,
112+
start: Instant::now(),
113+
category: category.name(),
114+
}
115+
}
116+
91117
/// Updates the app features to which the active measurement is attributed to.
92118
///
93119
/// # Example:
@@ -115,15 +141,22 @@ impl Token {
115141

116142
impl Drop for Token {
117143
fn drop(&mut self) {
118-
let elapsed = self.start.elapsed();
119-
120-
for (feature, ratio) in self.features.weights() {
121-
let time = elapsed.mul_f32(ratio);
122-
self.recorder.record(CogsMeasurement {
123-
resource: self.resource,
124-
feature,
125-
value: Value::Time(time),
126-
});
144+
let Some(recorder) = self.recorder.as_mut() else {
145+
return;
146+
};
147+
148+
let measurements = self.measurements.finish();
149+
150+
for measurement in measurements {
151+
for (feature, ratio) in self.features.weights() {
152+
let time = measurement.duration.mul_f32(ratio);
153+
recorder.record(CogsMeasurement {
154+
resource: self.resource,
155+
feature,
156+
category: measurement.category,
157+
value: Value::Time(time),
158+
});
159+
}
127160
}
128161
}
129162
}
@@ -137,6 +170,46 @@ impl fmt::Debug for Token {
137170
}
138171
}
139172

173+
/// A COGS category.
174+
pub trait Category {
175+
/// String representation of the category.
176+
fn name(&self) -> &'static str;
177+
}
178+
179+
impl Category for &'static str {
180+
fn name(&self) -> &'static str {
181+
self
182+
}
183+
}
184+
185+
/// A categorized COGS measurement.
186+
///
187+
/// Must be started with [`Token::start_category`].
188+
#[must_use]
189+
pub struct CategoryToken<'a> {
190+
parent: &'a mut Token,
191+
start: Instant,
192+
category: &'static str,
193+
}
194+
195+
impl Drop for CategoryToken<'_> {
196+
fn drop(&mut self) {
197+
self.parent
198+
.measurements
199+
.add(self.start.elapsed(), self.category);
200+
}
201+
}
202+
203+
impl fmt::Debug for CategoryToken<'_> {
204+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
205+
f.debug_struct("CategoryToken")
206+
.field("resource", &self.parent.resource)
207+
.field("features", &self.parent.features)
208+
.field("category", &self.category)
209+
.finish()
210+
}
211+
}
212+
140213
/// A collection of weighted [app features](AppFeature).
141214
///
142215
/// Used to attribute a single COGS measurement to multiple features.
@@ -278,7 +351,7 @@ impl FeatureWeightsBuilder {
278351

279352
#[cfg(test)]
280353
mod tests {
281-
use std::{collections::HashMap, time::Duration};
354+
use std::collections::HashMap;
282355

283356
use super::*;
284357
use crate::test::TestRecorder;
@@ -291,17 +364,25 @@ mod tests {
291364
drop(cogs.timed(ResourceId::Relay, AppFeature::Spans));
292365

293366
let measurements = recorder.measurements();
294-
assert_eq!(measurements.len(), 1);
295-
assert_eq!(measurements[0].resource, ResourceId::Relay);
296-
assert_eq!(measurements[0].feature, AppFeature::Spans);
367+
insta::assert_debug_snapshot!(measurements, @r###"
368+
[
369+
CogsMeasurement {
370+
resource: Relay,
371+
feature: Spans,
372+
category: None,
373+
value: Time(
374+
100ns,
375+
),
376+
},
377+
]
378+
"###);
297379
}
298380

299381
#[test]
300382
fn test_cogs_multiple_weights() {
301383
let recorder = TestRecorder::default();
302384
let cogs = Cogs::new(recorder.clone());
303385

304-
let start = Instant::now();
305386
let f = FeatureWeights::builder()
306387
.weight(AppFeature::Spans, 1)
307388
.weight(AppFeature::Transactions, 1)
@@ -311,20 +392,114 @@ mod tests {
311392
.build();
312393
{
313394
let _token = cogs.timed(ResourceId::Relay, f);
314-
std::thread::sleep(Duration::from_millis(50));
395+
crate::time::advance_millis(50);
396+
}
397+
398+
let measurements = recorder.measurements();
399+
insta::assert_debug_snapshot!(measurements, @r###"
400+
[
401+
CogsMeasurement {
402+
resource: Relay,
403+
feature: Spans,
404+
category: None,
405+
value: Time(
406+
25ms,
407+
),
408+
},
409+
CogsMeasurement {
410+
resource: Relay,
411+
feature: MetricsSpans,
412+
category: None,
413+
value: Time(
414+
25ms,
415+
),
416+
},
417+
]
418+
"###);
419+
}
420+
421+
#[test]
422+
fn test_cogs_categorized() {
423+
let recorder = TestRecorder::default();
424+
let cogs = Cogs::new(recorder.clone());
425+
426+
let features = FeatureWeights::builder()
427+
.weight(AppFeature::Spans, 1)
428+
.weight(AppFeature::Errors, 1)
429+
.build();
430+
431+
{
432+
let mut token = cogs.timed(ResourceId::Relay, features);
433+
crate::time::advance_millis(10);
434+
crate::with!(token, "s1", {
435+
crate::time::advance_millis(6);
436+
});
437+
crate::time::advance_millis(20);
438+
let _category = token.start_category("s2");
439+
crate::time::advance_millis(12);
315440
}
316-
let elapsed = start.elapsed();
317441

318442
let measurements = recorder.measurements();
319-
assert_eq!(measurements.len(), 2);
320-
assert_eq!(measurements[0].resource, ResourceId::Relay);
321-
assert_eq!(measurements[0].feature, AppFeature::Spans);
322-
assert_eq!(measurements[1].resource, ResourceId::Relay);
323-
assert_eq!(measurements[1].feature, AppFeature::MetricsSpans);
324-
assert_eq!(measurements[0].value, measurements[1].value);
325-
let Value::Time(time) = measurements[0].value;
326-
assert!(time >= Duration::from_millis(25), "{time:?}");
327-
assert!(time <= elapsed.div_f32(1.99), "{time:?}");
443+
insta::assert_debug_snapshot!(measurements, @r###"
444+
[
445+
CogsMeasurement {
446+
resource: Relay,
447+
feature: Errors,
448+
category: None,
449+
value: Time(
450+
15ms,
451+
),
452+
},
453+
CogsMeasurement {
454+
resource: Relay,
455+
feature: Spans,
456+
category: None,
457+
value: Time(
458+
15ms,
459+
),
460+
},
461+
CogsMeasurement {
462+
resource: Relay,
463+
feature: Errors,
464+
category: Some(
465+
"s1",
466+
),
467+
value: Time(
468+
3ms,
469+
),
470+
},
471+
CogsMeasurement {
472+
resource: Relay,
473+
feature: Spans,
474+
category: Some(
475+
"s1",
476+
),
477+
value: Time(
478+
3ms,
479+
),
480+
},
481+
CogsMeasurement {
482+
resource: Relay,
483+
feature: Errors,
484+
category: Some(
485+
"s2",
486+
),
487+
value: Time(
488+
6ms,
489+
),
490+
},
491+
CogsMeasurement {
492+
resource: Relay,
493+
feature: Spans,
494+
category: Some(
495+
"s2",
496+
),
497+
value: Time(
498+
6ms,
499+
),
500+
},
501+
]
502+
"###);
328503
}
329504

330505
#[test]

0 commit comments

Comments
 (0)