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

chore: collect replay network metrics #6881

Merged
merged 1 commit into from
Jan 26, 2023

Conversation

vaind
Copy link
Contributor

@vaind vaind commented Jan 19, 2023

Adds network info to the collected metrics - it shows the amount of data sent (Net Tx) and received (Net Rx) by the browser. It works by intercepting the network requests with a custom handler that logs the transmission stats. Because Sentry SDK is the only application code sending requests, it equals the amount of data sent & received by the SDK. You can see in the example below that because there are no errors triggered, there's no data sent/received with just the plain Sentry SDK; there's network traffic only when Replay is added.

Currently, only HTTP requests are captured, no file:/// URLs (these are used to load the actual page and scripts from the local filesystem).

vaind#2 (comment)

image

@vaind vaind force-pushed the chore/metrics-network branch from e82ddbd to 601ca51 Compare January 20, 2023 09:39
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

EDIT: I accidentally approved this PR and can't dismiss it as it's still a draft. LMK when this is ready for review, then I'll give it another look.

Very interesting! For future readers and because I'm also not a 100% certain I understood correctly what's going on: Could you add a small description of what these additional metrics mean in the context of our SDK/replay? Is it the amount of bytes the SDK sent/received?

@vaind vaind marked this pull request as ready for review January 26, 2023 12:10
@vaind
Copy link
Contributor Author

vaind commented Jan 26, 2023

@Lms24 it's OK, I just didn't change it from draft back then because I wanted to wait for the CI to pass on my own fork first. Just made the change to a ready PR without any more commits so your review is valid.

@Lms24 Lms24 merged commit 6098879 into getsentry:master Jan 26, 2023
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.

3 participants