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

Go lambda instrumentation enhancements #1413

Merged
merged 33 commits into from Nov 12, 2021
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
dcc01de
added failure scenario when getting container fails
bhautikpip Feb 5, 2021
1dd54d9
fix test case failure
bhautikpip Feb 5, 2021
03f6fb4
add changelog
bhautikpip Feb 5, 2021
5919be5
Merge branch 'main' into main
bhautikpip Feb 5, 2021
ebd45b4
Merge branch 'main' into main
Aneurysm9 Feb 5, 2021
e1ff7d0
fix ecs resource detector bug
bhautikpip Feb 7, 2021
041d9b8
Merge branch 'main' of https://github.com/bhautikpip/opentelemetry-go…
bhautikpip Feb 7, 2021
e12a4b1
fix struct name as per golint suggestion
bhautikpip Feb 7, 2021
e906d2a
fix merge conflict
bhautikpip Feb 7, 2021
86c04d1
minor changes
bhautikpip Feb 7, 2021
047f5d0
added NewResourceDetector func and interface assertions
bhautikpip Feb 9, 2021
21db8fa
fix golint failure
bhautikpip Feb 9, 2021
5204d27
minor changes to address review comments
bhautikpip Feb 10, 2021
c9c1bca
Merge branch 'main' into main
MrAlias Feb 10, 2021
c956d4b
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
bhautikpip Feb 10, 2021
12c6c74
Merge branch 'main' of https://github.com/bhautikpip/opentelemetry-go…
bhautikpip Feb 10, 2021
e042a6f
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
bhautikpip Jun 3, 2021
1f6b745
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
bhautikpip Jun 15, 2021
ada0137
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
bhautikpip Aug 12, 2021
9c6a430
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
bhautikpip Aug 20, 2021
79d6d15
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
bhautikpip Aug 24, 2021
e21f9a6
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
bhautikpip Nov 3, 2021
2e0f29a
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
bhautikpip Nov 5, 2021
f30b844
lambda go instrumentation enhancements
bhautikpip Nov 8, 2021
7cf0c34
minor changes to address review comments
bhautikpip Nov 9, 2021
2696b35
update go.mod
bhautikpip Nov 9, 2021
37f37da
Revert "update go.mod"
bhautikpip Nov 9, 2021
9ccfa6a
remove depending on SDK package
bhautikpip Nov 9, 2021
acb7ad3
renaming the public API for xrayconfig package
bhautikpip Nov 9, 2021
0a8ad6a
fix README review suggestions
bhautikpip Nov 9, 2021
c92c227
Merge branch 'main' into revise-lambda-enhancements
Aneurysm9 Nov 12, 2021
6eea789
remove logger and minor changes
bhautikpip Nov 12, 2021
284530b
Update instrumentation/github.com/aws/aws-lambda-go/otellambda/xrayco…
Aneurysm9 Nov 12, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -34,7 +34,7 @@ func HandleRequest(ctx context.Context, name MyEvent) (string, error) {
}

func main() {
lambda.Start(otellambda.WrapHandlerFunction(HandleRequest))
lambda.Start(otellambda.InstrumentHandler(HandleRequest))
Aneurysm9 marked this conversation as resolved.
Show resolved Hide resolved
}
```

Expand All @@ -44,19 +44,19 @@ Now configure the instrumentation with the provided options to export traces to
// Add import
import "go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-lambda-go/otellambda/xrayconfig"

// add options to WrapHandlerFunction call
// add options to InstrumentHandler call
func main() {
lambda.Start(otellambda.WrapHandlerFunction(HandleRequest, xrayconfig.AllRecommendedOptions()...))
lambda.Start(otellambda.InstrumentHandler(HandleRequest, xrayconfig.WithRecommendedOptions()...))
}
```
## Recommended AWS Lambda Instrumentation Options

| Instrumentation Option | Recommended Value | Exported As |
| --- | --- | --- |
| `WithTracerProvider` | An `sdktrace.TracerProvider` configured to export in batches to an OTel Collector running locally in Lambda | Not individually exported. Can only be used via `AllRecommendedOptions()`
| `WithFlusher` | An `otellambda.Flusher` which yields before calling ForceFlush on the configured `sdktrace.TracerProvider`. Yielding mitigates data delays caused by asynchronous nature of batching TracerProvider when in Lambda | Not individually exported. Can only be used via `AllRecommendedOptions()`
| `WithEventToCarrier` | Function which reads X-Ray TraceID from Lambda environment and inserts it into a `propagtation.TextMapCarrier` | Individually exported as `EventToCarrier()`, also included in `AllRecommendedOptions()`
| `WithPropagator` | An `xray.propagator` | Individually exported as `EventToCarrier()`, also included in `AllRecommendedOptions()`
| `WithTracerProvider` | An `sdktrace.TracerProvider` configured to export in batches to an OTel Collector running locally in Lambda | Not individually exported. Can only be used via `WithRecommendedOptions()`
| `WithFlusher` | An `otellambda.Flusher` which yields before calling ForceFlush on the configured `sdktrace.TracerProvider`. Yielding mitigates data delays caused by asynchronous nature of batching TracerProvider when in Lambda | Not individually exported. Can only be used via `WithRecommendedOptions()`
| `WithEventToCarrier` | Function which reads X-Ray TraceID from Lambda environment and inserts it into a `propagtation.TextMapCarrier` | Individually exported as `EventToCarrier()`, also included in `WithRecommendedOptions()`
| `WithPropagator` | An `xray.propagator` | Individually exported as `Propagator()`, also included in `WithRecommendedOptions()`
Aneurysm9 marked this conversation as resolved.
Show resolved Hide resolved


## Useful links
Expand Down
Expand Up @@ -18,7 +18,6 @@ import (
"context"
"log"
"os"
"runtime"

lambdadetector "go.opentelemetry.io/contrib/detectors/aws/lambda"
"go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-lambda-go/otellambda"
Expand All @@ -35,72 +34,45 @@ func xrayEventToCarrier([]byte) propagation.TextMapCarrier {
return propagation.HeaderCarrier{"X-Amzn-Trace-Id": []string{xrayTraceID}}
}

type asyncSafeFlusher struct {
tp *sdktrace.TracerProvider
}

func (f asyncSafeFlusher) ForceFlush(ctx context.Context) error {
// yield processor to attempt to ensure all spans have
// been consumed and are ready to be flushed
// - see https://github.com/open-telemetry/opentelemetry-go/issues/2080
// to be removed upon resolution of above issue
runtime.Gosched()

return f.tp.ForceFlush(ctx)
}

// tracerProviderAndFlusher returns a list of otellambda.Option(s) to
// enable using a TracerProvider configured for AWS XRay via a collector
// and an otellambda.Flusher to flush this TracerProvider.
// tracerProviderAndFlusher is not exported because it should not be used
// without the provided EventToCarrier function and XRay Propagator
func tracerProviderAndFlusher() ([]otellambda.Option, error) {
ctx := context.Background()

// Do not need transport security in Lambda because collector
// runs locally in Lambda execution environment
// NewTracerProvider returns a TracerProvider configured with an exporter,
// ID generator, and lambda resource detector to send trace data to AWS X-Ray
// via a Collector instance listening on localhost
func NewTracerProvider(ctx context.Context) (*sdktrace.TracerProvider, error) {
exp, err := otlptracegrpc.New(ctx, otlptracegrpc.WithInsecure())
if err != nil {
return []otellambda.Option{}, err
errorLogger.Println("failed to create exporter: ", err)
Copy link
Contributor

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.

return nil, err
}

detector := lambdadetector.NewResourceDetector()
res, err := detector.Detect(ctx)
resource, err := detector.Detect(ctx)
if err != nil {
return []otellambda.Option{}, err
errorLogger.Println("failed to detect lambda resources: ", err)
return nil, err
}

tp := sdktrace.NewTracerProvider(
return sdktrace.NewTracerProvider(
sdktrace.WithBatcher(exp),
sdktrace.WithIDGenerator(xray.NewIDGenerator()),
sdktrace.WithResource(res),
)

return []otellambda.Option{otellambda.WithTracerProvider(tp), otellambda.WithFlusher(asyncSafeFlusher{tp: tp})}, nil
sdktrace.WithResource(resource),
), nil
}

// EventToCarrier returns an otellambda.Option to enable
Aneurysm9 marked this conversation as resolved.
Show resolved Hide resolved
// an otellambda.EventToCarrier function which reads the XRay trace
// information from the environment and returns this information in
// a propagation.HeaderCarrier
func EventToCarrier() otellambda.Option {
func WithEventToCarrier() otellambda.Option {
return otellambda.WithEventToCarrier(xrayEventToCarrier)
}

// Propagator returns an otellambda.Option to enable the xray.Propagator
Aneurysm9 marked this conversation as resolved.
Show resolved Hide resolved
func Propagator() otellambda.Option {

func WithPropagator() otellambda.Option {
return otellambda.WithPropagator(xray.Propagator{})
}

// AllRecommendedOptions returns a list of all otellambda.Option(s)
// WithRecommendedOptions returns a list of all otellambda.Option(s)
// recommended for the otellambda package when using AWS XRay
func AllRecommendedOptions() []otellambda.Option {
options, err := tracerProviderAndFlusher()
if err != nil {
// should we fail to create the TracerProvider, do not alter otellambda's default configuration
errorLogger.Println("failed to create recommended configuration: ", err)
return []otellambda.Option{}
}
return append(options, []otellambda.Option{EventToCarrier(), Propagator()}...)
func WithRecommendedOptions(tp *sdktrace.TracerProvider) []otellambda.Option {
return []otellambda.Option{WithEventToCarrier(), WithPropagator(), otellambda.WithTracerProvider(tp), otellambda.WithFlusher(tp)}
}
Expand Up @@ -154,6 +154,9 @@ func assertSpanEqualsIgnoreTimeAndSpanID(t *testing.T, expected *v1trace.Resourc
func TestWrapEndToEnd(t *testing.T) {
setEnvVars()

ctx := context.Background()
tp, _ := NewTracerProvider(ctx)
Copy link
Contributor

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.


customerHandler := func() (string, error) {
return "hello world", nil
}
Expand All @@ -163,7 +166,7 @@ func TestWrapEndToEnd(t *testing.T) {
}()
<-time.After(5 * time.Millisecond)

wrapped := otellambda.InstrumentHandler(customerHandler, AllRecommendedOptions()...)
wrapped := otellambda.InstrumentHandler(customerHandler, WithRecommendedOptions(tp)...)
wrappedCallable := reflect.ValueOf(wrapped)
resp := wrappedCallable.Call([]reflect.Value{reflect.ValueOf(mockContext)})
assert.Len(t, resp, 2)
Expand Down