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

[receiver/k8scluster] Invalid image name and tag attributes when containing a digest #36279

Open
rogercoll opened this issue Nov 8, 2024 · 5 comments
Labels
bug Something isn't working receiver/k8scluster

Comments

@rogercoll
Copy link
Contributor

Component(s)

receiver/k8scluster

What happened?

Description

If a container that is being instrumented using the k8scluster receiver has been deployed using a container image that contains a cryptographic digest, the receiver produces invalid attributes. The regex used to parse the container does not apply to images with a digest.

Steps to Reproduce

  1. Deploy a collector in K8s with the k8scluster receiver enabled
  2. Deploy a Pod (as a daemonset or any other deployment strategy) with a container's image that contains a digest. For example (simplified output of a pod description):
Containers:
  opentelemetry-collector:
    Container ID:  containerd://6a8fb288d22908242a8508d8625e5a328307a09218a2e659cc9457a0c7a5fe28
    Image:         docker.elastic.co/beats/elastic-agent@sha256:52eeed9e66facc651d4344ef7d9ce912fccff5bb3969e745eed3ab953f309534
    Image ID:      docker.elastic.co/beats/elastic-agent@sha256:52eeed9e66facc651d4344ef7d9ce912fccff5bb3969e745eed3ab953f309534

Expected Result

Resource attributes:
     -> container.image.name: Str(docker.elastic.co/beats/elastic-agent)

Actual Result

Resource attributes:
     -> container.image.name: Str(sha256)
     -> container.image.tag: Str(a048efd6da2a01c20fb2342b624d740e44a7f651183b017c1962835d9e05186a)

Collector version

v0.112.0

Environment information

Environment

OS: (e.g., "Ubuntu 20.04")
Compiler(if manually compiled): (e.g., "go 14.2")

OpenTelemetry Collector configuration

receivers:
   k8s_cluster:
     auth_type: serviceAccount
    metrics:
      k8s.pod.status_reason:
        enabled: true

Log output

No response

Additional context

No response

@rogercoll rogercoll added bug Something isn't working needs triage New item requiring triage labels Nov 8, 2024
Copy link
Contributor

github-actions bot commented Nov 8, 2024

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@rogercoll
Copy link
Contributor Author

The following commit makes the receiver's tests fail: e47aa1a

What do you think if instead of using our custom image parser we use the K8s one? Similar to what I am proposing in this draft PR: https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/36281/files

@bacherfl
Copy link
Contributor

I also just had a look and it seems like the current regex expects the sha256 digest to consist of digits only.

I tried to adapt the existing regex to handle this, as well as images containing no tags, as included in the draft PR, so something like the following may be worth a try to use instead of the previous regex:

^(?P<repository>(?:[^/\s]+\/)*[\w\.-]+)(?::(?P<tag>[\w\.-]+))?(?:@sha256:(?P<sha256>[A-Fa-f0-9]+))?$

This may be an alternative to consider if the code owners would like to avoid adding a new dependency for this. However I do see the value of having an established library doing the parsing, as it may be easy to miss certain cases with an own regex.

Pinging the code owners (@dmitryax @TylerHelmuth @povilasv) - which approach would you say is the best in this case?

@bacherfl bacherfl removed the needs triage New item requiring triage label Nov 13, 2024
@ChrsMark
Copy link
Member

There is a related PR fixing the k8sattributes processor: #36145

Could we apply a similar logic here?

@rogercoll
Copy link
Contributor Author

There is a related PR fixing the k8sattributes processor: #36145

Could we apply a similar logic here?

Is there any reasoning behind a new function to parse the image in the k8sattributes processor? I would rather fix the internal docker.ParseImageName function to parse images with digest and use it across all components.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working receiver/k8scluster
Projects
None yet
Development

No branches or pull requests

3 participants