-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
selective attribute appending for resourcedetectionprocessor #8242
Comments
@jrcamp, are you still a code owner of this component? |
BTW, as a workaround one could use resource processor and delete unwanted attributes |
What are the benefits of having this on the resourcedetection processor vs. using a resource processor (or the transform processor in the future)? |
I think reduced memory pressure and some cpu cycles, though I believe the difference is not going to be anywhere substantial. Also, a simpler configuration |
I agree, at least for the detectors I am familiar with the calls that we do to the different cloud provider APIs are not going to change (we either get all atribute values or we get no attribute values), so any performance improvements would be in avoiding to add an attribute and then remove it (which seems like a negligible thing for most users).
That's fair, I personally would favor leaving this functionality to the transform/resource processor but it's true that the configuration is slightly more complex. |
The above mentioned benefits are all valid. One more minor benefit is that a configuration that specifies the desired attributes would not be surprised with new attributes in future releases. |
That's a good point. I think you have convinced me that this is worth having :) |
What would happen if someone added a string in the attributes section that doesn't make sense for the detector type? For example, if the detector is only Also, if the configuration includes multiple detectors, would the attributes section apply to all detectors or should you be able to configure the attribute values for each detector? |
@mx-psi @jpkrohling I'd like to take a shot at this issue as my first contribution. |
You could check this on the
From the example, I would say the original intention is for this to apply to all detectors. |
So in my And if the list includes both a valid and invalid, such as |
I would view the list of attributes as an allowlist. The user is indicating that the will accept only these attributes and that all others should be rejected. Alternately, it could be made more flexible by having inclusion/exclusion lists: processors:
resourcedetection:
detectors: [ec2]
timeout: 2s
override: false
attributes:
include: [host.name, host.id]
exclude: [cloud.provider] The exclusion here would be redundant since inclusion should take precedence and any attributes not explicitly included should be excluded, but I included both for illustration. |
Ok, treating it as an allowlist seems simple enough. I will move forward with that approach. I do like the idea of validating the allowlist to help stop customers from hurting themselves by listing invalid attributes, but maybe thats a future enhancement. |
Should have a PR for this tomorrow. |
@open-telemetry/collector-contrib-approvers I think this issue and be closed. |
Closed by #8547 |
ResourceDetectionProcessor adds several attributes to identify the host machine. There are scenarios where all of the attributes are not desired.
I would like to propose that we add a config setting to allow the user to specify that only a subset of the available attributes be appended.
For example, the AWS EC2 resource detection processor adds the following attributes by default:
My proposal would be to add an "attributes" config setting that allows the user to provide a list of attributes that they would like to have appended. If no "attributes" are specified then we would default to the current behavior of adding all attributes. Continuing with the AWS example, if a user only wanted "host.id" and "host.name" the config would be as follows:
The text was updated successfully, but these errors were encountered: