Skip to content

feat(replay): Allow to configure URLs to capture network bodies/headers #7953

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

Merged
merged 1 commit into from
Apr 26, 2023

Conversation

mydea
Copy link
Member

@mydea mydea commented Apr 25, 2023

This replaces the old experimental options for enhanced replay network capture with new, official configuration:

interface ReplayNetworkOptions {
  /**
   * Capture request/response details for XHR/Fetch requests that match the given URLs.
   * The URLs can be strings or regular expressions.
   * When provided a string, we will match any URL that contains the given string.
   * You can use a Regex to handle exact matches or more complex matching.
   *
   * Only URLs matching these patterns will have bodies & additional headers captured.
   */
  networkDetailAllowUrls: (string | RegExp)[];

  /**
   * If request & response bodies should be captured.
   * Only applies to URLs matched by `networkDetailAllowUrls`.
   * Defaults to true.
   */
  networkCaptureBodies: boolean;

  /**
   * Capture the following request headers, in addition to the default ones.
   * Only applies to URLs matched by `networkDetailAllowUrls`.
   * Any headers defined here will be captured in addition to the default headers.
   */
  networkRequestHeaders: string[];

  /**
   * Capture the following response headers, in addition to the default ones.
   * Only applies to URLs matched by `networkDetailAllowUrls`.
   * Any headers defined here will be captured in addition to the default headers.
   */
  networkResponseHeaders: string[];
}

Note: This brings a small change, in that we also just capture the default headers when the URL matches. So by default, no headers are captured for any request.

Example usage:

Capture bodies & default headers for given URL:

new Replay({
  // This will capture details for any path including this
  // So this will e.g. capture https://sentry.io/api/users
  networkDetailAllowUrls: ['https://sentry.io/api']
});

Capture bodies & default headers for given exact URLs:

new Replay({
  // By providing Regex, you can do exact matching
  networkDetailAllowUrls: [
    /^http:\/\/sentry.io\/api\/users$/, 
    /^http:\/\/sentry.io\/api\/projects$/, 
  ]
});

Capture only headers, including some custom ones:

new Replay({
  networkDetailAllowUrls: ['https://sentry.io/api'],
  networkCaptureBodies: false,
  networkRequestHeaders: ['X-Custom-Header'],
  networkResponseHeaders: ['X-Custom-Header', 'X-Custom-Header-2']
});

Warnings

For any request that does not match the given URL(s), we will add a warning to the request/response meta. For example:

{
  data: {
    method: 'POST',
    statusCode: 200,
    request: {
      headers: {},
      _meta: {
        warnings: ['URL_SKIPPED'],
      },
    },
    response: {
      headers: {},
      _meta: {
        warnings: ['URL_SKIPPED'],
      },
    },
  },
}

If this is not present, and no body/headers are sent, it means that the request actually had no body/headers. We can use this in the UI to show appropriate hints to the user.

Dropped experiment

This drops the related experimental options.

Closes #7830

@mydea mydea requested review from billyvg and Lms24 April 25, 2023 11:26
@mydea mydea self-assigned this Apr 25, 2023
@mydea mydea added Type: Feature Package: replay Issues related to the Sentry Replay SDK labels Apr 25, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Apr 25, 2023

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 21.02 KB (-0.01% 🔽)
@sentry/browser - ES5 CDN Bundle (minified) 65.66 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 19.56 KB (+0.01% 🔺)
@sentry/browser - ES6 CDN Bundle (minified) 58.12 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 21.17 KB (0%)
@sentry/browser - Webpack (minified) 69.07 KB (0%)
@sentry/react - Webpack (gzipped + minified) 21.19 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 49.04 KB (0%)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 28.59 KB (+0.01% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 26.82 KB (-0.01% 🔽)
@sentry/replay ES6 CDN Bundle (gzipped + minified) 46.48 KB (+0.46% 🔺)
@sentry/replay - Webpack (gzipped + minified) 40.27 KB (+0.33% 🔺)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 65.25 KB (+0.21% 🔺)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 58.24 KB (+0.25% 🔺)

captureNetworkBodies: true,
},

networkDetailAllowUrls: ['http://localhost:7654/foo'],
Copy link
Member

Choose a reason for hiding this comment

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

Just double checking, is the port here stable?

Copy link
Member Author

Choose a reason for hiding this comment

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

🤷 this is also in the tests, so it would fail there as well (we check the url of the events is this in there)

@@ -78,33 +80,37 @@ async function _prepareFetchData(
const {
url,
method,
status_code: statusCode,
status_code: statusCode = 0,
Copy link
Member

Choose a reason for hiding this comment

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

Should this default to 0? Does undefined vs explicit 0 have different meanings?

Copy link
Member Author

Choose a reason for hiding this comment

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

We've been doing this before as well (only setting it further down) - but 🤷 I'd say it's fine?

/**
* Capture the following request headers, in addition to the default ones.
* Only applies to URLs matched by `networkDetailAllowUrls`.
* Any headers defined here will be captured in addition to the default headers.
Copy link
Member

Choose a reason for hiding this comment

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

Should we list the default headers here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd just document them in the docs, IMHO should be good enough? As this may change, I guess.

* Only applies to URLs matched by `networkDetailAllowUrls`.
* Any headers defined here will be captured in addition to the default headers.
*/
networkRequestHeaders: string[];
Copy link
Member

Choose a reason for hiding this comment

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

Wonder if headers should be RegExp + string as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's a valid point, although I think this is less of a use case probably. IMHO I'd leave this like this for now, we can always extend this later if we want to also allow regex.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah we can add it later -- I was testing this locally for sentry and found myself wanting to add x-sentry-*

Copy link
Member

@billyvg billyvg left a comment

Choose a reason for hiding this comment

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

:shipit:

@ryan953
Copy link
Member

ryan953 commented Apr 25, 2023

I think we need to also have a BODY_SKIPPED warning, so we can tell the difference between 'this GET request didn't have a body' and the 'networkCaptureBodies was set to false' case.

@mydea
Copy link
Member Author

mydea commented Apr 26, 2023

I think we need to also have a BODY_SKIPPED warning, so we can tell the difference between 'this GET request didn't have a body' and the 'networkCaptureBodies was set to false' case.

Good point! I think I can add this in a follow up PR, though.

@mydea mydea merged commit 7d10c44 into develop Apr 26, 2023
@mydea mydea deleted the fn/replay-urls branch April 26, 2023 08:54
billyvg added a commit to getsentry/sentry that referenced this pull request Feb 5, 2025
…IPPED` warning

This actually does not exist in our SDK, instead we can make the same assumption using sdk configuration options (e.g. if they have the option enabled) and if the body is empty to determine if we should show the onboarding setup.

ref: getsentry/sentry-javascript#7953 (comment)
closes: getsentry/sentry-javascript#8004
@billyvg
Copy link
Member

billyvg commented Feb 7, 2025

I think we need to also have a BODY_SKIPPED warning, so we can tell the difference between 'this GET request didn't have a body' and the 'networkCaptureBodies was set to false' case.

Good point! I think I can add this in a follow up PR, though.

Doing some tidying, we did not add a BODY_SKIPPED warning, but rather, added SDK options so that you can check if networkCaptureBodies is true or false.

billyvg added a commit to getsentry/sentry that referenced this pull request Feb 7, 2025
…IPPED` warning (#84654)

This actually does not exist in our SDK, instead we can make the same
assumption using sdk configuration options (e.g. if they have the option
enabled) and if the body is empty to determine if we should show the
onboarding setup.

ref:
getsentry/sentry-javascript#7953 (comment)
closes: getsentry/sentry-javascript#8004
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: replay Issues related to the Sentry Replay SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Define allowlist of URLs to capture replay request bodies/headers for
3 participants