-
Notifications
You must be signed in to change notification settings - Fork 550
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
@opentelemetry/instrumentation-aws-lambda should read API gateway parameters by default #999
Comments
This issue 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. |
still is a problem/footgun. |
Tagging @vasireddy99 and @bryan-aguilar to triage this for investigation. |
This is not stale. Tagging component owners @willarmiros @NathanielRN |
This issue 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. |
Still not stale. Happy to contribute a fix myself, just unsure where I'd access the kind of information that we should be attaching to the created span. |
Hi @cartermp, It seems these additional attributes for APIGW-invoked Lambdas would have to be added in the Lambda instrumentation around here: Line 166 in 4a9442c
You would need to parse the |
In case the lambda is triggered from the Gateway API, should the span status be set to Status.Error if the lambda is returning a statusCode != 2XX ? (See https://docs.aws.amazon.com/apigateway/latest/developerguide/handle-errors-in-lambda-integration.html) We have Sentry that's catching our thrown exceptions and returning a proper 500, but in that case, the lambda itself doesn't really fail, hence the span is not errored, while we receive a Sentry alert. Just wondering |
This issue 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. |
Hi @NPellet - sorry for the delayed response I must have missed this. Yeah I think based on those docs we are only handling the "standard error" case, but not the "custom error" case. IMO if the customer is creating a custom error as described in those docs and we can read the HTTP Status code of that custom error, then it is clear that the customer intent is to mark that request as failed and we should update the span status to Error. Also commenting to remove stale. |
@dyladan can we make this a never-stale? |
This issue 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. |
We are definitely interested in this feature, especially since API Gateway v2 (HTTP API) isn't supported by AWS X-ray and many other third party providers. Having that as the root span would be immensely helpful. |
Is your feature request related to a problem? Please describe
Invokes create a root span, but populate only details about the lambda itself (e.g. /usr/bin/node, lambda runtime version, function name) rather than about the request being performed. When the lambda is being called directly from API gateway, these details should automatically be populated, rather than relying upon telemetry from API gateway to have been also plumbed up and amazon trace header used for correlation.
Describe the solution you'd like to see
the following
event.input
(but notevent.body
, to avoid PII leaks) values should be automatically extracted and put into attributes (essentially as a default requestHook); example from https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-create-api-as-simple-proxy-for-lambda.htmlDescribe alternatives you've considered
These attributes could be left on an API gateway span, except there's no easy way to produce OTel formatted spans out of API gateway logs, and even then, API gateway only speaks amazon trace header, not w3c tracecontext, so you'd need to use amazon propagator.
Additional context
tagging component owners @willarmiros @NathanielRN and tagging Honeycomb PM @cartermp
tagging users who encountered this problem and showed me: @longility @nikordaris @meleksomai
The text was updated successfully, but these errors were encountered: