[processor/k8sattributes] Automatic resource attributes

[zeitlinger]

Description

Implements https://github.com/open-telemetry/semantic-conventions/blob/main/docs/non-normative/k8s-attributes.md#how-serviceversion-should-be-calculate

The OTel operator has a set of rules to create resource attributes - which is described here.

This PR allows the collector to create resource attributes with the same logic.

Why

If the operator supplies resource attributes (that are sent with OTLP), it seems redundant that the collector should be able to do the same - it will just overwrite the resource attribute with identical values.

This feature gets interesting if you are not only getting all data from OTLP - but some data elsewhere, e.g. from the file log receiver.

It's crucial to use the same values for resource attributes across signals - this makes correlation possible.
For example, it allows you to find file based the log entries on the same service instance as a trace you're viewing (around the same time).

Alternatives

The same result can be achieved using the transforprocessor - but it's a very long and hard-to-understand configuration.
It that way, it's similar to the container parser.

Testing

Unit tests were added - e2e tests later

Documentation

Added here
This also has a complete config.

[fatsheep9146]

Could you please add a changelog and also updating the document to describe how to use this feature?

[zeitlinger]

@fatsheep9146 added changelog and docs

I also tried adding e2e tests - but didn't get it to work. Please advise if/how to do this.

[zeitlinger]

@TylerHelmuth @ChrsMark can you take a look?

[fatsheep9146]

Could we use like resource_labels_enabled instead of labels? I think labels are easy to be confused with other field also called labels, but they are of slice type.

[zeitlinger]

here's the config format I have in mind:

    automatic_attributes:
      # Apply the operator rules - see https://github.com/open-telemetry/semantic-conventions/blob/main/docs/non-normative/k8s-attributes.md
      well_known_labels: true # default is false
      annotation_prefixes: ["foo/"] # default is ["resource.opentelemetry.io/"] - use empty list to disable
      exclude: ["service.version"] # default is empty list

1. the feature to add the attributes is added when the automatic_attributes: key is added (even when it has no content

• this is similar to how you can add http: otlp receiver
• it would require that the existence of the key can be checked as in https://github.com/open-telemetry/opentelemetry-collector/blob/96e860b9cbd815788142918801950d5fc93bcb0d/receiver/otlpreceiver/config.go#L71 or https://github.com/open-telemetry/opentelemetry-collector/blob/13470870dbb6b4c52f853c72c9dd8393d1286a08/otelcol/config.go#L23 - not sure if that's possible here

2. allow the customization of annotation prefix
3. allow to exclude attributes

@ChrsMark @fatsheep9146 wdyt?

[fatsheep9146]

I'm ok with the key automatic_attributes and well_known_labels
I wonder that why we need annotation_prefixes, you mentioned that you want to implement the same logic with The OTel operator, but I don't see the otel operator can support user-defined annotation prefix. (Maybe I missed it)

[zeitlinger]

Note: The first split-off PR is #38825

• annotation_prefixes is not part of the operator
• I thought that some more flexibility is in the spirit of the collector - but no strong opinion

[ChrsMark]

So the #38825 adds the otel_annotations setting which will handle the "resource.opentelemetry.io/foo" becomes "foo" part. Do we we have consensus on the rest of the settings?

[zeitlinger]

I think so - since it's defined in the otel semantic conventions: https://opentelemetry.io/docs/specs/semconv/non-normative/k8s-attributes/

[fatsheep9146]

@zeitlinger I want to make sure I understand it right, so the scenario of this pr is like, if I create a pod PodA which export log to a otel-collector which has k8sattributes processor enabled with this operator_rules.enabled to be true.

Then if the PodA has a annotation called

resource.opentelemetry.io/foo: "bar"

Then the log exported to the otel-collector will have a new attribute called foo added to its resource attributes with value bar?

[ChrsMark]

This proposal has shown up a couple of times in the past. We need to decide on where is the best place to implement this.

Check open-telemetry/opentelemetry-helm-charts#905 and #27261.

[zeitlinger]

@zeitlinger I want to make sure I understand it right, so the scenario of this pr is like, if I create a pod PodA which export log to a otel-collector which has k8sattributes processor enabled with this operator_rules.enabled to be true.

Then if the PodA has a annotation called

resource.opentelemetry.io/foo: "bar"

Then the log exported to the otel-collector will have a new attribute called foo added to its resource attributes with value bar?

correct

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[zeitlinger]

please keep open - the spec issue is still ongoing

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[zeitlinger]

please keep open

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[zeitlinger]

the spec is merged - I'm planning to finish the PR this week

[zeitlinger]

the spec is merged - I'm planning to finish the PR this week

PR has been updated to reflect the spec
Tests will be fixed once the config syntax is agreed upon

[atoulme]

Please resolve the CI issues:

Error: k8sattributesprocessor/internal/kube/client.go:89:5: error-naming: error var cannotRetrieveImage should have name of the form errFoo (revive)
var cannotRetrieveImage = errors.New("cannot retrieve image name")
    ^

Please also address the conflict.

Once ready, please mark the PR as ready to review again.

[zeitlinger]

@dmitryax as discussed in SIG, I'll break it down im multiple PRs, starting with the annotations

[zeitlinger]

@zeitlinger I want to make sure I understand it right, so the scenario of this pr is like, if I create a pod PodA which export log to a otel-collector which has k8sattributes processor enabled with this operator_rules.enabled to be true.

Then if the PodA has a annotation called

resource.opentelemetry.io/foo: "bar"

Then the log exported to the otel-collector will have a new attribute called foo added to its resource attributes with value bar?

that's correct - that's what https://github.com/open-telemetry/semantic-conventions/blob/main/docs/non-normative/k8s-attributes.md#specify-resource-attributes-using-kubernetes-annotations says

[zeitlinger]

This proposal has shown up a couple of times in the past. We need to decide on where is the best place to implement this.

Check open-telemetry/opentelemetry-helm-charts#905 and #27261.

Those are older discussions about semantic conventions - which have now been merged: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/non-normative/k8s-attributes.md

This PR is about implementing this spec.

[zeitlinger]

Converted to draft, because some PRs will be split off first - #38825 so far

[dmitryax]

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

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.

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

[zeitlinger]

the second and (maybe) final part is #39335