Skip to content
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 for the BEAM-VM #827

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

RoadRunnr
Copy link

Changes

Add metrics for the BEAM-VM

Merge requirement checklist

@RoadRunnr RoadRunnr marked this pull request as ready for review March 20, 2024 11:55
@RoadRunnr RoadRunnr requested review from a team as code owners March 20, 2024 11:55
@RoadRunnr
Copy link
Author

RoadRunnr commented Mar 20, 2024

@tsloughter @ferd following the comments on open-telemetry/opentelemetry-erlang-contrib#299, I've prepared a suggestion for the BEAM-VM metrics.

I think it makes sense to agree on a specification before continuing with the implementation.

| `nif` | Time spent in NIFs. Without extra states this time is part of the emulator state. | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| `send` | Time spent sending messages (processes only). Without extra states this time is part of the emulator state. | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| `timers` | Time spent managing timers. Without extra states this time is part of the other state. | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
<!-- endsemconv -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably repeat the microstate accounting caveat that seems to only exist in the declaration yaml and code for this section?

| Attribute | Type | Description | Examples | [Requirement Level](https://opentelemetry.io/docs/specs/semconv/general/attribute-requirement-level/) | Stability |
|---|---|---|---|---|---|
| `beam.memory.allocators.alloc` | string | The allocation type. | `sys_alloc` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| `beam.memory.allocators.instance_no` | int | The logical instance number [0..n] | `1` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like using beam.memory.allocators.instance here is not losing any descriptiveness but also lines up with what the OTP docs would provide?

@jsuereth
Copy link
Contributor

@tsloughter would love a review on this.

Copy link

github-actions bot commented May 5, 2024

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Discussions
Development

Successfully merging this pull request may close these issues.

None yet

4 participants