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
Lambda instrumentation enhancements #1390
Lambda instrumentation enhancements #1390
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1390 +/- ##
=======================================
- Coverage 69.3% 69.0% -0.3%
=======================================
Files 127 127
Lines 5496 5498 +2
=======================================
- Hits 3810 3798 -12
- Misses 1538 1552 +14
Partials 148 148
|
type MyEvent struct { | ||
Name string `json:"name"` | ||
} | ||
|
||
func HandleRequest(ctx context.Context, name MyEvent) (string, error) { | ||
return fmt.Sprintf("Hello %s!", name.Name ), nil | ||
func HandleRequest(ctx context.Context) (error) { | ||
fmt.Println("Hello World!" ) | ||
return nil | ||
} | ||
|
||
func main() { | ||
lambda.Start(HandleRequest) | ||
ctx := context.Background() | ||
lambda.Start(otellambda.InstrumentHandler(HandleRequest(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.
I don't think this change is correct. The previous example was correct and this change makes it likely to not even compile since it reduces to:
lambda.Start(otellambda.InstrumentHandler(nil))
Also, the prior state was trying to illustrate what a starting point for an uninstrumented function would look like.
} | ||
``` | ||
|
||
Now use the provided wrapper to instrument your basic Lambda function: | ||
Now configure the instrumentation with the provided options to export traces to AWS X-Ray via [the OpenTelemetry Collector](https://github.com/open-telemetry/opentelemetry-collector) running as a Lambda Extension. Instructions for running the OTel Collector as a Lambda Extension can be found in the [AWS OpenTelemetry Documentation](https://aws-otel.github.io/docs/getting-started/lambda). |
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.
Do not conflate the otellambda
instrumentation and the xrayconfig
setup packages. This is documentation for otellambda
and should not need to mention X-Ray at all.
@@ -60,45 +87,6 @@ func main() { | |||
| `WithEventToCarrier` | `func(eventJSON []byte) propagation.TextMapCarrier{}` | Function for providing custom logic to support retrieving trace header from different event types that are handled by AWS Lambda (e.g., SQS, CloudWatch, Kinesis, API Gateway) and returning them in a `propagation.TextMapCarrier` which a Propagator can use to extract the trace header into the context. | Function which returns an empty `TextMapCarrier` - new spans will be part of a new Trace and have no parent past Lambda instrumentation span | |||
| `WithPropagator` | `propagation.Propagator` | The `Propagator` the instrumentation will use to extract trace information into the context. | `otel.GetTextMapPropagator()` | | |||
|
|||
### Usage With Options Example |
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.
Restore this section, it is a useful generic example of how to use the options this package provides.
type asyncSafeFlusher struct { | ||
flusher Flusher | ||
} | ||
|
||
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.flusher.ForceFlush(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.
I have just landed open-telemetry/opentelemetry-go#2335 which will enable us to remove this entirely and simply call ForceFlush
on the provided TracerProvider
. The entire Flusher
type and its implementations and configuration options can be removed.
@@ -11,52 +11,14 @@ This module provides recommended configuration options for [`AWS Lambda Instrume | |||
go get -u go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-lambda-go/otellambda/xrayconfig | |||
``` | |||
|
|||
## Usage |
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.
Restore this section. It is distinct from the otellambda
documentation.
// PrepareTracerProvider returns a TracerProvider configured with exporter, | ||
// id generator and lambda resource detector to send trace data to AWS X-Ray via Collector | ||
func PrepareTracerProvider(ctx context.Context) (*sdktrace.TracerProvider, error) { |
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.
// PrepareTracerProvider returns a TracerProvider configured with exporter, | |
// id generator and lambda resource detector to send trace data to AWS X-Ray via Collector | |
func PrepareTracerProvider(ctx context.Context) (*sdktrace.TracerProvider, error) { | |
// NewTracerProvider returns a TracerProvider configured with exporter, | |
// id generator and lambda resource detector to send trace data to AWS X-Ray via Collector | |
func NewTracerProvider(ctx context.Context) (*sdktrace.TracerProvider, error) { |
New*()
is idiiomatic.
// PrepareTracerProvider returns a TracerProvider configured with exporter, | ||
// id generator and lambda resource detector to send trace data to AWS X-Ray via Collector | ||
func PrepareTracerProvider(ctx context.Context) (*sdktrace.TracerProvider, error) { | ||
log.Println("creating trace exporter") |
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.
log.Println("creating trace exporter") |
// tracerProviderAndFlusher is not exported because it should not be used | ||
// without the provided EventToCarrier function and XRay Propagator |
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.
Why should these two options not be used without the other two options?
return otellambda.WithPropagator(xray.Propagator{}) | ||
} | ||
|
||
// AllRecommendedOptions returns a list of all otellambda.Option(s) | ||
// recommended for the otellambda package when using AWS XRay | ||
func AllRecommendedOptions() []otellambda.Option { | ||
options, err := tracerProviderAndFlusher() | ||
func AllRecommendedOptions(tp *sdktrace.TracerProvider) []otellambda.Option { |
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.
func AllRecommendedOptions(tp *sdktrace.TracerProvider) []otellambda.Option { | |
func RecommendedOptions(tp *sdktrace.TracerProvider) []otellambda.Option { |
The All
feels superfluous. Can we drop it?
Closing this PR due to some local git conflicts will open a new one with the changes suggested. |
Fix typo Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
xrayconfig
package