Remove container_name from Metricbeat autodiscover eventID #14787
Conversation
ChrsMark
added
bug
Metricbeat
In Metricbeat we don't want to have different eventIDs for containers within the same Pod, since these containers share the same IP and handling them seperately can lead in launching hint based modules for all of the containers. Signed-off-by: chrismark <chrismarkou92@gmail.com>
ChrsMark
force-pushed
the
unify_eventid_autodiscover_metricbeat
branch
from
0effaad
to
c79a817
Compare
ChrsMark
changed the title
There was a problem hiding this comment.
Great analysis for the issue in #12011!
Regarding the fix, I wonder if it will work if we want to monitor ports of multiple containers in the same pod.
I also don't like to add conditional logic for a single beat in libbeat, but I could agree with this if it fixes the issue by now without breaking anything else. And if we plan to continue looking for other solutions.
Regarding longer-term possible solutions for this, if we see that different beats can be interested on different things (filebeat needs to know about all the containers to get their logs, but it doesn't care about the ports, metricbeat is interested about the ports exposed by the pod, but not so much about each container), maybe we should emit different kinds of events, with different information, and each beat in their hints builder would decide what events to attend or ignore.
For example, we could emit:
- An event per pod with the network information (all containers in a pod share the same network), including a list of all the ports exposed by their containers, and maybe the list of containers to match container-specific hints. This event could be used by beats interested on network endpoints, like metricbeat or heartbeat.
- An event per container, without network information. This event could be used by filebeat to get the logs of each container.
I am not sure what would be the best approach to differentiate these events, it could be by the presence or absence of certain fields, or we could add an extra field to distinguish them.
| @@ -217,7 +217,10 @@ func (p *Provider) emitEvents(pod *kubernetes.Pod, flag string, containers []kub | |||
|
|
|||
| // This must be an id that doesn't depend on the state of the container | |||
| // so it works also on `stop` if containers have been already deleted. | |||
| eventID := fmt.Sprintf("%s.%s", pod.GetObjectMeta().GetUID(), c.Name) | |||
| eventID := fmt.Sprintf("%s", pod.GetObjectMeta().GetUID()) | |||
There was a problem hiding this comment.
Would this still work if we want to monitor more than one container per pod? For example if a pod has more than one container, each one exposing a port, and each one with a dedicated annotation. Something like this:
apiVersion: v1
kind: Pod
metadata:
name: two-containers-prometheus
annotations:
co.elastic.metrics.prometheus-container/module: prometheus
co.elastic.metrics.prometheus-container/hosts: ${data.host}:8080
co.elastic.metrics.redis-container/module: redis
co.elastic.metrics.redis-container/hosts: ${data.host}:6379
spec:
restartPolicy: Never
containers:
- name: prometheus-container
image: prom/prometheus
ports:
- containerPort: 8080
- name: redis-container
image: redis
ports:
- containerPort: 6379
There was a problem hiding this comment.
we have many use cases that use this feature. also, it would be necessary what container is logging the metric. We run filebeat and metricbeat on the same daemonset. since most of the metrics are same, we would need to be able to know what container the app is reporting from.
| @@ -217,7 +217,10 @@ func (p *Provider) emitEvents(pod *kubernetes.Pod, flag string, containers []kub | |||
|
|
|||
| // This must be an id that doesn't depend on the state of the container | |||
| // so it works also on `stop` if containers have been already deleted. | |||
| eventID := fmt.Sprintf("%s.%s", pod.GetObjectMeta().GetUID(), c.Name) | |||
| eventID := fmt.Sprintf("%s", pod.GetObjectMeta().GetUID()) | |||
| if p.bus.GetName() != "metricbeat" { | |||
There was a problem hiding this comment.
I would avoid this kind of ifs, if at some point in the future we want to use autodiscover monitor network endpoints in other beat (for example with heartbeat), we can get crazy finding why it behaves differently in metricbeat.
If we decide to go on with this solution as a quick fix for #12011 please create a follow-up issue to find a longer term solution.
There was a problem hiding this comment.
+1 on avoiding beat specific checks on libbeat.
|
Thanks for the comments everyone! Closing this for now and we could consider about possible solutions in #12011. |
In Metricbeat we don't want to have different eventIDs for
containers within the same Pod, since these containers
share the same IP and handling them seperately can lead in
launching hint based modules for all of the containers.
Signed-off-by: chrismark chrismarkou92@gmail.com
What this PR solves
This PR tackles the problem of having a module to be launched twice in case we have 2 containers in the Pod with annotations. See #12011 .
This patch will make
beats/libbeat/autodiscover/autodiscover.go
Line 211 in d8cd8c4
work as expected, in cases like:
closes: #12011