-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(eventstream): Synchronously write occurrences to EAP from the SnubaEventStream
#103566
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
feat(eventstream): Synchronously write occurrences to EAP from the SnubaEventStream
#103566
Conversation
src/sentry/eventstream/snuba.py
Outdated
| ) | ||
|
|
||
| if resp.status == 200: | ||
| metrics.incr("eventstream.eap.occurrence_insert.success") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open to different names for these metrics, currently eventstream.eap.occurrence_insert.success & eventstream.eap.occurrence_insert.failure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might tweak to make clear that these are the synchronous metrics (as opposed to the kafka metrics) somewhow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, will add a tag to the metrics and make the log message more specific
| try: | ||
| resp = snuba._snuba_pool.urlopen( | ||
| "POST", | ||
| "/tests/entities/eap_items/insert_bytes", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: This is not the only place we use this URL in the repo... do you think it would be appropriate to break out to a constant? Maybe in a followup PR, probably not worth doing here/now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, good idea. Will make that change in a follow-up PR.
src/sentry/eventstream/snuba.py
Outdated
| ) | ||
|
|
||
| if resp.status == 200: | ||
| metrics.incr("eventstream.eap.occurrence_insert.success") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might tweak to make clear that these are the synchronous metrics (as opposed to the kafka metrics) somewhow.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #103566 +/- ##
===========================================
+ Coverage 80.34% 80.60% +0.25%
===========================================
Files 9255 9269 +14
Lines 395203 395930 +727
Branches 25201 25201
===========================================
+ Hits 317527 319124 +1597
+ Misses 77246 76376 -870
Partials 430 430 |
…103780) Follow-up to #103566. Fixes [ID-1112](https://linear.app/getsentry/issue/ID-1112/extract-testsentitieseap-itemsinsert-bytes-endpoint-name-out-into-a). No change in logic; creates a constant for the EAP insert items endpoint path ("/tests/entities/eap_items/insert_bytes") in a new utils file `src/sentry/utils/eap.py`.
#103857) Follow-up to #103566. The previous logic did not correctly serialize the trace item when making a POST request to the "/tests/entities/eap_items/insert_bytes" endpoint. With this updated logic we should see successful EAP inserts (example below). Querying via `docker exec -it snuba-clickhouse-1 clickhouse-client --query "SELECT item_id, project_id, organization_id, trace_id, item_type, timestamp FROM eap_items_1_local ORDER BY timestamp DESC LIMIT 5 FORMAT Pretty"`. Before ingesting new event: <img width="1061" height="183" alt="image" src="https://github.com/user-attachments/assets/9d2eca89-8fc9-4ab5-9fb3-ae34fd862ef8" /> After ingesting new event: <img width="1055" height="186" alt="image" src="https://github.com/user-attachments/assets/18caf67e-4d4b-4232-a2a9-b00225a52396" />
Resolves EAP-318.
We're already forwarding occurrences to EAP via the Kafka backend based on the
eventstream.eap_forwarding_rateoption. This PR adds support for synchronously writing occurrences to EAP via POST request to/tests/entities/eap_items/insert_bytesin theSnubaEventStream. This logic will begin running ifeventstream.eap_forwarding_rateis enabled for a self-hosted org.