Repository navigation
merge coroot/logparser v1.4.2: JSON parser, pattern rate limit, level fixes - #34
Merged
Merged
Conversation
…ttern rate limit, level fixes
Brings in coroot's JSON log parser (message, level and attributes), the
multiline fixes for JSON logs, the pattern-extraction rate limit, and
the level fixes (MySQL/MariaDB [Note]/[System], level reported with an
entry, mongodb {"$date"} timestamps).
Resolution:
- NewParser takes both parseJson/limiter (upstream) and SensitiveConfig
(this fork). OnMsgCallbackF gets upstream's attributes argument.
- ParseStructuredLog still drives levels and patterns (JSON and logfmt).
With parseJson, a JSON log is emitted as its message with the other
fields as attributes, as upstream does.
- Over the rate limit, pattern extraction and sensitive detection are
skipped, but the message is still passed to the callback under the
sampled pattern hash; upstream drops it. TestParserRateLimit asserts
this.
- pattern.go's jsonMessageKeys/jsonLevelKeys are renamed to
patternMessageKeys/patternLevelKeys: json.go declares the same names.
This was referenced Oct 8, 2026
There was a problem hiding this comment.
Code Review
This pull request introduces JSON log parsing capabilities to extract levels, messages, and attributes, integrates this parsing into the main parser with rate limiting, updates multiline log collection to handle JSON lines, and expands log level guessing for MySQL/MariaDB. Feedback on these changes highlights a performance concern regarding redundant double-parsing of JSON logs in Parser.inc. Additionally, potential bugs were identified in both multiline.go and json.go where CRLF line endings or trailing whitespace could cause JSON detection and parsing to fail due to strict suffix checks on the closing brace.
…lines With parseJson, inc() decoded each JSON line twice: in ParseStructuredLog for the pattern content and level, and in ParseJsonLog for the message and attributes. It now decodes once (decodeJsonObject) and derives both from the same map. structuredFromJSON renders json.Number exactly as the json.Unmarshal float64 path did, so pattern hashes are unchanged; a test checks this across numeric, nested and fallback cases. Per JSON line: 16.0 -> 10.7 us, 103 -> 57 allocations. A JSON line ending in \r or other trailing whitespace was not parsed as JSON and not recognised as the start of a new multiline message; both checks now ignore it.
blue4209211
approved these changes
Oct 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Merges coroot/logparser
main(v1.4.2) into this fork:[Note]/[System], trusting the level reported with an entry, and mongodb{"$date"}timestamps.Please merge with "Create a merge commit", not squash. That keeps coroot's history, so the next sync only brings new commits, and keeps the commit node-agent pins (
2d6d6c9) reachable frommain.Engineering detail
How the two sides were combined:
NewParsertakes both upstream'sparseJsonandlimiterand this fork'sSensitiveConfig.OnMsgCallbackFgets upstream'sattributesargument.ParseStructuredLog(JSON and logfmt). WithparseJson, a JSON log is emitted as its message, with the other fields as attributes, as upstream does.TestParserRateLimitasserts this.pattern.go'sjsonMessageKeys/jsonLevelKeysare renamed topatternMessageKeys/patternLevelKeys, becausejson.godeclares the same names.Tests:
go vetandgo test ./...pass.Local e2e: I built nudgebee/node-agent against this commit and ran it as a systemd service on a local Debian 12 VM (kernel 6.1), exporting logs over OTLP to a local OpenTelemetry collector.
{"level":"error","msg":"payment failed","order_id":...,"trace_id":...,"span_id":...}): it arrived with bodypayment failed, severityerror, attributeorder_id, and the trace and span IDs set. The build against the previous commit sent the raw JSON line, without trace context.