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

feat(opentelemetry-instrumentation-aws-lambda): Handle event when coming from an API Gateway, see #999 #1290

Closed
wants to merge 8 commits into from

Conversation

NPellet
Copy link
Contributor

@NPellet NPellet commented Nov 17, 2022

Which problem is this PR solving?

Enhancement proposal as discussed in #999

Short description of the changes

When we determine that the Lambda has been triggered from the API Gateway, we create a new span (wrapping the lambda span itself), giving it the name of the path

Given https://docs.aws.amazon.com/apigateway/latest/developerguide/set-up-lambda-proxy-integrations.html (See "Output format of a Lambda function for proxy integration") we analyze the statusCode replied by the lambda, and fail the span if that error is < 200 or >= 400.

Reacting to thrown exceptions doesn't fail the span, but the exception gets recorded.

I think creating a wrapping span makes more sense than annotating the existing one. It allows a clear separation in the tracing UI between lambdas that are called through SQS, the event bridge, etc. and the ones triggered through the Gateway.

@NPellet NPellet requested a review from a team November 17, 2022 22:33
@github-actions github-actions bot requested a review from willarmiros November 17, 2022 22:33
@NPellet NPellet changed the title Handle event coming an API Gateway, see #999 AWS Lambda - Handle event when coming from an API Gateway, see #999 Nov 17, 2022
@NPellet NPellet changed the title AWS Lambda - Handle event when coming from an API Gateway, see #999 feat(opentelemetry-instrumentation-aws-lambda) - Handle event when coming from an API Gateway, see #999 Nov 17, 2022
@NPellet NPellet changed the title feat(opentelemetry-instrumentation-aws-lambda) - Handle event when coming from an API Gateway, see #999 feat(opentelemetry-instrumentation-aws-lambda): Handle event when coming from an API Gateway, see #999 Nov 17, 2022
@NPellet
Copy link
Contributor Author

NPellet commented Nov 18, 2022

By the way, this is how it would look in Jaeger (juste tested on a Lambda):

image

@NPellet
Copy link
Contributor Author

NPellet commented Nov 19, 2022

I guess this doesn't respect the specification: https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/http/#status

Because I set the status to errored when replying 400 or 500.
But to be honest I don't understand the motivation behind this part of the spec. If the caller is not instrumented then we can raise no alert due to a failed Gateway API request.

@NPellet
Copy link
Contributor Author

NPellet commented Nov 19, 2022

Edited to add the query parameters, the path parameters and the headers as span attributes:

image

@NPellet
Copy link
Contributor Author

NPellet commented Nov 25, 2022

Added configuration to select whether returning error codes should set the span status to failed or not:

Configuration:

 detectApiGateway?: {
    enable: true,
    errorCodes?: Array<RegExp | number>
  }

When using errorCodes: [ 400 ], only the error code 400 will set the span status to error, otherwise it will be set to ok

When using errorCodes: [/^4[0-9]{2}$/], codes 400-499 will set the span status to error, otherwise it will be set to ok

When using errorCodes: undefined, the span status will be left unset

Essentially, any match in the array will set the span status to error

@github-actions
Copy link
Contributor

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Jan 30, 2023
@github-actions
Copy link
Contributor

This PR was closed because it has been stale for 14 days with no activity.

@github-actions github-actions bot closed this Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants