-
Notifications
You must be signed in to change notification settings - Fork 579
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 support for end-to-end tracing on AWS Lambda #882
Conversation
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
079d41d
to
9cb55d6
Compare
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both modules require README.md
describing their use and the instrumentation should have an example. See other instrumentation for samples.
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/README.md
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/config.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/wrapLambdaHandler.go
Outdated
Show resolved
Hide resolved
My remaining comments are aesthetic/conventional and not structural.
Codecov Report
@@ Coverage Diff @@
## main #882 +/- ##
=======================================
+ Coverage 75.7% 76.6% +0.9%
=======================================
Files 70 75 +5
Lines 4467 4685 +218
=======================================
+ Hits 3382 3592 +210
- Misses 943 946 +3
- Partials 142 147 +5
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the AWS X-Ray team, generally lgtm, just a couple questions!
instrumentation/github.com/aws/aws-lambda-go/otellambda/wrapLambdaHandler.go
Show resolved
Hide resolved
@bhautikpip would you mind taking a quick look from AWS side as well? @open-telemetry/go-approvers would anyone else also be able to give this a look and get it merged? Many of the files changed are examples/documentation, and we would love to release this to OTel Go users! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM :)
Closing in favor of #983, @bhautikpip will own related PRs going forward |
See for example use case: https://github.com/garrettwegan/opentelemetry-lambda/blob/main/go/sample-apps/function/function.go