cdk-eks-blueprints icon indicating copy to clipboard operation
cdk-eks-blueprints copied to clipboard

Enable awsproxy extension in XRay ADOT Addon

Open RobKenis opened this issue 3 years ago • 4 comments

When using the OpenTelemetry collector with the javaagent, in combination with Sampling Rules in Cloudwatch or Xray, configuration is as follows: https://aws-otel.github.io/docs/getting-started/remote-sampling

By default, the proxy will run on port 2000.

Issue #, if available:

Description of changes:

  • Enable awsproxy in adot collector. When using CloudWatch Sampling Rules, the awsproxy needs to be enabled. It handles authentication to AWS, so the containers using the javaagent don't have to.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

RobKenis avatar Nov 24 '22 08:11 RobKenis

@elamaran11 could you please review this PR when you get a chance? I am hesitant to always add the awsproxy extension, it could be a config option, but that will require some rework.

shapirov103 avatar Nov 30 '22 21:11 shapirov103

@RobKenis @shapirov103 Im not supportive to add this feature this at this point. This is not officially supported in ADOT XRay AWS documentation. Please check the reference to collector-config-xray.yaml and this does not support awsproxy today. I would recommend working on this with aws-otel-community and once they are able to approve and merge this change, we can work towards this change. Also even in future this should be an option given to user to add and should not be a mandate to use awsproxy, this change is looking like it has to be a mandate. At this point i dont approve to merge this change as this might break the current functioning of the addon.

elamaran11 avatar Dec 05 '22 00:12 elamaran11

@elamaran11 I understand that this option should not be forced upon users, so making it optional is the best way to go. However, as per this documentation https://aws-otel.github.io/docs/getting-started/remote-sampling we can enable the proxy by using the config in OpenTelemetryCollector. Our current solution is to install the addon and overwrite the OpenTelemetryCollector manifest after the installation is done. So the documentation supports this feature, but the blueprint doesn't. Can't we make the addon more flexible to adding extensions? Also, is https://aws-otel.github.io/docs/getting-started/remote-sampling official AWS documentation?

RobKenis avatar Dec 05 '22 09:12 RobKenis

@RobKenis As discussed above, This is not officially supported in ADOT XRay AWS documentation. Please check the reference to collector-config-xray.yaml and this does not support awsproxy today. I would recommend working on this with aws-otel-community and once they are able to approve and merge this change, we can work towards this change. Once they approve this, we can find a way to allow external file for configuration vs modifying the existing one.

elamaran11 avatar Dec 05 '22 16:12 elamaran11

@elamaran11 can you confirm if awsproxy extension can be added as a separate extensions, e.g. a private add-on that maybe depends on xray ADOT add-on? If so, I suggest posting that approach here and closing the PR.

shapirov103 avatar Feb 06 '23 16:02 shapirov103

@RobKenis If Remote sampling is a feature you are looking for, I would recommend submitting a new PR with a new Addon. The new add-on can have an approach to overwrite the OpenTelemetryCollector custom resource after the XRay Adot addon is installed. We will take a look in to your PR, Run some tests and approve the same.

elamaran11 avatar Feb 06 '23 17:02 elamaran11

Closing this Issue.

elamaran11 avatar Feb 06 '23 17:02 elamaran11