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

Extract APIGatewayProxyRequestEvent headers for context propagation #12440

Merged
merged 3 commits into from
Oct 17, 2024

Conversation

cleverchuk
Copy link
Contributor

@cleverchuk cleverchuk commented Oct 14, 2024

Extract APIGatewayProxyRequestEvent headers for context propagation. This change is only added to aws-lambda-events-2.2 package because the aws-lambda-core instrumentation only applies to Lambdas that use custom events.

@cleverchuk cleverchuk requested a review from a team as a code owner October 14, 2024 12:59
@cleverchuk cleverchuk changed the title (#12422) Fixes (#12422) Oct 14, 2024
@cleverchuk cleverchuk changed the title Fixes (#12422) Fixes #12422 Oct 14, 2024
- extract `APIGatewayProxyRequestEvent` headers for context propagation
@cleverchuk
Copy link
Contributor Author

@tylerbenson @laurit

@laurit
Copy link
Contributor

laurit commented Oct 15, 2024

@cleverchuk could you update the PR title, we use PR titles in changelog.

@cleverchuk cleverchuk changed the title Fixes #12422 Extract APIGatewayProxyRequestEvent headers for context propagation Oct 15, 2024
Copy link
Member

@tylerbenson tylerbenson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An additional unit test might be nice. Not sure how much effort that would be though.

@laurit
Copy link
Contributor

laurit commented Oct 16, 2024

@cleverchuk could you see if it is possible to build a test similar to

that would test the functionality that you have implemented.

@cleverchuk
Copy link
Contributor Author

@laurit @tylerbenson added a test. I don't understand entirely how the testing works though. However the test fails without the change, which I think is a good sign. This check kept failing so I removed it because I don't know how to get it to work.

equalTo(HttpAttributes.HTTP_RESPONSE_STATUS_CODE, 201L)

@laurit
Copy link
Contributor

laurit commented Oct 17, 2024

@laurit @tylerbenson added a test. I don't understand entirely how the testing works though. However the test fails without the change, which I think is a good sign. This check kept failing so I removed it because I don't know how to get it to work.

equalTo(HttpAttributes.HTTP_RESPONSE_STATUS_CODE, 201L)

Thanks, I added the http.response.status_code attribute.

@trask trask merged commit c5d6b4a into open-telemetry:main Oct 17, 2024
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants