getsentry / sentry-rust

Official Sentry SDK for Rust
https://sentry.io/
Apache License 2.0
620 stars 153 forks source link

Metric::parse_statsd does not roundtrip with gauges #639

Closed untitaker closed 8 months ago

untitaker commented 9 months ago

This is a problem because I would like to use the SDK to send metrics from within existing metrics abstractions, which may receive dogstatsd packed value format. Ideally parse_statsd would either support the full output format, or store the metric as a raw string internally so it can be forwarded as-is to sentry (where it parses fine)

Testcase:

    #[test]
    fn test_regression_parse_statsd() {
        let payload = "docker.net.bytes_rcvd:27763.20237096717:27763.20237096717:27763.20237096717:27763.20237096717:1|g|#container_id:97df61f5c55b58ec9c04da3e03edc8a875ec90eb405eb5645ad9a86d0a7cd3ee,container_name:app_sidekiq_1,docker_image:ghcr.io/mastodon/mastodon:nightly.2024-02-02-security,docker_network:app_internal_network,git.commit.sha:1726085db5cd73dd30953da858f9887bcc90b958,git.repository_url:https://github.com/mastodon/mastodon,host:aaaa,image_name:ghcr.io/mastodon/mastodon,image_tag:nightly.2024-02-02-security,runtime:docker,short_image:mastodon,source_type_name:System,environment:production|T1707081890";
        let metric = Metric::parse_statsd(payload).unwrap();
        assert_eq!(metric.name, "docker.net.bytes_rcvd");
    }

The statsd parser cannot actually deal with multiple values. I don't know if there are issues with other metric types, this is just the first one I found.

SDK version 0.32.2

loewenheim commented 9 months ago

This would be the ~metric~ value packing described under https://docs.datadoghq.com/developers/dogstatsd/datagram_shell/?tab=metrics#dogstatsd-protocol-v11?

untitaker commented 9 months ago

This would be the metric value packing described under docs.datadoghq.com/developers/dogstatsd/datagram_shell/?tab=metrics#dogstatsd-protocol-v11?

for example, but the above payload is generated by the SDK to begin with. GaugeSummary can hold multiple values

untitaker commented 9 months ago

https://github.com/getsentry/sentry-rust/blob/ea6375d14f4a81bf30afc020b8ce58881f32e15a/sentry-core/src/metrics.rs#L796-L800

loewenheim commented 9 months ago

Ah, I see. Thanks for the heads up.

mitsuhiko commented 9 months ago

It sounds like the SDK has not updated to the updated spec where gauges support a quintuple.

loewenheim commented 9 months ago

Where is that spec, for reference?

mitsuhiko commented 9 months ago

https://getsentry.github.io/relay/relay_metrics/enum.BucketValue.html#variant.Gauge

loewenheim commented 9 months ago

The gauge part, at least, should be fixed by https://github.com/getsentry/sentry-rust/pull/640.