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

Network Error Logging: backend part #55135

Merged
merged 41 commits into from
Nov 9, 2023
Merged

Network Error Logging: backend part #55135

merged 41 commits into from
Nov 9, 2023

Conversation

oioki
Copy link
Member

@oioki oioki commented Aug 21, 2023

Sentry backend part of the NEL (Network Error Logging) implementation.

  • Added a new nel event type and interface
  • Special logic for metadata and culprit generation

To be deployed after merging Relay part (getsentry/relay#2421) and releasing sentry-relay python package.

@github-actions github-actions bot added Scope: Frontend Automatically applied to PRs that change frontend components Scope: Backend Automatically applied to PRs that change backend components labels Aug 21, 2023
@oioki oioki requested review from olksdr and mdtro August 21, 2023 14:13
@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Merging #55135 (0fa2a8d) into master (5688a49) will increase coverage by 0.00%.
Report is 52 commits behind head on master.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master   #55135   +/-   ##
=======================================
  Coverage   80.75%   80.76%           
=======================================
  Files        5171     5174    +3     
  Lines      226919   226961   +42     
  Branches    38149    38160   +11     
=======================================
+ Hits       183253   183309   +56     
+ Misses      38111    38096   -15     
- Partials     5555     5556    +1     
Files Coverage Δ
src/sentry/api/serializers/models/project_key.py 100.00% <100.00%> (ø)
src/sentry/apidocs/examples/project_examples.py 100.00% <ø> (ø)
src/sentry/conf/server.py 89.88% <ø> (ø)
src/sentry/eventtypes/__init__.py 100.00% <100.00%> (ø)
src/sentry/eventtypes/nel.py 100.00% <100.00%> (ø)
src/sentry/models/projectkey.py 94.73% <100.00%> (+0.12%) ⬆️
src/sentry/utils/canonical.py 97.53% <ø> (ø)

... and 57 files with indirect coverage changes

@oioki
Copy link
Member Author

oioki commented Oct 16, 2023

is there a product doc somewhere on this?

No product doc for now, I'd prefer to release this internally and try this on our own org. Then we can promote it, etc.

will there be grouping or anything?

For now, grouping will be default by message (for example, connection / tcp.refused) but probably it makes sense to group by some parts of URL. This will require more effort for sure.

how do SDKs use this new API?

There is no need to touch SDKs, since NEL is a feature implemented on the browser side. The idea is similar to CSP in this sense.

oioki added a commit to getsentry/relay that referenced this pull request Nov 6, 2023
The first part of the NEL ([Network Error
Logging](https://developer.mozilla.org/en-US/docs/Web/HTTP/Network_Error_Logging))
implementation.

* Adds a new `Nel` EventType
* Adds a new `Nel` protocol with minimal validation (i.e. only specific
fields are captured)
* Adds a new `/api/:project_id/nel/` endpoint for NEL reports ingestion
  * simple content type validation
* splits payload into separate envelopes (a single HTTP NEL request
could contain several independent reports)
* `user.ip_address` field is set to the IP address of the request (real
user's IP address)
* An event is enriched with browser information derived from the
request's `User-Agent` header

Related PRs:
getsentry/sentry#55135
getsentry/sentry-kafka-schemas#177

---------

Co-authored-by: Oleksandr Kylymnychenko <oleksandr@sentry.io>
Co-authored-by: Oleksandr <1931331+olksdr@users.noreply.github.com>
Co-authored-by: Jan Michael Auer <mail@jauer.org>
@oioki oioki marked this pull request as ready for review November 6, 2023 16:57
@oioki oioki requested a review from a team as a code owner November 6, 2023 16:57
@oioki oioki requested a review from a team November 7, 2023 22:38
Copy link
Member

@jjbayer jjbayer left a comment

Choose a reason for hiding this comment

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

LGTM, one comment about "Unknown type"

src/sentry/culprit.py Outdated Show resolved Hide resolved
oioki and others added 2 commits November 9, 2023 12:24
Co-authored-by: Joris Bayer <joris.bayer@sentry.io>
@oioki oioki merged commit 685d98c into master Nov 9, 2023
48 of 49 checks passed
@oioki oioki deleted the hackweek/nel branch November 9, 2023 12:12
@github-actions github-actions bot locked and limited conversation to collaborators Nov 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants