[processor/k8sattributes] Automatic resource attributes - annotation prefix #38825

zeitlinger · 2025-03-20T11:19:47Z

This is the first part of #37114 - the ability to use an annotation prefix to define a resource attribute

zeitlinger · 2025-03-20T12:23:34Z

@dmitryax as discussed, this is the first PR for automatic resource attributes

mattsains

I am not an approver unfortunately, but I looked through this PR and it looks good to me

mattsains · 2025-04-02T18:20:55Z

perhaps @andrzej-stencel could take a look?

dmitryax

Let's agree on the configuration interface before proceeding https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/37114/files#r2025819122

zeitlinger · 2025-04-03T09:34:43Z

@dmitryax answering here:

Can we just move well known attributes from this section to where all the other pre-defined resource attributes are configured extract.metadata, and disabled by default?

Then this section would be responsible for taking resource attributes from annotations only. Instead of calling it automatic_attributes, it can be something like
extract:
  otel_annotations: <bool> // false by default

will do

Then, if we want to provide flexibility for annotations extraction, we could add extract.annotations with the same syntax as we currently have for extract.labels. That is not really needed right now.

will remove

Providing such interface would be consistent with the existing one and easier to understand by the end users. Please let me know WDYT.

sounds good 😄

zeitlinger · 2025-04-03T11:17:20Z

@dmitryax done - the test failures seem unrelated to this PR

processor/k8sattributesprocessor/internal/kube/automatic.go

dmitryax · 2025-04-09T04:57:03Z

I actually missed that we already have extract::annotations config option. What if instead of adding another boolean flag otel_annotations, we just add this rule by default in extract::annotations. Users who don't want it, can disable it by setting it to an empty slice. The prefix is very specific, I don't think it's being used extensively, so it shouldn't surprise the users. WDYT?

If that sounds good, we should clearly describe it in the changelog and how to disable it.

zeitlinger · 2025-04-09T09:55:26Z

I actually missed that we already have extract::annotations config option. What if instead of adding another boolean flag otel_annotations, we just add this rule by default in extract::annotations. Users who don't want it, can disable it by setting it to an empty slice. The prefix is very specific, I don't think it's being used extensively, so it shouldn't surprise the users. WDYT?

If that sounds good, we should clearly describe it in the changelog and how to disable it.

I like that 😄

zeitlinger · 2025-04-09T10:01:23Z

I actually missed that we already have extract::annotations config option. What if instead of adding another boolean flag otel_annotations, we just add this rule by default in extract::annotations. Users who don't want it, can disable it by setting it to an empty slice. The prefix is very specific, I don't think it's being used extensively, so it shouldn't surprise the users. WDYT?
If that sounds good, we should clearly describe it in the changelog and how to disable it.

I like that 😄

@dmitryax on second thought - what about users who do have their own attribute rules?

They would have to add a rule with the exact prefix - I would like to avoid that users have to know those details.

ChrsMark

I like the idea of keeping the extract:otel_annotations setting different from the extract::annotations. Changing the behaviour of extract::annotations now to include the logic of this PR would affect users as a breaking change and disabling this or just overriding the rules would require extra manual work for the users. In addition, it's not a bad design to group this rulings under a specific setting as a shortcut, right?

Based on this I'd lean towards going with the separate one.

processor/k8sattributesprocessor/README.md

ChrsMark · 2025-04-09T13:30:51Z

processor/k8sattributesprocessor/README.md

+        key: app.kubernetes.io/component
+        from: pod
+    # Apply the operator rules - see https://github.com/open-telemetry/semantic-conventions/blob/main/docs/non-normative/k8s-attributes.md
+    otel_annotations: true # default is false - use true to enable


I'm fine with the addition of this specific one. However, I wonder how this will be combined with the rest of the settings as described at #37114 (comment). Sorry if I miss any context here but that'd help clarifying this.

I think we'll need to have another 2 flag to cover this

Should we decide on those at a high level before adding this specific one? My preference would be to have alignment on the high level config schema before starting any partial implementations. Would that make sense?

I don't think we need any other config options for other things. See https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/37114/files#r2025819122

What about the well known labels that are described at https://github.com/open-telemetry/opentelemetry-operator?tab=readme-ov-file#configure-resource-attributes-with-labels and was originally added in https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/37114/files#diff-57b0d6448d516c0c80b326a7f7f95c3256b2b774e496870dc1dd35dcf8bfc256R281?

I suggest we just add them as options to the existing extract::metadata set https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/processor/k8sattributesprocessor/config.go#L156

Hmm, extract::metadata is a ~~map~~ string array so I'm not sure how we could add extra options without breaking it, right?
Also in this PR we extract the annotation through the extract::otel_annotations setting so I assume the equivalent would be extract::well_known_labels, @zeitlinger could you confirm if that's the direction you had in mind?

From my perspective I'm coverd with the current state and the future plans. @dmitryax feel free to resolve this comment if we are on agreement.

dmitryax

LGTM

dmitryax · 2025-04-09T16:30:32Z

They would have to add a rule with the exact prefix - I would like to avoid that users have to know those details.

I was thinking of avoiding complicating the configuration interface which is already big and complex, but this is a valid concern. So I agree with keeping another option.

Co-authored-by: Christos Markou <[email protected]>

ChrsMark · 2025-04-10T13:44:20Z

We will need a third one for the service attributes

I see, my only concern would be that the service attributes group will have an overlap with the other 2 but we can discuss it then (probably that should be fine though).

I'm going to approve this PR as is since this top level otel_annotations setting makes sense on its own and the path forward seems to be clear.

…prefix (open-telemetry#38825) This is the first part of open-telemetry#37114 - the ability to use an annotation prefix to define a resource attribute --------- Co-authored-by: Christos Markou <[email protected]>

zeitlinger requested review from dmitryax, fatsheep9146, TylerHelmuth, ChrsMark and a team as code owners March 20, 2025 11:19

github-actions bot assigned andrzej-stencel Mar 20, 2025

github-actions bot added the processor/k8sattributes k8s Attributes processor label Mar 20, 2025

atoulme added the waiting-for-code-owners label Mar 20, 2025

zeitlinger mentioned this pull request Mar 21, 2025

[processor/k8sattributes] Automatic resource attributes #37114

Closed

This was referenced Mar 23, 2025

Weekly Report: 2025-03-16 - 2025-03-23 LucaLanziani/opentelemetry-collector-contrib#16

Closed

Weekly Report: 2025-03-16 - 2025-03-23 LucaLanziani/opentelemetry-collector-contrib#17

Closed

github-actions bot mentioned this pull request Mar 25, 2025

Weekly Report: 2025-03-18 - 2025-03-25 #38935

Closed

github-actions bot mentioned this pull request Apr 1, 2025

Weekly Report: 2025-03-25 - 2025-04-01 #39070

Closed

mattsains approved these changes Apr 2, 2025

View reviewed changes

dmitryax requested changes Apr 3, 2025

View reviewed changes

zeitlinger force-pushed the automatic-attributes-from-annotations branch 2 times, most recently from 8de94f4 to 5ec4a2d Compare April 3, 2025 10:49

github-actions bot mentioned this pull request Apr 8, 2025

Weekly Report: 2025-04-01 - 2025-04-08 #39228

Closed

dmitryax reviewed Apr 9, 2025

View reviewed changes

processor/k8sattributesprocessor/internal/kube/automatic.go Outdated Show resolved Hide resolved

processor/k8sattributesprocessor/internal/kube/automatic.go Outdated Show resolved Hide resolved

zeitlinger force-pushed the automatic-attributes-from-annotations branch from 5ec4a2d to 496c904 Compare April 9, 2025 10:01

ChrsMark reviewed Apr 9, 2025

View reviewed changes

dmitryax approved these changes Apr 9, 2025

View reviewed changes

zeitlinger and others added 21 commits April 10, 2025 15:17

update service.version

426d023

add app.kubernetes.io/instance

6ff2190

rename

5aa2bc8

fix merge, revert implicit "enabled: true"

267dc4b

custom prefixes

8be905e

add, customizations and missing namespace rule

c80664b

add, customizations and missing namespace rule

6d017da

add, customizations and missing namespace rule

e8b4d5e

format

78c96b7

format

b89e877

format

6dd5617

strip down to just annotation prefix for the first PR

c3998c1

strip down to just annotation prefix for the first PR

ff06307

strip down to just annotation prefix for the first PR

6cef4a6

use otel_annotations

0ee345b

remove unneeded methods

1dd158e

fix rebase

88805c2

Update processor/k8sattributesprocessor/README.md

3ebfa3e

Co-authored-by: Christos Markou <[email protected]>

fix test

f73790f

add readme section

d0d1a5d

fix attribute

0e8a3c9

zeitlinger force-pushed the automatic-attributes-from-annotations branch from 952cd90 to 0e8a3c9 Compare April 10, 2025 13:17

ChrsMark approved these changes Apr 10, 2025

View reviewed changes

dmitryax merged commit 6682df5 into open-telemetry:main Apr 11, 2025
171 checks passed

github-actions bot added this to the next release milestone Apr 11, 2025

zeitlinger deleted the automatic-attributes-from-annotations branch April 11, 2025 08:34

dmitryax mentioned this pull request May 13, 2025

service resource attributes #39335

Merged

[processor/k8sattributes] Automatic resource attributes - annotation prefix #38825

[processor/k8sattributes] Automatic resource attributes - annotation prefix #38825

Uh oh!

Conversation

zeitlinger commented Mar 20, 2025

Uh oh!

zeitlinger commented Mar 20, 2025

Uh oh!

mattsains left a comment

Choose a reason for hiding this comment

Uh oh!

mattsains commented Apr 2, 2025

Uh oh!

dmitryax left a comment

Choose a reason for hiding this comment

Uh oh!

zeitlinger commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zeitlinger commented Apr 3, 2025

Uh oh!

Uh oh!

Uh oh!

dmitryax commented Apr 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zeitlinger commented Apr 9, 2025

Uh oh!

zeitlinger commented Apr 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChrsMark left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ChrsMark Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

zeitlinger Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

ChrsMark Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

dmitryax Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

ChrsMark Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

dmitryax Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

ChrsMark Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChrsMark Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

dmitryax left a comment

Choose a reason for hiding this comment

Uh oh!

dmitryax commented Apr 9, 2025

Uh oh!

ChrsMark commented Apr 10, 2025

Uh oh!

Uh oh!

Uh oh!

zeitlinger commented Apr 3, 2025 •

edited

Loading

dmitryax commented Apr 9, 2025 •

edited

Loading

zeitlinger commented Apr 9, 2025 •

edited

Loading

ChrsMark Apr 10, 2025 •

edited

Loading