Closed
Description
There have gotten to be enough moving parts and dependencies involved in the RequestData
integration work that I figured it's worth writing them all down.
Background
- The underlying request-data-adding functions got moved from
@sentry/node
to@sentry/utils
in order to be able to use them in the_error
helper in the nextjs SDK, which runs in both browser and node and therefore must be platform agnostic. (The function is only actually called in node, but has to be imported from a neutral source.) As a result of that change, node-specific dependencies needed for those functions were forced to be provided via dependency injection, by wrapper functions in the node SDK. - The new data-fetcher wrappers in the nextjs SDK needed a new way to use the functions. (We currently use them in scope-specific event processors, but the events those wrappers create pass through a number of different scopes, so that won't work for them.) To solve this, and to bring continuity to our use of the functions, we decided to create an integration.
- There was a recent DACI wherein we decided that we eventually want to add the same data to events tracking fetch requests in the browser.
Problems/Constraints
- The functions themselves are part of the Great Transaction Mess (see Clean up code relating to scope/event
transaction
value #5718 and Problematic API:Scope.setTransactionName()
#5660). - The dependency injection is kind of gross and hard to follow.
- It's unclear how much overlap there'll be between the node and browser versions of the integration, and the browser version isn't yet roadmapped (and therefore we don't know how soon it will happen).
- Because of the above two points, we decided to undo the move from node to utils, because once the integration is in use, the _error helper doesn't need to use the functions directly. Until it is, though, the helper still needs the function to live in the utils package.
Order of Operations
Immediate term:
- Create polymorphic request type so request can be stored on transaction (feat(types): Add
PolymorphicRequest
type #5744) - Move the functions back to the node package, but leave them also in utils for now (ref(node): Move request data functions back to
@sentry/node
#5759) - Use integration for nextjs transactions (ref(nextjs): Use integration to add request data to transaction events #5703)
- Use integration for nextjs errors (ref(nextjs): Use
RequestData
integration for errors #5729) - Make integration default for node-based SDKs (ref(node): Make
RequestData
integration default #5980) - Clean up integration (ref(node): Small
RequestData
integration tweaks #5979) - Use integration for express transactions and errors (ref(node): Use
RequestData
integration in express handlers #5990) - Use integration for GCP wrapper (ref(serverless): Use
RequestData
integration in GCP wrapper #5991) - Use integration for remix (feat(remix): Enable
RequestData
integration for server-side requests #6007) - Document integration and options to control what data is attached (add
RequestData
integration sentry-docs#5674) - Mark future removals as deprecated and label all related TODOs (see v8 section below)
Longer term:
- Clean up transaction mess
- Move extra logic from integration to revamped request-data-adding functions
- Once browser version of integration is needed, figure out if we want to factor out common functionality or just make a separate integration
v8 Removals/Deprecations/Breaking Changes:
(WIP - may not be an exhaustive list)
- Remove
requestDataDeprecated.ts
(parseRequest
and friends) - Remove tests testing
parseRequest
/addRequestDataToEvent
compatibility - Remove utils copy of request data functions
- Make express request handler no longer take request data options (users can use integration options)
- Make CGP wrapper no longer take request data options (users can use integration options)
- Remove helper functions converting between
parseRequest
,addRequestDataToEvent
, andRequestData
integration options
Metadata
Metadata
Assignees
Labels
No labels