opentelemetry-erlang-contrib
opentelemetry-erlang-contrib copied to clipboard
add metrics collector for BEAM VM
I'm not sure that this doesn't belong in the sdk since it's vm runtime. @tsloughter thoughts? I'm not sure how this is being handled in the other languages. I do know the otel collector receivers just hook into existing APIs of the thing they're integrated with and are configurable.
Some general notes on first glance, forgive me if I misinterpreted something. I see that you relied heavily on prior art but I believe a fair amount of it doesn't comply with otel requirements.
- The naming here does not comply with semantic conventions for runtime names. Please review and adjust according to https://opentelemetry.io/docs/specs/semconv/general/metrics/
- I think some units will need to be adjusted. Review the conventions and spec carefully.
- How is filtering handled? Is that handled someplace else? In general, defaults are set for each available metric in a receiver, for instance, with the user having the ability to opt in and out of the defaults.
@bryannaegele
The naming here does not comply with semantic conventions for runtime names. Please review and adjust according to https://opentelemetry.io/docs/specs/semconv/general/metrics/
The intention was to keep the metric names compatible with VM collectors from prometheus.erl. That way the OTel instrumentation would be a simple drop-in replacement for projects that currently use prometheus.erl and Grafana dashboards from there could be reused with minimum changes.
However, I do like the semantic conventions from OTel better than the old names. So, I'll change to them.
How is filtering handled? Is that handled someplace else?
I was under the impression that filtering would always be handled in a collector and not in the metric. Can you point me to example (even in anther language) where filtering is done in the meter?
I don't have the time or context for that, so relying on you and Tristan to do that legwork ;)
@bryannaegele it looks like Java has their's in contrib, so probably belongs here. I'd have ben fine either way
Cool.
What do y'all think of making the prefix be beam rather than erlang.vm? I don't think the double-namespace provides anything and it provides better specificity imo.
@bryannaegele @tsloughter updated:
- uses prefix
beam - split some observable callbacks
- based on proposed semantic conventions from https://github.com/open-telemetry/semantic-conventions/pull/827
@RoadRunnr thanks! taking another look.
new PR for semantic conventions for BEAM. This PR here will need to synced once that has been agreed upon.