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

Export another metric load / number of cores #1868

Closed
wants to merge 2 commits into from

Conversation

monicasarbu
Copy link
Contributor

@monicasarbu monicasarbu commented Jun 15, 2016

Implements #420

cc @andrewkroh @ruflin

@monicasarbu monicasarbu added review and removed review labels Jun 15, 2016
@@ -100,6 +100,11 @@ func (m *MetricSet) Fetch() (common.MapStr, error) {
"5": loadStat.Load5,
"15": loadStat.Load15,
}
cpuStat["load_normalized"] = common.MapStr{
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we go here with load.1 and load.normlized.1 or load.1.value and load.1.normlized. I think at least both should be in the namespace load. Not sure about the naming below it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that they should be under load. I suggest load.avg.1 and load.normalized.1.

@ruflin
Copy link
Contributor

ruflin commented Jun 15, 2016

I think the fields.yml updates are missing here (which means probably also the system tests will fail).

@@ -26,6 +26,7 @@ The goal is to have a similar experience across all metrics.
List of standardised words and units across all metricsets. On the left are the ones to be used, on the right the options seen in metricsets.

* avg: average
* norm: normalized
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should sort the list alphabetical

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, I didn't notice that it's sorted.

@monicasarbu monicasarbu force-pushed the add_load_normalized branch from 36d0a24 to 5ade3ca Compare June 15, 2016 14:19
Use load.avg.1, load.avg.5 and load.avg.15 for the average load
Use load.norm.1, load.norm.5, load.norm.15 for the load divided by the number of cores
The kibana dashboard for the system module needs to be updated

update kibana dashboard with the latest changes from the system module
@@ -1,5 +1,5 @@
{
"visState": "{\"title\":\"New Visualization\",\"type\":\"line\",\"params\":{\"shareYAxis\":true,\"addTooltip\":true,\"addLegend\":true,\"showCircles\":true,\"smoothLines\":false,\"interpolate\":\"linear\",\"scale\":\"linear\",\"drawLinesBetweenPoints\":true,\"radiusRatio\":9,\"times\":[],\"addTimeMarker\":false,\"defaultYExtents\":false,\"setYExtents\":false,\"yAxis\":{}},\"aggs\":[{\"id\":\"1\",\"type\":\"avg\",\"schema\":\"metric\",\"params\":{\"field\":\"system.cpu.load.1\",\"customLabel\":\"Load\"}},{\"id\":\"2\",\"type\":\"date_histogram\",\"schema\":\"segment\",\"params\":{\"field\":\"@timestamp\",\"interval\":\"auto\",\"customInterval\":\"2h\",\"min_doc_count\":1,\"extended_bounds\":{}}},{\"id\":\"3\",\"type\":\"terms\",\"schema\":\"group\",\"params\":{\"field\":\"beat.name\",\"size\":5,\"order\":\"desc\",\"orderBy\":\"1\"}}],\"listeners\":{}}",
Copy link
Contributor

@ruflin ruflin Jun 15, 2016

Choose a reason for hiding this comment

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

The kibana files are also in the _meta folder now, so changes should be made there. You can just copy the files over and then run make collect

@andrewkroh
Copy link
Member

LGTM

@ruflin
Copy link
Contributor

ruflin commented Jun 16, 2016

LGTM, but it seems to break the system tests for topbeat on windows.

@monicasarbu
Copy link
Contributor Author

Closed in favor of #2101

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.

3 participants