Skip to content

Conversation

@thetruecpaul
Copy link
Contributor

I recently set up ingestion for group_first_seen. This column is defined as DateTime.

When I set up the rust processor, I was basing my processing on the datetime field, since I knew that the two were sent over identically from sentry/**/eventstream/snuba.py. This included using the StringToIntDatetime64 deserializer, which I still believe is correct. Unfortunately I followed that 64 further than I should've and set the field to be a u64 in the row — when it should've been a u32.

This PR fixes that to make future ingestions correct. (I based my changes on the timestamp field's parsing, which is similarly a DateTime (no 64) column.)

@thetruecpaul thetruecpaul requested a review from a team September 8, 2025 21:26
@thetruecpaul thetruecpaul requested a review from a team as a code owner September 8, 2025 21:26
Copy link
Member

@JoshFerge JoshFerge left a comment

Choose a reason for hiding this comment

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

would it be possible to add a test for this?

@thetruecpaul
Copy link
Contributor Author

It seems like the proper way to test things like this is through the snapshot system — I can look into adding the group_first_seen field to the snapshots as a followup, but that'll be in a different repo (I believe the snapshots live in sentry-kafka-schemas) and will be slow, so will land this ASAP now without the test.

@codecov
Copy link

codecov bot commented Sep 8, 2025

⚠️ File not in storage

No result to display due to the CLI not being able to find the file.
Please ensure the file contains junit in the name and automated file search is enabled,
or the desired file specified by the file and search_dir arguments of the CLI.

I recently set up ingestion for group_first_seen. This column is defined as `DateTime`.

When I set up the rust processor, I was basing my processing on the `datetime` field, since I knew that the two were sent over identically from `sentry/**/eventstream/snuba.py`. This included using the `StringToIntDatetime64` deserializer, which I still believe is correct. Unfortunately I followed that 64 further than I should've and set the field to be a `u64` in the row — when it should've been a `u32`.

This PR fixes that to make future ingestions correct. (I based my changes on the `timestamp` field's parsing, which is similarly a `DateTime` (no 64) column.)
@thetruecpaul thetruecpaul force-pushed the cpaul/fix/issuefirstseen3264 branch from 20a8b59 to dd3a433 Compare September 8, 2025 21:51
@thetruecpaul thetruecpaul merged commit a0478f6 into master Sep 8, 2025
34 checks passed
@thetruecpaul thetruecpaul deleted the cpaul/fix/issuefirstseen3264 branch September 8, 2025 22:34
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.

4 participants