Skip to content

Conversation

@liustve
Copy link
Contributor

@liustve liustve commented Feb 13, 2025

Issue #, if available:
Adding SigV4 Authentication extension for Exporting traces to OTLP CloudWatch endpoint without needing to explictily install the collector.

Description of changes:
Added a new class that extends upstream's OTLP http span exporter. Overrides the export method so that if the endpoint is CW, we add an extra step of injecting SigV4 authentication to the headers.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@liustve liustve requested a review from a team as a code owner February 13, 2025 23:24
Copy link
Contributor

@majanjua-amzn majanjua-amzn left a comment

Choose a reason for hiding this comment

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

Great work!

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 53.03030% with 31 lines in your changes missing coverage. Please review.

Project coverage is 68.56%. Comparing base (09e6487) to head (51a4478).
Report is 393 commits behind head on main.

Files with missing lines Patch % Lines
...metry/javaagent/providers/OtlpAwsSpanExporter.java 63.63% 19 Missing and 1 partial ⚠️
...iders/AwsApplicationSignalsCustomizerProvider.java 0.00% 10 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@              Coverage Diff              @@
##               main    #1019       +/-   ##
=============================================
- Coverage     85.71%   68.56%   -17.15%     
- Complexity       19      328      +309     
=============================================
  Files             3       27       +24     
  Lines            49     1244     +1195     
  Branches          5      170      +165     
=============================================
+ Hits             42      853      +811     
- Misses            3      334      +331     
- Partials          4       57       +53     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@liustve liustve merged commit 83e5ade into aws-observability:main Feb 20, 2025
4 checks passed
// For Udp emitter
compileOnly("io.opentelemetry:opentelemetry-exporter-otlp-common")
// For HTTP SigV4 emitter
implementation("software.amazon.awssdk:auth:2.30.14")
Copy link
Member

Choose a reason for hiding this comment

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

why use fixed versions of http auth. Will it cause conflicts when customers use a different version in their application?

implementation("com.amazonaws:aws-java-sdk-core:1.12.773")
// Export configuration
compileOnly("io.opentelemetry:opentelemetry-exporter-otlp")
implementation("io.opentelemetry:opentelemetry-exporter-otlp")
Copy link
Member

Choose a reason for hiding this comment

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

Why do we change it to implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it from compileOnly to implementation is because the http exporter is now a required dependency when I began executing the auto instrumentation at runtime. Otherwise I believe I was getting a ClassNotFoundException everytime I ran the exporter

Copy link
Member

Choose a reason for hiding this comment

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

We changed it to compileOnly due to a regression it introduced, see #651. You need to figure out a way to avoid the same issue before submit the change.

liustve added a commit to liustve/aws-otel-java-instrumentation that referenced this pull request Mar 10, 2025
bjrara pushed a commit that referenced this pull request Mar 11, 2025
*Description of changes:*
This reverts SigV4 Exporter added in PR #1019 as it is unstable and is
blocking the current Java V2 release


By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
XinRanZhAWS pushed a commit that referenced this pull request Mar 11, 2025
Issue #, if available:
Adding SigV4 Authentication extension for Exporting traces to OTLP
CloudWatch endpoint without needing to explictily install the collector.

Description of changes:
Added a new class that extends upstream's OTLP http span exporter.
Overrides the export method so that if the endpoint is CW, we add an
extra step of injecting SigV4 authentication to the headers.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

---------

Co-authored-by: Mahad Janjua <[email protected]>
XinRanZhAWS pushed a commit that referenced this pull request Mar 11, 2025
*Description of changes:*
This reverts SigV4 Exporter added in PR #1019 as it is unstable and is
blocking the current Java V2 release


By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
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