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

add RequestData integration #5674

Merged
merged 1 commit into from
Nov 3, 2022

Conversation

lobsterkatie
Copy link
Member

@lobsterkatie lobsterkatie commented Oct 21, 2022

This adds documentation for the new RequestData integration in the node SDK, which adds request data to events. For frameworks where we don't have wrappers in the SDK, and merely tell people how to use sentry in the docs, this also adds storing the request in scope metadata to our boilerplate.

@vercel
Copy link

vercel bot commented Oct 21, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
sentry-docs ✅ Ready (Inspect) Visit Preview Oct 28, 2022 at 4:20AM (UTC)

Copy link
Contributor

@lizokm lizokm left a comment

Choose a reason for hiding this comment

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

Made a small suggestion, otherwise, this looks great :)


_Import name: `Sentry.Integrations.RequestData`_

This integration adds data from the current incoming request (if any) to events.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This integration adds data from the current incoming request (if any) to events.
This integration adds data from an incoming request, (if one is in progress), to events.

Would like to simplify to just say "incoming request" unless it's commonly expected to call it "current incoming request" :)

Copy link
Member Author

@lobsterkatie lobsterkatie Oct 28, 2022

Choose a reason for hiding this comment

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

Okay, tried again. LMK what you think.

UPDATE: Given that this feature has now been released, I'm going to go ahead and merge this, and we can always come back to the specific wording later.

Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

@lobsterkatie lobsterkatie merged commit 431dee6 into master Nov 3, 2022
@lobsterkatie lobsterkatie deleted the kmclb-document-requestdata-integration branch November 3, 2022 06:23
@github-actions github-actions bot locked and limited conversation to collaborators Nov 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants