Handle invalid timestamps in Kafka consume integration#8653
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08b94cd981
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
BenchmarksBenchmark execution time: 2026-05-18 13:18:31 Comparing candidate commit a70651c in PR branch Some scenarios are present only in baseline or only in candidate runs. If you didn't create or remove some scenarios in your branch, this maybe a sign of crashed benchmarks 💥💥💥 Scenarios present only in baseline:
Found 2 performance improvements and 2 performance regressions! Performance is the same for 49 metrics, 19 unstable metrics, 87 known flaky benchmarks, 39 flaky benchmarks without significant changes.
|
bouwkast
left a comment
There was a problem hiding this comment.
Seems reasonable
Curious as to why there are either very old or very far into the future times (or if it is just totally invalid)
A great question, and I have no idea 😅 It's happening for one customer, in one environmemnt, and so my best guess is that something is creating "invalid" values, maybe even for "test" purposes, but I really don't know 😅 |
Summary of changes
Add try-catch when grabbing the timestamp from a Kafka message
Reason for change
We have seen some cases where the conversion from a timestamp to a
DateTime(in the Confluent.Kafka library) was throwing anOverflowException. Likely due to an "invalid" value associated with the stored message.Implementation details
Throwing here would mean a lot of details would be lost (including DSM) so wrap in a try-catch to handle it. Other options are changing to grab the raw timestamp and detecting this issue before triggering it, but the net effect is the same (the tag can't be set) and it's a niche issue caused by an invalid scenario, so not worth the effort IMO
Test coverage
Meh, it's just a try-catch
Other details
Fixes error tracking issue