Add experimental metrics implementation #618

Swatinem · 2023-10-17T15:33:33Z

This adds:

A new statsd EnvelopeItem
A new MetricAggregator which is implemented similarly to the Python prototype implementation
The Client instantiates a metric flusher and starts it in the background
A Metric type that allows building and sending metrics, which is the primary user-facing API
A cadence-compatible SentryMetricSink that makes it easier to integrate Sentry into existing environments
Internal types that are mostly adapted from relay-metrics

This PR does not yet include support for code locations and span linking; both will follow at a later stage.

codecov · 2023-10-17T15:41:48Z

Codecov Report

Merging #618 (69080ab) into master (dfdb7b6) will increase coverage by 0.31%.
The diff coverage is 75.72%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #618      +/-   ##
==========================================
+ Coverage   72.75%   73.07%   +0.31%     
==========================================
  Files          59       62       +3     
  Lines        6706     7494     +788     
==========================================
+ Hits         4879     5476     +597     
- Misses       1827     2018     +191

sentry-core/src/metrics.rs

loewenheim · 2023-10-19T10:50:11Z

sentry-core/src/metrics.rs

+    count: u64,
+}
+enum BucketValue {
+    Counter(f64),


Why are counters f64?

That is what relay-metrics uses: https://getsentry.github.io/relay/relay_metrics/type.CounterType.html
So I just stuck with that. Admittedly it does not make as much sense.

This allows the user to count / accumulate things that are not whole numbers.

sentry-core/src/metrics.rs

sentry-core/src/client.rs

sentry-core/src/metrics.rs

jan-auer · 2023-12-07T09:59:54Z

sentry-core/src/metrics.rs

+            let mut value = mk_value(ty, values.next()?)?;
+
+            for value_s in values {
+                value.merge(mk_value(ty, value_s)?).ok()?;


I notice that this parser is written differently than the one in Relay -- have you performed benchmarks on this? I'm asking because we might want to update the implementation in Relay in that case, too.

sentry-core/src/metrics.rs

sentry-core/src/client.rs

* master: tracing: send spans' attributes along with the event ones (#629) release: 0.32.0 Update crate dependencies (#628) feat: bump http to 1.0.0 (#627) release: 0.31.8 MonitorSchedule constructor that validates crontab syntax (#625) fix(docs): Fix some doc errors that slipped in (#623) docs(tower): Mention how to enable http feature from sentry crate (#622) build(deps): bump rustix from 0.37.23 to 0.37.25 (#619)

jan-auer · 2023-12-11T17:24:12Z

I've updated the implementation to address above comments. The current state is still missing a few things, however:

There's no shift while flushing. Edit: I believe we might not need such a shift. The flush cycle begins immediately when the client is started, so it is shifted naturally.
The cadence sink test is flakey
There are no tests of the metrics module
Default tags (release and environment) are still missing
The public API needs a review, we should probably add top-level utility functions
flush does not work properly, as it doesn't wait

loewenheim

Some nits, but otherwise looks good

sentry-core/src/cadence.rs

sentry-core/src/client.rs

sentry-core/src/metrics.rs

loewenheim · 2023-12-12T13:30:50Z

sentry-core/src/units.rs

+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Ok(match s {
+            "nanosecond" | "ns" => Self::Duration(DurationUnit::NanoSecond),
+            "microsecond" => Self::Duration(DurationUnit::MicroSecond),


Could do μs here, but I don't know if that's common.

Good point, μ is an ASCII character so we could add this as an alias. We'd have to add this to the spec and Relay too, though.

Add experimental metrics implementation

9f33090

Swatinem added 2 commits October 18, 2023 14:13

finish up impl

c83b9aa

implement sets the naive way

dd22afb

Swatinem self-assigned this Oct 18, 2023

do not emit an empty envelope item on shutdown

2fc55ae

Swatinem marked this pull request as ready for review October 18, 2023 13:47

loewenheim reviewed Oct 19, 2023

View reviewed changes

review

97c9a0c

mitsuhiko mentioned this pull request Oct 20, 2023

feat(metrics): Preliminary work for metrics aggregation #621

Closed

Swatinem added 6 commits October 23, 2023 10:04

add rate limit category

b6bdece

fix typo

331a457

fix build

a61de48

try to fix flush race on windows

06c483a

fix doc builds

c692048

remove forced shutdown from metrics aggregator

b568ebc

ashwoods requested a review from jan-auer October 30, 2023 10:44

bruno-garcia mentioned this pull request Dec 5, 2023

Add Metrics API getsentry/sentry-dotnet#2880

Closed

jan-auer reviewed Dec 7, 2023

View reviewed changes

jan-auer assigned jan-auer and unassigned Swatinem Dec 11, 2023

jan-auer added 4 commits December 11, 2023 11:36

ref(metrics): Minor review comments

dc5ea74

ref: Separate cadence from metrics feature

f2c6410

ref(metrics): Minor code-level changes

45a4ac1

jan-auer force-pushed the swatinem/metrics branch from 52c9dd0 to 45a4ac1 Compare December 11, 2023 12:27

ref(metrics): Refactor to match Python SDK

d24c9b2

jan-auer force-pushed the swatinem/metrics branch from fd9783f to d24c9b2 Compare December 11, 2023 17:18

fix: First bugfixes

b515db3

jan-auer added 3 commits December 11, 2023 19:09

fix: Flakey submission

1267668

feat(metrics): Add a convenience API to track metrics directly

c164983

fix: Move statsd parsing to metrics module

ba4afe0

jan-auer force-pushed the swatinem/metrics branch from cdd36e4 to ba4afe0 Compare December 11, 2023 18:30

jan-auer added 11 commits December 11, 2023 20:25

ref(metrics): Reorganize and add docs

bcc665b

test: Add a first unit test

32e6baa

feat(metrics): Inject default tags

0ef19a3

ref(metrics): Simplify unit handling

12bf258

feat(metrics): Further improvements to docs and cadence

d90d14c

fix(metrics): Sanitation behavior

524a5a9

fix: Docs

c506251

ref(metrics): Refactor worker into separate struct

c3f7247

ref(metrics): Add more Debug impls

edbbb95

ref: Rename metric item to "statsd"

55b7f28

ref(metrics): Flush synchronously

ab14afa

loewenheim approved these changes Dec 12, 2023

View reviewed changes

jan-auer added 3 commits December 12, 2023 14:45

ref: Address review feedback

0c4064c

ref(metrics): Reduce duplication of bucket key

139cdf3

meta: Changelog

69080ab

jan-auer merged commit a2b60da into master Dec 12, 2023
13 checks passed

jan-auer deleted the swatinem/metrics branch December 12, 2023 15:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add experimental metrics implementation #618

Add experimental metrics implementation #618

Swatinem commented Oct 17, 2023 •

edited by jan-auer

Loading

codecov bot commented Oct 17, 2023 •

edited

Loading

loewenheim Oct 19, 2023

Swatinem Oct 20, 2023

jan-auer Dec 11, 2023

jan-auer Dec 7, 2023

jan-auer commented Dec 11, 2023 •

edited

Loading

loewenheim left a comment

loewenheim Dec 12, 2023

jan-auer Dec 12, 2023

Add experimental metrics implementation #618

Add experimental metrics implementation #618

Conversation

Swatinem commented Oct 17, 2023 • edited by jan-auer Loading

codecov bot commented Oct 17, 2023 • edited Loading

Codecov Report

loewenheim Oct 19, 2023

Choose a reason for hiding this comment

Swatinem Oct 20, 2023

Choose a reason for hiding this comment

jan-auer Dec 11, 2023

Choose a reason for hiding this comment

jan-auer Dec 7, 2023

Choose a reason for hiding this comment

jan-auer commented Dec 11, 2023 • edited Loading

loewenheim left a comment

Choose a reason for hiding this comment

loewenheim Dec 12, 2023

Choose a reason for hiding this comment

jan-auer Dec 12, 2023

Choose a reason for hiding this comment

Swatinem commented Oct 17, 2023 •

edited by jan-auer

Loading

codecov bot commented Oct 17, 2023 •

edited

Loading

jan-auer commented Dec 11, 2023 •

edited

Loading