-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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(generic-metrics): Don't produce headers unnecessarily #67396
Conversation
These are no longer needed by snuba, it is unnecessary to produce this information as we don't use it for anything
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #67396 +/- ##
========================================
Coverage 84.33% 84.33%
========================================
Files 5309 5317 +8
Lines 237239 237594 +355
Branches 41031 41104 +73
========================================
+ Hits 200079 200379 +300
- Misses 36942 36997 +55
Partials 218 218
|
@@ -501,12 +497,7 @@ def reconstruct_messages( | |||
kafka_payload = KafkaPayload( | |||
key=message.payload.key, | |||
value=rapidjson.dumps(new_payload_value).encode(), | |||
headers=[ | |||
*message.payload.headers, | |||
("mapping_sources", mapping_header_content), |
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 just realized, this is technically still used by the last seen updater, so I'm not sure we can safely delete these headers while that consumer is still around.
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.
We should be fine, as I think we'll just hit this condition in the last seen updater https://github.com/getsentry/sentry/blob/master/src/sentry/sentry_metrics/consumers/last_seen_updater.py#L52-L65 and the message will be ignored/dropped.
Still, maybe let's make a note of this -- that by removing the headers the last seen updater will no longer be updating the last_seen column in PG.
These are no longer needed by snuba, it is unnecessary to produce this information as we don't use it for anything