Skip to content

fix: resolve duplicated status parsing in ltsvreader - #120

Merged
kazeburo merged 3 commits into
masterfrom
fix/ltsvparser-duplicated-status
Oct 10, 2026
Merged

kazeburo merged 3 commits into
masterfrom
fix/ltsvparser-duplicated-status

Conversation

@kazeburo

@kazeburo kazeburo commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

PR Type

Bug fix, Enhancement


Description

  • Preserve first duplicate LTSV status value.

  • Prioritize configured status keys in both readers.

  • Return no values when parsing fails.

  • Replace parse flags with value checks.


File Walkthrough

Relevant files
Enhancement
2 files
axslog.go
Remove parse flags from the reader interface                         
+10/-19 
parser.go
Validate parsed values instead of status flags                     
+3/-3     
Tests
3 files
axslog_test.go
Remove tests for obsolete parse flags                                       
+0/-12   
jsonreader_test.go
Update JSON tests for two-value parse results                       
+3/-14   
ltsvreader_test.go
Add duplicate-key coverage and update parse assertions     
+15/-18 
Error handling
1 files
jsonreader.go
Simplify JSON results and handle parse errors                       
+8/-10   
Bug fix
1 files
ltsvreader.go
Ignore duplicate fields and prioritize configured statuses
+16/-8   
Configuration changes
1 files
Makefile
Declare additional build and check targets phony                 
+1/-1     

@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit 0e7faa8)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Duplicate Tracking

With at least 64 configured status keys, shifting a uint64 by an index of 64 or more produces zero. Repeated occurrences of those keys are therefore counted as new matches, allowing remaining to reach zero before a higher-priority status key appears. A log entry can then return the wrong status.

bit := uint64(1) << idx
if found&bit != 0 {
	return nil
}
found |= bit
remaining--
✅ Resolved findings

jsonreader/jsonreader.go:73-75

Malformed JSON

A JSON parse error now returns values found before the error instead of discarding them. With status keys status and code, malformed input such as {"reqtime":0.03,"code":200, can be counted as a valid request. Return empty values on unexpected parse errors.

ltsvreader/ltsvreader.go:41-46

Bitset Overflow

For a configured key at index 64 or higher, this shift produces a zero bit. Repeated occurrences are then counted as new matches, so enough duplicates can make remaining reach zero and stop parsing before a later processing-time or higher-priority status field.

ltsvreader/ltsvreader.go:43-48

Duplicate Tracking

For configurations with at least 65 keys, shifting a uint64 by an index of 64 or more produces zero, so duplicates of those keys are never marked as found. A duplicate can decrement remaining twice and stop parsing before a later reqtime field is read.

Comment thread ltsvreader/ltsvreader.go
@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Benchmark Comparison

Base: master
PR: fix/ltsvparser-duplicated-status

BenchmarkMainParse results
goos: linux
goarch: amd64
pkg: github.com/monitoring-forge/mackerel-plugin-axslog
cpu: AMD EPYC 9V45 96-Core Processor                
                             │ base-bench.txt │            pr-bench.txt            │
                             │     sec/op     │   sec/op     vs base               │
MainParse_jsonl-4                18.69m ±  4%   18.66m ± 6%       ~ (p=0.971 n=10)
MainParse_ltsv-4                 13.23m ± 17%   12.98m ± 8%       ~ (p=0.631 n=10)
MainParse_jsonl_and_output-4     20.24m ±  8%   20.58m ± 4%       ~ (p=0.218 n=10)
MainParse_ltsv_and_output-4      14.13m ±  4%   13.79m ± 4%  -2.37% (p=0.011 n=10)
geomean                          16.31m         16.19m       -0.70%

                             │ base-bench.txt │            pr-bench.txt             │
                             │      B/op      │     B/op      vs base               │
MainParse_jsonl-4                4.289Mi ± 0%   4.289Mi ± 0%       ~ (p=0.796 n=10)
MainParse_ltsv-4                 4.289Mi ± 0%   4.289Mi ± 0%       ~ (p=0.424 n=10)
MainParse_jsonl_and_output-4     5.059Mi ± 0%   5.059Mi ± 0%       ~ (p=0.093 n=10)
MainParse_ltsv_and_output-4      5.059Mi ± 0%   5.059Mi ± 0%  +0.00% (p=0.000 n=10)
geomean                          4.658Mi        4.658Mi       +0.00%

                             │ base-bench.txt │            pr-bench.txt             │
                             │   allocs/op    │ allocs/op   vs base                 │
MainParse_jsonl-4                  96.00 ± 0%   96.00 ± 0%       ~ (p=1.000 n=10)
MainParse_ltsv-4                   96.00 ± 0%   96.00 ± 0%       ~ (p=1.000 n=10)
MainParse_jsonl_and_output-4       157.0 ± 1%   157.0 ± 0%       ~ (p=1.000 n=10)
MainParse_ltsv_and_output-4        157.0 ± 0%   157.0 ± 0%       ~ (p=1.000 n=10) ¹
geomean                            122.8        122.8       +0.00%
¹ all samples are equal

@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit c7049f8

Comment thread jsonreader/jsonreader.go
Comment thread ltsvreader/ltsvreader.go
@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 0e7faa8

Comment thread ltsvreader/ltsvreader.go
Comment thread ltsvreader/ltsvreader.go
@kazeburo
kazeburo merged commit b720611 into master Oct 10, 2026
5 checks passed
@kazeburo
kazeburo deleted the fix/ltsvparser-duplicated-status branch October 10, 2026 13:31
@github-actions github-actions Bot mentioned this pull request Oct 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant