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

change mkHuman() to exclude unit conversion and format large numbers … #4850

Merged
merged 5 commits into from
Feb 10, 2020

Conversation

darthnater007
Copy link
Contributor

What does this PR do?

Agent statuses currently display imprecise numbers for large numbers (>1,000,000 shows a 1 M). This excludes thsi unit conversion in favor of more precise numbers from agent statuses

Motivation

Card: https://trello.com/c/5GcxDW96/1489-status-cmd-keep-precision-for-all-internal-stats-shown-by-status-command

Additional Notes

There is a test for the mkHuman() function as well.

My local test runs showed that this change forwards to the agent status command and the GUI (screenshots below):

Feed dummy value to MkHuman to test:
https://share.getcloudapp.com/2NurGqXY

GUI:
https://share.getcloudapp.com/jkun9PqJ

Command Line Status:
https://share.getcloudapp.com/4gux6nP5

@darthnater007 darthnater007 added [deprecated] team/agent-core Deprecated. Use metrics-logs / shared-components labels instead.. component/GUI labels Feb 6, 2020
@darthnater007 darthnater007 added this to the 7.18.0 milestone Feb 6, 2020
@darthnater007 darthnater007 requested a review from a team as a code owner February 6, 2020 19:28
@bits-bot
Copy link
Collaborator

bits-bot commented Feb 6, 2020

CLA assistant check
All committers have signed the CLA.

remeh
remeh previously approved these changes Feb 7, 2020
Copy link
Contributor

@remeh remeh left a comment

Choose a reason for hiding this comment

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

LGTM! Just left a small comment.
🚀

pkg/status/helpers.go Outdated Show resolved Hide resolved
Copy link
Contributor

@remeh remeh left a comment

Choose a reason for hiding this comment

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

LGTM!
We could reconsider the needs of the mkHuman method in helpers.go now that it only calls humanize, but in the end, it abstracts it and its nice to have it in our helpers anyway (it makes sense to have this kind of helper directly in the datadog-agent without having to think about which lib to import, etc.).
Thank you!

@darthnater007 darthnater007 merged commit eff1e0f into master Feb 10, 2020
@darthnater007 darthnater007 deleted the nate_perkins/precision_values_on_agent_status branch February 10, 2020 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/no-changelog component/GUI [deprecated] team/agent-core Deprecated. Use metrics-logs / shared-components labels instead..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants