-
Notifications
You must be signed in to change notification settings - Fork 941
Experimental support for disabling resource keys #4169
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
Experimental support for disabling resource keys #4169
Conversation
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
jkwatson
left a comment
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.
👍🏽 given the experimental namespace
|
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())) |
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.
| |------------------------------------------|------------------------------------------|------------------------------------------------------------------------------------------------------------| | ||
| | 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. | |
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.
otel.experimental.resource.disabled_keys
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.
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.
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_lineresource attributes, which are lengthy, and may contain sensitive information.Its also possible to do accomplish something similar by using
otel.java.disabled.resource-providersto disable the entireProcessResourceProvider, 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_lineinProcessResourceProviderby 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.