-
Notifications
You must be signed in to change notification settings - Fork 595
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
Go lambda instrumentation enhancements #1413
Go lambda instrumentation enhancements #1413
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1413 +/- ##
=======================================
- Coverage 69.3% 69.2% -0.1%
=======================================
Files 127 127
Lines 5496 5475 -21
=======================================
- Hits 3810 3794 -16
+ Misses 1538 1534 -4
+ Partials 148 147 -1
|
instrumentation/github.com/aws/aws-lambda-go/otellambda/config.go
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/xrayconfig/README.md
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/xrayconfig/README.md
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/xrayconfig/xrayconfig.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/xrayconfig/xrayconfig.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
This reverts commit 2696b35.
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.
Instrumentation must not depend on an SDK.
instrumentation/github.com/aws/aws-lambda-go/otellambda/xrayconfig/README.md
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/xrayconfig/xrayconfig.go
Outdated
Show resolved
Hide resolved
instrumentation/github.com/aws/aws-lambda-go/otellambda/xrayconfig/xrayconfig.go
Outdated
Show resolved
Hide resolved
The failing unit test is a result of no longer trying to yield to the go runtime before invoking |
@open-telemetry/proto-go-approvers @dashpole can you please provide a second review so that this PR can be merged. We have a downstream dependency on this. Many thanks in advance. |
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.
Just nits. Looks good overall
exp, err := otlptracegrpc.New(ctx, otlptracegrpc.WithInsecure()) | ||
if err != nil { | ||
return []otellambda.Option{}, err | ||
errorLogger.Println("failed to create exporter: ", err) |
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.
We probably don't need to log here. Since we return the error, we expect the caller to handle it (and log it if they would like). Same applies below and elsewhere.
@@ -154,6 +154,9 @@ func assertSpanEqualsIgnoreTimeAndSpanID(t *testing.T, expected *v1trace.Resourc | |||
func TestWrapEndToEnd(t *testing.T) { | |||
setEnvVars() | |||
|
|||
ctx := context.Background() | |||
tp, _ := NewTracerProvider(ctx) |
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.
Probably best to check that the err is non-nil here.
instrumentation/github.com/aws/aws-lambda-go/otellambda/xrayconfig/xrayconfig_test.go
Outdated
Show resolved
Hide resolved
…nfig/xrayconfig_test.go
* added failure scenario when getting container fails * fix test case failure * add changelog * fix ecs resource detector bug * fix struct name as per golint suggestion * minor changes * added NewResourceDetector func and interface assertions * fix golint failure * minor changes to address review comments * lambda go instrumentation enhancements * minor changes to address review comments * update go.mod * Revert "update go.mod" This reverts commit 2696b35. * remove depending on SDK package * renaming the public API for xrayconfig package * fix README review suggestions * remove logger and minor changes * Update instrumentation/github.com/aws/aws-lambda-go/otellambda/xrayconfig/xrayconfig_test.go Co-authored-by: Anthony Mirabella <a9@aneurysm9.com> Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
xrayconfig
package more useful.