Kubernetes state_* metricsets are ignoring namespace #6281 #6294
Conversation
Conflicts: metricbeat/docs/modules_list.asciidoc
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
Can you please add a changelog entry? jenkins, test it please |
|
jenkins, retest it please |
|
Added changelog entry... someday I'll remember to add it upfront!!! ;) ;) ;) |
| @@ -21,11 +21,14 @@ func eventMapping(families []*dto.MetricFamily) ([]common.MapStr, error) { | |||
| if deployment == "" { | |||
| continue | |||
| } | |||
| event, ok := eventsMap[deployment] | |||
| namespace := util.GetLabel(metric, "namespace") | |||
| deploymentKey := namespace + "::" + deployment | |||
There was a problem hiding this comment.
@exekias Does this mean we have :: in the field key?
There was a problem hiding this comment.
No, this is a key in the eventMap, we use it to correlate all different metrics into a single event, then we ship events, forgetting about the keys in the map
| if err == nil { | ||
| namespace, err := event.GetValue("_module.namespace") | ||
| if err == nil { | ||
| eventKey := namespace.(string) + "@" + name.(string) |
There was a problem hiding this comment.
And I assume this one means an @ in the field key? Not that ES would disallow this, but I wonder if it could cause issues on the query side or KB?
There was a problem hiding this comment.
This @ works in the same way, it's used to fetch the correct test case from here: https://github.com/elastic/beats/pull/6294/files/b514a509da8360ab225e85eb73df79468057cbd7#diff-2b3ff3319749ebeecd269e64715d1969R80
The @ is not in the final event
Most of this was fixed by elastic#6294, but for the specific case of containers, their uniqueness is defined by namespace + pod name. As several pods may have containers with the same name. Rest of metrics are not affected in the same way, and namespace should be enough for them.
As discussed here: #6236
Requested by @exekias
Fixes #6281