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

Use Available memory instead of Free to compute system.memory.usage:free #24033

Closed
wants to merge 3 commits into from

Conversation

ItsLastDay
Copy link
Contributor

@ItsLastDay ItsLastDay commented Jul 7, 2023

Description: Fixes a bug where system.memory.usage:free has strange values

Link to tracking Issue: #24031

Testing: Not yet tested

Documentation: nothing added. What should I add?

@crobert-1
Copy link
Member

You'll need to add a changelog entry for this change since this will have user impact.

Copy link
Member

@crobert-1 crobert-1 left a comment

Choose a reason for hiding this comment

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

From the documentation you referenced it looks like this is the right variable to use.

It would be really good to have a comparison before and after the change to confirm that the new metric being used is what's expected, and charts (like you shared in your original issue) make sense with the new values.

Copy link
Member

@dmitryax dmitryax left a comment

Choose a reason for hiding this comment

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

Please look at this issue first #7417 . I don't believe we agreed on this solution. The work on that issue got stuck, but there is a System semantic conventions WG started recently, so it's a good chance to come to a solution finally. Please feel free to join open-telemetry/opentelemetry-specification#3556. There are weekly calls on Web 8:30 AM PST.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2023

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

@github-actions github-actions bot added the Stale label Aug 3, 2023
@ItsLastDay
Copy link
Contributor Author

Thank you Dmitrii for information! I'll close this PR and my issue, and will continue in #7417.

I see that Braydon (who's also from Google) expressed interest in helping with #7417 in open-telemetry/opentelemetry-specification#3556 (comment). So I hope I'll personally be able to skip the WG meetings, and instead provide all the context asynchronously through comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants