Skip to content
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

Add instrumentation of SQS. #1210

Merged
merged 8 commits into from
Sep 24, 2020

Conversation

anuraaga
Copy link
Contributor

No description provided.

@anuraaga
Copy link
Contributor Author

Sent this as a draft in advance to refer to from spec issue, no need to even start reviewing in detail yet (unless you're interested :) )

@anuraaga anuraaga marked this pull request as ready for review September 18, 2020 07:12
@anuraaga
Copy link
Contributor Author

Finished it up with my understanding of the current spec. I expect some changes in the spec as working through scenarios in open-telemetry/semantic-conventions#652 but I think this is a pretty good first version.

@@ -21,6 +21,7 @@ muzzle {
group = "com.amazonaws"
module = "aws-lambda-java-core"
versions = "[1.0.0,)"
extraDependency('com.amazonaws:aws-lambda-java-events:2.2.1')
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 wonder if I should instead of a separate instrumentation module, aws-lambda-sqs-2.2.1 due to this extra, optional dependency. Not quite sure where we want to set boundaries for highly related instrumentation like this case, let me know any thoughts.

Copy link
Member

Choose a reason for hiding this comment

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

I think it makes sense here to add muzzle requirement on aws-lambda-java-events also, e.g.

  pass {
    group = "com.amazonaws"
    module = "aws-lambda-java-events"
    versions = "[2.2.1,)"
    extraDependency('com.amazonaws:aws-lambda-java-core:1.0.0')
  }

Anuraag Agrawal added 2 commits September 23, 2020 15:52
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

nice, I'm glad that worked out to require user to configure the xray propagator, makes this instrumentation feel a lot cleaner

…etry/instrumentation/auto/awslambda/v1_0/AbstractAwsLambdaInstrumentation.java

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
@anuraaga anuraaga merged commit fc47fd5 into open-telemetry:master Sep 24, 2020
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.

None yet

3 participants