Skip to content

[receiver/k8sclusterreceiver] Add missing attributes to entities in experimental entity feature #39038

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

Merged
merged 27 commits into from
Apr 7, 2025

Conversation

k-shaikh
Copy link
Contributor

@k-shaikh k-shaikh commented Mar 28, 2025

Description

Modified entity payloads to include additional attributes for the following entities. Some of which are mentioned here -

Container

  • k8s.container.name
  • container.image.name
  • container.image.tag
  • k8s.pod.name
  • k8s.pod.uid
  • k8s.namespace.name
  • k8s.node.name

Pod

  • k8s.namespace.name
  • k8s.pod.name

Workload

  • k8s.namespace.name

Link to tracking issue

Partially Resolves: #38687

Testing

Updated unit tests to validate the desired attributes for the mentioned entities are included in the metadata for that entity

Documentation

None.

…lowing entities

Container
- container.name
- container.uid
- k8s.pod.name
- k8s.pod.uid

Pod
- k8s.namespace.name
- k8s.pod.name

Workload
- k8s,namespace.name
@k-shaikh k-shaikh requested review from dmitryax, TylerHelmuth, ChrsMark and a team as code owners March 28, 2025 22:35
Copy link

linux-foundation-easycla bot commented Mar 28, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link
Member

@dmitryax dmitryax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed couple of things

@@ -23,6 +24,9 @@ const (
containerKeyStatus = "container.status"
containerKeyStatusReason = "container.status.reason"
containerCreationTimestamp = "container.creation_timestamp"
containerName = "k8s.container.name"
containerImageName = "container.image.name"
containerImageTag = "container.image.tag"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
containerImageTag = "container.image.tag"
containerImageTags = "container.image.tags"

I think this needs to be alist based on https://opentelemetry.io/docs/specs/semconv/attributes-registry/container/#container-image-tags

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@povilasv , the current struct for sending metadata only allows values of type string. We can change the label to 'container.image.tags' but changing the value to a list will be very big change.

IMO, we can use the label container.image.tag since its also used here

Also even though an image may contain multiple tags the container spec references a specific tag so the multiple tags maybe relevant for the 'Image' entity it may not be relevant for the container entity.

@dmitryax , do you have any opinion?

Copy link
Member

@dmitryax dmitryax Apr 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the collector currently doesn't send any container.image.tags attribute, only container.image.tag, it'd be in inconsistent state sending data that cannot be easily correlated. I believe, in order to migrate from container.image.tag to container.image.tags attribute, we need to apply the changes to all sources consistently via a feature gate, maybe even keeping the old behavior as an option.

Copy link
Member

@dmitryax dmitryax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@povilasv, is this good to go from your perspective?

@povilasv
Copy link
Contributor

povilasv commented Apr 7, 2025

Good to go

@dmitryax
Copy link
Member

dmitryax commented Apr 7, 2025

The failing test is unrelated, #39210

@dmitryax dmitryax merged commit 85eb186 into open-telemetry:main Apr 7, 2025
171 checks passed
@github-actions github-actions bot added this to the next release milestone Apr 7, 2025
dmathieu pushed a commit to dmathieu/opentelemetry-collector-contrib that referenced this pull request Apr 8, 2025
…xperimental entity feature (open-telemetry#39038)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
Modified entity payloads to include additional attributes for the
following entities. Some of which are mentioned here -

Container
- k8s.container.name
- container.image.name
- container.image.tag
- k8s.pod.name
- k8s.pod.uid
- k8s.namespace.name
- k8s.node.name

Pod
- k8s.namespace.name
- k8s.pod.name

Workload
- k8s.namespace.name

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Partially Resolves:
open-telemetry#38687

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Updated unit tests to validate the desired attributes for the mentioned
entities are included in the metadata for that entity

<!--Describe the documentation added.-->
#### Documentation
None.

<!--Please delete paragraphs that you did not use before submitting.-->
LucianoGiannotti pushed a commit to LucianoGiannotti/opentelemetry-collector-contrib that referenced this pull request Apr 9, 2025
…xperimental entity feature (open-telemetry#39038)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
Modified entity payloads to include additional attributes for the
following entities. Some of which are mentioned here -

Container
- k8s.container.name
- container.image.name
- container.image.tag
- k8s.pod.name
- k8s.pod.uid
- k8s.namespace.name
- k8s.node.name

Pod
- k8s.namespace.name
- k8s.pod.name

Workload
- k8s.namespace.name

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Partially Resolves:
open-telemetry#38687

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Updated unit tests to validate the desired attributes for the mentioned
entities are included in the metadata for that entity

<!--Describe the documentation added.-->
#### Documentation
None.

<!--Please delete paragraphs that you did not use before submitting.-->
Fiery-Fenix pushed a commit to Fiery-Fenix/opentelemetry-collector-contrib that referenced this pull request Apr 24, 2025
…xperimental entity feature (open-telemetry#39038)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
Modified entity payloads to include additional attributes for the
following entities. Some of which are mentioned here -

Container
- k8s.container.name
- container.image.name
- container.image.tag
- k8s.pod.name
- k8s.pod.uid
- k8s.namespace.name
- k8s.node.name

Pod
- k8s.namespace.name
- k8s.pod.name

Workload
- k8s.namespace.name

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Partially Resolves:
open-telemetry#38687

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Updated unit tests to validate the desired attributes for the mentioned
entities are included in the metadata for that entity

<!--Describe the documentation added.-->
#### Documentation
None.

<!--Please delete paragraphs that you did not use before submitting.-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Additional K8s entity attributes are needed
4 participants