google / cadvisor

Analyzes resource usage and performance characteristics of running containers.
Other
16.85k stars 2.31k forks source link

Cadvisor create container metrics in diffirent ways for containerd and cadvisor for "name" filed #2965

Open xiaoping8385 opened 2 years ago

xiaoping8385 commented 2 years ago

Hi there, we recently notice that when using cadvisor to get metrics from the clusters, our docker clusters have a human-readable value for the name label, but the containerd clusters have a long hex value,checking the source code we found: cadvisor generates the Aliases field in different ways for docker and containerd.

dockerd: Aliases = [name, id]: https://github.com/google/cadvisor/blob/aa0afbee6da04e8d8a214333bf2747f0ae27f7a0/container/docker/handler.go#L215 containerd: Aliases = [id, name]: https://github.com/google/cadvisor/blob/2dd0cd908dca0c7e3ca255f601ab80a0ef2ddbc1/container/containerd/handler.go#L119

As far as the prometheus reference is hard coded as “container.Aliases[0]“,https://github.com/google/cadvisor/blob/2b6fbacac7598e0140b5bc8428e3bdd7d86cf5b9/metrics/prometheus.go#L1849 it will leads to two different results if using prometheus interface to obtain the metrics.

So is there any reason we design like this? or it is a cadvisor bug?

xiaoping8385 commented 2 years ago

@abhi I notice you submitted the change at the very beginning, could you please take a look at this, thanks!

bobbypage commented 2 years ago

/cc @qiutongs

pknight37 commented 2 years ago

Hi do you have any update on this issue?

qiutongs commented 2 years ago

I don't know the history but I also noticed this difference. The docker has k8s_<container-name>_<pod-name>_<namespace>_<pod-uid>_<restart-count> format.

I would consider this is a cadvisor bug as it doesn't provide consistent naming format. A few months ago, I merged this patch https://github.com/google/cadvisor/commit/56f032e06ddf2c7a700fcce51cb9e009d809f5b0 to a separate branch https://github.com/google/cadvisor/tree/containerd-cri. I couldn't merge it to master because it integrates with CRI package and will incur a circular dependency issue when cadvisor is vendored back into k8s.

ystkfujii commented 4 months ago

Hi do you have any update on this issue?