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

Few changes to the metricbeat exported fields #2101

Merged

Conversation

monicasarbu
Copy link
Contributor

@monicasarbu monicasarbu commented Jul 26, 2016

The PR includes the following changes:

  • create a load metricset in the system module
  • add system.load.norm.*
  • system.filesystem.avail is replaced with system.filesystem.available

NOTE: This PR also replacessystem.memory.actual.freewith system.memory.available and
removes system.memory.actual.used as it can be calculated from system.memory.total - system.memory.available. But these changes are reverted in the #2124.

@monicasarbu monicasarbu added the in progress Pull request is currently in progress. label Jul 27, 2016
@monicasarbu monicasarbu force-pushed the few_changes_to_the_metricbeat_exported_fields branch from fd05079 to 5a7d9d9 Compare July 27, 2016 11:40
@monicasarbu monicasarbu added review and removed in progress Pull request is currently in progress. labels Jul 27, 2016
"5": system.Round(loadStat.Load5, .5, 4),
"15": system.Round(loadStat.Load15, .5, 4),
},
"norm": common.MapStr{
Copy link
Member

Choose a reason for hiding this comment

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

I had to look up what the difference is between avg and norm here. As far as I know, the periods are in minutes. So it could be for avg: system.load.minute(s).1. Not sure how per core would fit into this model.

Alternatively what about system.load.1 and system.load.core.1 ?

Copy link
Member

Choose a reason for hiding this comment

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

Thinking about it again, norm is potentially better then I initially thought. it means, all the values here will be between 0-1 ? Third option: system.load.1 and system.load.norm.1. I'm curious if we should ship the number of cores as part of load, so the calculations to "normalise" could happen on the kibana side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good idea, to keep system.load.1. I will do the changes.

@ruflin
Copy link
Member

ruflin commented Jul 27, 2016

@monicasarbu For system.memory.actual.used is not needed as it can calculated from system.memory.total - [ ] system.memory.available it seems we need 2 out of the three. My initial thought was that the total should be the one that should be calculated and used and available should be shipped. The results is the same for me it just feels more natural that "total" would be a calculated value. What were your thoughts here?

@@ -21,6 +21,9 @@ https://github.com/elastic/beats/compare/v5.0.0-alpha4...master[Check the HEAD d
- The undocumented file output `index` setting was removed. Use `filename` instead. {pull}2077[2077]

*Metricbeat*
- Create a separate metricSet for load under the system module {pull}2101[2101]
Copy link
Member

Choose a reason for hiding this comment

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

It should also be mentioned, that load was removed from CPU.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

@monicasarbu monicasarbu force-pushed the few_changes_to_the_metricbeat_exported_fields branch 2 times, most recently from 0f8064a to c32fe54 Compare July 27, 2016 13:56
{
Copy link
Member

Choose a reason for hiding this comment

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

The previous example data had better example values. The new ones are all 0.

@andrewkroh
Copy link
Member

LGTM. I just left a minor comment that can be addressed at another time if you like.

- system.memory.actual.free is replaced with system.memory.available
- system.memory.actual.used is not needed as it can calculated from system.memory.total - [ ] system.memory.available
- system.filesystem.avail is replaced with system.filesystem.available
- add system.load.norm.*
@monicasarbu monicasarbu force-pushed the few_changes_to_the_metricbeat_exported_fields branch from 638b950 to b47ab14 Compare July 28, 2016 08:20
@tsg tsg merged commit 02c9b37 into elastic:master Jul 28, 2016
@monicasarbu monicasarbu mentioned this pull request Jul 28, 2016
@monicasarbu monicasarbu added the Metricbeat Metricbeat label Aug 1, 2016
@monicasarbu monicasarbu mentioned this pull request Aug 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants