Refactoring config handling for perfmon metricset #3854
Conversation
|
Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run. |
1 similar comment
|
Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run. |
| result[v.GroupName] = groupVal | ||
| doubleValue := (*float64)(unsafe.Pointer(&v.displayValue.LongValue)) | ||
|
|
||
| result[v.counterName] = *doubleValue |
There was a problem hiding this comment.
You should use here result.Put(v.counterName, *doubleValue), see https://github.com/elastic/beats/blob/master/libbeat/common/mapstr.go#L105 This will create sub documents for each dot which will make it compatible with elasticsearch 2.x.
| - name: group | ||
| Grouping of different counters | ||
| fields: | ||
| - name: alias |
There was a problem hiding this comment.
Is the alias and query going to be part of the event? Based on your example object above this is not the case. Means we don't need it in here? I think this file can actually be empty as the name is defined by the namespace.
| "beat":{ | ||
| "hostname":"beathost", | ||
| "name":"beathost" | ||
| "@timestamp": "2016-05-23T08:05:34.853Z", |
There was a problem hiding this comment.
Not an issue for this PR but I assume this currently manually created? We should create this automatically in the future and make sure in the docs it is made clear that it is only an example.
| @@ -34,7 +29,7 @@ func init() { | |||
| // multiple fetch calls. | |||
| type MetricSet struct { | |||
| mb.BaseMetricSet | |||
| counters []CounterConfig | |||
| counters []CounterConfigGroup | |||
There was a problem hiding this comment.
I see that this in the MetricSet, but do we ever need it outside the New part? It seems to be all in the handle/query anyways?
| if err != ERROR_SUCCESS { | ||
| return nil, PdhError(err) | ||
| } | ||
| counters[i] = Counter{counterName: v.Alias, counterPath: v.Query} |
There was a problem hiding this comment.
👍 It looks less complex with fewer loops.
|
@andrewkroh, @ruflin. Does this PR make sense? Because i see that @andrewkroh had refactor a lot of code in #3871 which makes this PR unnecessary i think. |
|
I would first merge @andrewkroh changes and then see what is still left. |
This PR simplified the using of the config for the perfmon metricset.
Sample output