-
Notifications
You must be signed in to change notification settings - Fork 822
opentelemetry-instrumentation-aws-lambda: Fix aws lambda span creation #3966
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
base: main
Are you sure you want to change the base?
opentelemetry-instrumentation-aws-lambda: Fix aws lambda span creation #3966
Conversation
tammy-baylis-swi
left a comment
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.
Thanks for fixes and linking the AWS docs in docstring.
| event_source = lambda_event["Records"][0].get( | ||
| "eventSource" | ||
| ) or lambda_event["Records"][0].get("EventSource") | ||
| if event_source in { |
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.
There are cases where span can be of other kinds:
"The span kind for both types of SQS spans SHOULD be CONSUMER"
https://opentelemetry.io/docs/specs/semconv/faas/aws-lambda/#sqs
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.
@xrmx I saw the documentation for SQS events too and I find it a bit ambiguous, but to me it still appears that the parent invocation span type should be SERVER based on the following statement in the spec:
For the SQS event span, if all the messages in the event have the same event source, the name of the span MUST be process. If there are multiple sources in the batch, the name MUST be multiple_sources process. The parent SHOULD be the SERVER span corresponding to the function invocation.
The key phrase being "The parent SHOULD be the SERVER span corresponding to the function invocation."
From what I can tell, both the Java and JavaScript instrumentation libraries always have the initial Span be the SERVER span corresponding to the function invocation, and create additional child Spans for the SQS events.
Description
Fixes bug where the Lambda invocation
Spanname would be incorrectly set to the name of the Python handler function instead of the function name. Also fixes an issue where the Lambda invocationSpankind would occasionally not be set toSERVERwhich violates the OpenTelemetry FaaS semantic conventions.Fixes #3963
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Unit tests have been updated to reflect changing the invocation
Spanname to the Lambda function name and changing theSpankind to always beSERVER.Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.