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

Collector resource detection shouldn't override library resource attributes #1521

Open
rakyll opened this issue Mar 9, 2021 · 3 comments
Open
Labels
spec:resource Related to the specification/resource directory

Comments

@rakyll
Copy link
Contributor

rakyll commented Mar 9, 2021

The spec specifies a merge behavior where the updating resource is overriding the existing resource attributes.

The resulting resource MUST have all attributes that are on any of the two input resources. If a key exists on both the old and updating resource, the value of the updating resource MUST be picked (even if the updated value is empty).

This behavior is unfortunately not great if collector detects and overrides resources via the resourcedetectionprocessor (https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/processor/resourcedetectionprocessor). For example, in cases where libraries are sending telemetry data to a collector running on a different host, this model will lead to the loss of host-specific attributes.

We should recommend collector not to override the resource attributes coming from client libraries.

@rakyll rakyll added the spec:resource Related to the specification/resource directory label Mar 9, 2021
@arminru
Copy link
Member

arminru commented Mar 16, 2021

I don't think the Resource SDK spec mentioned above applies to the collector but rather only the client libraries.

The resourcedetectionprocessor you mentioned allows to configure if it should override existing attributes or not:
https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/processor/resourcedetectionprocessor#configuration

# determines if existing resource attributes should be overridden or preserved, defaults to true
override: <bool>

It says that the default is true. Given the reasons you stated above, I think it would indeed make sense to have it default to false.
Could you open an issue/PR right in the collector(-contrib) repo for that or should we gather opinions/agreement here first?

@jmacd
Copy link
Contributor

jmacd commented Mar 16, 2021

I see a connection with #1298

@jmacd
Copy link
Contributor

jmacd commented Mar 16, 2021

Also when Prometheus does federation they use an option named "honor_labels" to enforce this type of behavior: https://prometheus.io/docs/prometheus/latest/federation/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:resource Related to the specification/resource directory
Projects
None yet
Development

No branches or pull requests

3 participants