Add experimental metrics implementation#618
Conversation
Codecov Report
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
Outdated
| count: u64, | ||
| } | ||
| enum BucketValue { | ||
| Counter(f64), |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
This allows the user to count / accumulate things that are not whole numbers.
sentry-core/src/metrics.rs
Outdated
| let mut value = mk_value(ty, values.next()?)?; | ||
|
|
||
| for value_s in values { | ||
| value.merge(mk_value(ty, value_s)?).ok()?; |
There was a problem hiding this comment.
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.
* 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)
52c9dd0 to
45a4ac1
Compare
fd9783f to
d24c9b2
Compare
|
I've updated the implementation to address above comments. The current state is still missing a few things, however:
|
cdd36e4 to
ba4afe0
Compare
loewenheim
left a comment
There was a problem hiding this comment.
Some nits, but otherwise looks good
| fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
| Ok(match s { | ||
| "nanosecond" | "ns" => Self::Duration(DurationUnit::NanoSecond), | ||
| "microsecond" => Self::Duration(DurationUnit::MicroSecond), |
There was a problem hiding this comment.
Could do μs here, but I don't know if that's common.
There was a problem hiding this comment.
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.
This adds:
statsdEnvelopeItemMetricAggregatorwhich is implemented similarly to the Python prototype implementationMetrictype that allows building and sending metrics, which is the primary user-facing APIcadence-compatibleSentryMetricSinkthat makes it easier to integrate Sentry into existing environmentsrelay-metricsThis PR does not yet include support for code locations and span linking; both will follow at a later stage.