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

Feature request: HTTP request/response annotations #1179

Open
2 tasks done
revmischa opened this issue Nov 18, 2022 · 9 comments
Open
2 tasks done

Feature request: HTTP request/response annotations #1179

revmischa opened this issue Nov 18, 2022 · 9 comments
Labels
confirmed The scope is clear, ready for implementation feature-request This item refers to a feature request for an existing or new utility help-wanted We would really appreciate some support from community for this one tracer This item relates to the Tracer Utility

Comments

@revmischa
Copy link

revmischa commented Nov 18, 2022

Use case

I have a nextjs CDK construct with a simple APIGW lambda wrapper around nextjs. I want to annotate my traces with HTTP request and response metadata, e.g. "URL" and "method" and "status code"

I would like to use it here: https://github.com/jetbridge/cdk-nextjs/blob/main/assets/lambda/NextJsHandler.ts#L45

I'd like to see the status code, path, method, etc in xray:
image

Solution/User Experience

Perhaps something like traceRequestResponseCycle in https://docs.aws.amazon.com/xray-sdk-for-nodejs/latest/reference/AWSXRay.html

Alternative solutions

Discussed on discord: https://discord.com/channels/1006478942305263677/1006527385409179678/1042309163818168380

Acknowledgment

@revmischa revmischa added the triage This item has not been triaged by a maintainer, please wait label Nov 18, 2022
@dreamorosi dreamorosi added tracer This item relates to the Tracer Utility feature-request This item refers to a feature request for an existing or new utility discussing The issue needs to be discussed, elaborated, or refined need-more-information Requires more information before making any calls and removed triage This item has not been triaged by a maintainer, please wait labels Nov 18, 2022
@dreamorosi
Copy link
Contributor

Hi @revmischa thank you for taking the time to open a feature request.

As I have briefly mentioned on Discord: at the moment we don't use any of the middleware features from aws-xray-sdk so I'm unsure on how it will behave with Tracer. With that said, a first cursory look at traceRequestResponseCycle's implementation suggests that the utility provides some syntactic sugar around segment manipulation/annotation, which bodes well for Powertools compatibility.

Before considering the feature and putting it into the backlog, I would like some investigation to take place to make a small PoC and understand the impact and compatibility of this feature with Powertools.

Like with all features, contributions are more than welcome, and in case this is left to the maintainers, having 👍 on the feature request will help increasing its priority in our backlog.

If anyone is interested in picking up this issue, please leave a comment below to express your interest and agree on next steps. The maintainers will be happy to support you with clearing doubts or questions on the scope and codebase.

@dreamorosi dreamorosi added the help-wanted We would really appreciate some support from community for this one label Nov 18, 2022
@revmischa
Copy link
Author

For my purposes a simpler version of this would be to let me manually set the HTTP path, method, status code directly on the segment. But I have no idea how to do this and couldn't find any documentation on how to manually set those attributes.

@kennethwkz-mm
Copy link

this is https://www.npmjs.com/package/aws-xray-sdk-express feature, this is very useful when we are tracing some ip address or api URL in a easy way

@dreamorosi
Copy link
Contributor

FYI @revmischa, I have been looking at the aws-sdk-for-nodejs implementation to see if I could find a way to make it work for Tracer without changes but didn't have much luck.

I'll keep looking, also using the example that was just posted above.

@github-actions

This comment was marked as outdated.

@github-actions github-actions bot added the pending-close-response-required This issue will be closed soon unless the discussion moves forward label Feb 28, 2023
@revmischa
Copy link
Author

Please keep open

@dreamorosi dreamorosi added need-customer-feedback Requires more customers feedback before making or revisiting a decision and removed pending-close-response-required This issue will be closed soon unless the discussion moves forward need-more-information Requires more information before making any calls labels Feb 28, 2023
@github-actions

This comment was marked as off-topic.

@github-actions github-actions bot added the pending-close-response-required This issue will be closed soon unless the discussion moves forward label Mar 25, 2023
@dreamorosi dreamorosi added need-more-information Requires more information before making any calls and removed pending-close-response-required This issue will be closed soon unless the discussion moves forward need-customer-feedback Requires more customers feedback before making or revisiting a decision labels Mar 25, 2023
@dreamorosi dreamorosi added this to the Version 2.0 milestone Aug 2, 2023
@dreamorosi dreamorosi removed this from the Version 2.0 milestone Jan 30, 2024
@dreamorosi dreamorosi added confirmed The scope is clear, ready for implementation and removed discussing The issue needs to be discussed, elaborated, or refined need-more-information Requires more information before making any calls labels Jan 28, 2025
@dreamorosi dreamorosi moved this from Ideas to Backlog in Powertools for AWS Lambda (TypeScript) Jan 28, 2025
@dreamorosi
Copy link
Contributor

With the upcoming Event Handler feature incoming, we'll need a way to add these annotations to segments generated by Tracer.

I've been looking at the implementation more closely and I think that we should implement a version of this method so that the http field of the X-Ray segment data is populated as described in the original post.

I'm adding this to the backlog so we can work on it at some point in the coming months.

The issue is open for contributions. If you’re interested, please leave a comment before starting a PR. Let’s discuss and agree on a design for the new feature before implementing it. I don’t want to expose the method as-is from the X-Ray SDK.

@VatsalGoel3
Copy link
Contributor

Hi @dreamorosi,

I’d love to take on this feature request and implement HTTP metadata annotation for X-Ray segments in the Tracer utility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed The scope is clear, ready for implementation feature-request This item refers to a feature request for an existing or new utility help-wanted We would really appreciate some support from community for this one tracer This item relates to the Tracer Utility
Projects
Development

No branches or pull requests

4 participants