Repository navigation
fix(datadog grok): don't panic on NaN in keyvalue filter - #1990
Conversation
nom's `double` accepts `nan`, `inf` and `infinity` and overflows large exponents to infinity, so an unquoted value like `ratio=NaN` reached the `NotNan::new(v).expect(..)` in `parse_number` and panicked. Reject non-finite values there so they fall through to the string parser. Generated with Claude Opus 5.5
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30cd12d7b0
ℹ️ 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".
![datadoghq-integration[bot]](https://avatars.githubusercontent.com/u/16866914?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
Non-finite values produce a recoverable parse error, allowing the existing string fallback to preserve their original text while leaving finite-number parsing unchanged.
🤖 Bits Code Review · Commit 30cd12d
|
Infinities were never the problem since `NotNan` accepts them, so keep parsing `inf`, `infinity` and overflowing values like `1e999` as floats and only fall back to a string for NaN. Generated with Claude Opus 5.5
tessneau
changed the title
4 participants
Summary
The
keyvaluegrok filter panics when a value is an unquotedNaN, so an ordinary log line likeratio=NaNcrashes whatever is callingparse_grok/parse_groks:parse_numberuses nom'sdouble, which also acceptsnan(case-insensitive). NaN is never integral, so it ends up inNotNan::new(v).expect("not a float").The fix turns that
expectinto a recoverable parse error, so NaN falls through to the string parser and keeps its original text. Infinities are unaffected sinceNotNanaccepts them, soinf,Infinityand overflowing values like1e999still parse as floats.How did you test this PR?
Added
parse_valueunit tests and an end-to-endparse_groktest coveringNaN/nan/NAN, including the=delimiter form, plusinf/Infinity/±1e999to pin the existing float behavior. The NaN tests reproduced the panic before the fix.Does this PR include user-facing changes?
our guidelines.
Checklist
run
dd-rust-license-tool writeand commit the changes. More details here.References
N/A