docs icon indicating copy to clipboard operation
docs copied to clipboard

Add Sentry Exporter to exporters.md

Open italux opened this issue 4 years ago • 3 comments

Description

Add Sentry Exporter into exporters list

italux avatar Feb 20 '21 00:02 italux

@RichiH @brian-brazil could you please check this out

italux avatar Feb 20 '21 01:02 italux

@juliusv @RichiH Could you please review this doc update?

italux avatar Oct 30 '22 01:10 italux

@italux Hi, sorry for the radio silence!

We usually just check whether added exporters follow best practices such as https://prometheus.io/docs/practices/naming/#metric-and-label-naming and a few more general best practices.

Would it be possible to get some of the metrics in the exporter to follow the conventions a bit better?

  • sentry_rate_limit_events_sec: Clarify and spell out unit: sentry_rate_limit_events_per_second
  • sentry_issues: As far as I can tell by looking at the code, this is using a histogram metric type under the hood, but it's not used like a valid Prometheus histogram, as it manually sets the le labels to values that cannot be parsed as a float (e.g. 24h instead of just the number of seconds corresponding to that). I would recommend doing either of:
    • Switching it to a normal gauge metric family instead and not use the special le histogram label for non-numeric values.
    • Or you could also keep it a histogram and use the le label as intended, but then you'd also have to put the time unit (seconds) into the metric name somewhere (so the user knows that the duration in the le label is in seconds). It also seems from the code like there's _gcount and _gsum time series that I guess should just be called _sum and _count?

Overall it would be very helpful to see a sample metrics output (either in the exporter docs or in the PR) to get an easier idea of what the metrics actually look like.

juliusv avatar Oct 30 '22 09:10 juliusv