-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[processor/k8sattributes] Operator resource attributes #37114
base: main
Are you sure you want to change the base?
[processor/k8sattributes] Operator resource attributes #37114
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add a changelog and also updating the document to describe how to use this feature?
@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. |
@TylerHelmuth @ChrsMark can you take a look? |
# app.kubernetes.io/version => service.version | ||
# app.kubernetes.io/part-of => service.namespace | ||
# This setting is ignored if 'enabled' is set to false | ||
labels: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
key: app.kubernetes.io/component | ||
from: pod | ||
operator_rules: | ||
# Apply the operator rules - see https://github.com/open-telemetry/opentelemetry-operator#configure-resource-attributes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would avoid calling this feature operator_rules
. If we decide on supporting this from the Collector at first place then it should have a generic name not coupled to the Operator project.
Also from this section it's not clear if we support both labels and annotations (as described in the Operator's docs). Is sth like resource.opentelemetry.io/service.name: "my-service"
supported?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
avoid calling this feature...
good idea - I just didn't find a good name so far
annotations and labels
the linked page describes it:
Annotations like this
annotations:
# this is just an example, you can create any resource attributes you need
resource.opentelemetry.io/service.name: "my-service"
resource.opentelemetry.io/service.version: "1.0.0"
resource.opentelemetry.io/deployment.environment.name: "production"
labels like this
apiVersion: v1
kind: Pod
metadata:
name: example-pod
labels:
app.kubernetes.io/name: "my-service"
app.kubernetes.io/version: "1.0.0"
app.kubernetes.io/part-of: "shop"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#27261 suggests well_known_labels
# Apply the operator rules - see https://github.com/open-telemetry/opentelemetry-operator#configure-resource-attributes | ||
enabled: true | ||
# Also translate the following labels to the specified resource attributes: | ||
# app.kubernetes.io/name => service.name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is mentioned before that these should be standardized at some point: open-telemetry/opentelemetry-helm-charts#905
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK - I'll work with semconv before I continue with this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've created open-telemetry/semantic-conventions#1756 for the spec part
correct |
Description
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.