New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add metrics collector for BEAM VM #299
base: main
Are you sure you want to change the base?
Conversation
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 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.
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 ;) |
instrumentation/opentelemetry_beam/src/opentelemetry_vm_system_info.erl
Outdated
Show resolved
Hide resolved
Meter, 'erlang.vm.statistics.garbage_collection_words_reclaimed', | ||
#{description => <<"Garbage collection: words reclaimed.">>})], | ||
|
||
otel_meter:register_callback(Meter, Instruments, fun vm_stats/1, []), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooops, completely missed these were created without callbacks, haha.
So yea, this is good.
But still I'd suggest grouping them so they aren't all read even if you only want certain of them? What do you think?
@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 |
@bryannaegele @tsloughter updated:
|
No description provided.