add EC2 resource detector to default config #32
Conversation
config.yaml
Outdated
| @@ -20,7 +25,7 @@ service: | |||
| pipelines: | |||
| traces: | |||
| receivers: [otlp] | |||
| processors: [batch, queued_retry] | |||
| processors: [batch, queued_retry, resourcedetection/ec2] | |||
There was a problem hiding this comment.
Pls follow the OTel recommended processor sequence. The order of processors matters when processing data
https://github.com/open-telemetry/opentelemetry-collector/tree/master/processor#traces
Traces
- memory_limiter
- any sampling processors
- batch
- any other processors
- queued_retry
There was a problem hiding this comment.
Will move, thanks for the callout.
|
Do we plan to include this processor in the default config for beta launch? |
| resourcedetection: | ||
| resourcedetection/ec2: | ||
| detectors: [env, ec2] | ||
| timeout: 2s | ||
| override: false |
There was a problem hiding this comment.
Why do we define two processors here if we only need one?
There was a problem hiding this comment.
sorry for late catch, I also realize resourcedetection processor is located in Contrib project which we don't have those processor enabled by default. we have to update code to enable it in our collector.
There was a problem hiding this comment.
Why do we define two processors here if we only need one?
I guess I was including env because it was best practice to always have it included to allow the customer to provide arbitrary resource metadata about their environment, but I can separate it into a different resourcedetection block or remove it altogether if you'd like.
Regarding the contrib project, so we have to add the collector-contrib to our go.mod? Is there anything else preventing us from adding these resource detectors?
There was a problem hiding this comment.
I mean we can have both env and ec2 in one resourcedetection setup, I see you defined 2 like below, can we remove the top one? Do you know what's the test cases we need to perform on this new processor? We may want to add the test cases into our integration test package.
resourcedetection: // this one can be removed
resourcedetection/ec2:
There was a problem hiding this comment.
No, we need to enable it in our own repo here. see ref PR - #31
As discussed offline, we do plan on having resource detectors enabled by default on the AWS Collector Distro |
…t-trace-validator TraceValidator
We would like all AWS resource metadata to be recorded in X-Ray traces by default (e.g. opt-out). This PR adds the EC2 resource detector to the default config we'll vend to customers in our distribution to accomplish just that for EC2 instances. It will record EC2 instance metadata in traces or fail fast if it does not detect that it's in an EC2 environment.