Skip to content

Conversation

@jack-berg
Copy link
Member

@jack-berg jack-berg commented Feb 11, 2022

Allow you to exclude resource attribute keys with export OTEL_EXPERIMENTAL_RESOURCE_DISABLED_KEYS=foo,bar.

This stems from conversations about filtering out process.command_line resource attributes, which are lengthy, and may contain sensitive information.

Its also possible to do accomplish something similar by using otel.java.disabled.resource-providers to disable the entire ProcessResourceProvider, but that results in other useful process resource attributes being filtered out.

Its also possible to split out a separate ProcessCommandLineResourceProvider, so the command line can be disabled on its own.

Its also possible to disable process.command_line in ProcessResourceProvider by default, and make it opt in through some env var / system property.

However, I think having a general mechanism for this is a nice tool because it allows users to be able to make fine-grained decisions about each resource attribute, weighing the cost from the additional bytes on the wire against the value it provides.

I didn't find any conversation about this in the spec, but may have missed something. Its possible that this issue doesn't affect other language SDKs if they lack features to automatically detect resources (i.e. if the resource attributes are already opt in), but my hunch is that this would be a valuable feature to have general support.

@codecov
Copy link

codecov bot commented Feb 11, 2022

Codecov Report

Merging #4169 (ecc661c) into main (19b0fec) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #4169      +/-   ##
============================================
- Coverage     90.31%   90.28%   -0.03%     
- Complexity     4612     4615       +3     
============================================
  Files           537      537              
  Lines         14100    14107       +7     
  Branches       1348     1349       +1     
============================================
+ Hits          12734    12737       +3     
- Misses          923      927       +4     
  Partials        443      443              
Impacted Files Coverage Δ
...metry/sdk/autoconfigure/ResourceConfiguration.java 92.59% <100.00%> (+4.35%) ⬆️
...ension/trace/jaeger/sampler/OkHttpGrpcService.java 75.30% <0.00%> (-6.18%) ⬇️
...emconv/resource/attributes/ResourceAttributes.java 100.00% <0.00%> (ø)
...rter/internal/otlp/traces/SpanStatusMarshaler.java 100.00% <0.00%> (ø)
...otlp/logs/InstrumentationLibraryLogsMarshaler.java 100.00% <0.00%> (ø)
...ava/io/opentelemetry/sdk/internal/RateLimiter.java 100.00% <0.00%> (+5.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19b0fec...ecc661c. Read the comment docs.

Copy link
Contributor

@jkwatson jkwatson left a comment

Choose a reason for hiding this comment

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

👍🏽 given the experimental namespace

@trask
Copy link
Member

trask commented Feb 11, 2022

I like this, it's a lot easier for users to know attribute names they want to disable, compared to knowing the class name of the resource provider that they want to disable


Attributes filteredAttributes =
resource.getAttributes().toBuilder()
.removeIf(attributeKey -> disabledKeys.contains(attributeKey.getKey()))
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI filed a couple of issues which would improve this code

#4175 #4176

|------------------------------------------|------------------------------------------|------------------------------------------------------------------------------------------------------------|
| otel.resource.attributes | OTEL_RESOURCE_ATTRIBUTES | Specify resource attributes in the following format: key1=val1,key2=val2,key3=val3 |
| otel.service.name | OTEL_SERVICE_NAME | Specify logical service name. Takes precedence over `service.name` defined with `otel.resource.attributes` |
| otel.experimental.disabled.resource.keys | OTEL_EXPERIMENTAL_DISABLED_RESOURCE_KEYS | Specify resource attribute keys that are filtered. |
Copy link
Contributor

Choose a reason for hiding this comment

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

otel.experimental.resource.disabled_keys

Copy link
Member Author

Choose a reason for hiding this comment

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

How about otel.experimental.resource.disabled-keys for consistency with otel.java.disabled.resource-providers?

Reminder that the - and _ only matter for the readme because they're replaced by . when reading in from env / system props.

@jack-berg jack-berg merged commit 9456aee into open-telemetry:main Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants