Skip to content

merge coroot/logparser v1.4.2: JSON parser, pattern rate limit, level fixes - #34

Merged
blue4209211 merged 8 commits into
mainfrom
sync/coroot-logparser-v1.4.2
Oct 9, 2026
Merged

blue4209211 merged 8 commits into
mainfrom
sync/coroot-logparser-v1.4.2

Conversation

@mayankpande88

Copy link
Copy Markdown
Contributor

Summary

Merges coroot/logparser main (v1.4.2) into this fork:

  • a JSON log parser that extracts the message, level and attributes;
  • multiline handling for JSON logs;
  • a rate limit on pattern extraction under log storms;
  • level fixes: MySQL/MariaDB [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 from main.

Engineering detail

How the two sides were combined:

  • NewParser takes both upstream's parseJson and limiter and this fork's SensitiveConfig. OnMsgCallbackF gets upstream's attributes argument.
  • Levels and patterns still come from this fork's ParseStructuredLog (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-data detection are skipped, but the message is still passed to the callback under the sampled pattern hash. Upstream drops it, so this differs: a limit meant to cap CPU must not drop log records. TestParserRateLimit asserts this.
  • Name clash: pattern.go's jsonMessageKeys/jsonLevelKeys are renamed to patternMessageKeys/patternLevelKeys, because json.go declares the same names.

Tests: go vet and go 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.

  • JSON log line ({"level":"error","msg":"payment failed","order_id":...,"trace_id":...,"span_id":...}): it arrived with body payment failed, severity error, attribute order_id, and the trace and span IDs set. The build against the previous commit sent the raw JSON line, without trace context.
  • Pattern hash: the same in both builds.
  • Error-line storm (about 59,000 lines in 30s): every line was counted. With the limiter at 100/s, 55,070 went under the sampled pattern; with no limiter, all went under their own single pattern.

def and others added 7 commits July 24, 2026 17:10
…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.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment thread parser.go Outdated
Comment thread multiline.go Outdated
Comment thread json.go
…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
blue4209211 merged commit 99e21f7 into main Oct 9, 2026
1 check passed
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.

3 participants