Skip to content

feat: add multiply function and numeric modifiers support to JSON key… - #81

Merged
kazeburo merged 5 commits into
mainfrom
feat/multiply
Oct 11, 2026
Merged

kazeburo merged 5 commits into
mainfrom
feat/multiply

Conversation

@kazeburo

@kazeburo kazeburo commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

User description

… parsing


PR Type

Enhancement, Tests, Documentation


Description

  • Add multiply(factor) to JSON-path modifiers.

  • Scale count, group-by, and percentile output.

  • Reject invalid or non-finite multiply factors.

  • Test and document numeric modifier behavior.


Diagram Walkthrough

flowchart LR
  A["JSON path with multiply"] -- "parse factor" --> B["Numeric modifier"]
  B -- "apply before output" --> C["Count and group-by metrics"]
  B -- "apply before output" --> D["Percentile metrics"]
Loading

File Walkthrough

Relevant files
Enhancement
jsonkey.go
Parse and validate numeric multiplication modifiers           

cmd/mackerel-plugin-jsonl/jsonkey.go

  • Parse numeric modifiers separately from string modifiers and
    initializers.
  • Add multiply(factor) parsing with finite-number validation.
+49/-15 
reader.go
Apply numeric modifiers to emitted metrics                             

cmd/mackerel-plugin-jsonl/reader.go

  • Store and apply numeric modifiers on aggregator functions.
  • Scale count, group-by, and percentile values before output.
  • Accept floating-point counts for duration calculations.
+37/-24 
Tests
jsonkey_test.go
Test numeric modifier parsing and application                       

cmd/mackerel-plugin-jsonl/jsonkey_test.go

  • Add a helper for checking parsed modifier counts.
  • Test multiplication alongside existing modifiers and invalid inputs.
+45/-60 
Documentation
README.md
Document multiplication and aggregator modifier support   

README.md

  • Document multiply(factor) and its supported aggregators.
  • Clarify modifier and initializer usage restrictions.
+5/-2     

@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit 4c9fb5f)

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

Non-finite output

A finite factor can still produce an infinite metric. For example, multiply(1e308) applied to a count of 2 yields +Inf, which is then written as a metric value. Reject or handle non-finite results after multiplication.

return func(n float64) float64 { return n * factor }, nil
Invalid format

splitter.IgnoreEmpties discards empty arguments, so malformed inputs such as multiply(,2) and multiply(2,) are accepted as multiply(2). Reject empty arguments rather than silently changing the configured factor expression.

parts, err := s.Split(inner, splitter.TrimSpaces, splitter.UnescapeQuotes, splitter.IgnoreEmpties)
if err != nil {
	return nil, err
}
if len(parts) != 1 {
	return nil, fmt.Errorf("invalid multiply() format: %s", fn)
✅ Resolved findings

cmd/mackerel-plugin-jsonl/reader.go:192-193

Numeric Overflow

A finite factor can still overflow when applied to a value. For example, multiply(1e308) with a count of 2 emits +Inf, even if dividing by the duration would produce a finite rate. Handle non-finite results before emitting metrics.

cmd/mackerel-plugin-jsonl/jsonkey.go:140

Infinite Output

A finite factor can still produce an infinite result: multiply(1e308) applied to a count of 2 returns +Inf, which is then emitted as a metric value. Check the result before output so a valid factor cannot produce an invalid metric.

cmd/mackerel-plugin-jsonl/jsonkey.go:135-138

Numeric Overflow

A finite factor can still produce a non-finite metric: multiply(1e308) applied to a count of 2 emits +Inf. Check the result before emitting it, not just the parsed factor.

cmd/mackerel-plugin-jsonl/jsonkey.go:131-135

Invalid Factors

strconv.ParseFloat accepts NaN and Inf, so multiply(NaN) or multiply(Inf) passes validation and emits non-finite metric values. Reject non-finite factors before creating the modifier.

Comment thread cmd/mackerel-plugin-jsonl/jsonkey.go
@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Benchmark Comparison

Base: main
PR: feat/multiply

BenchmarkMainParse results
goos: linux
goarch: amd64
pkg: github.com/monitoring-forge/mackerel-plugin-jsonl/cmd/mackerel-plugin-jsonl
cpu: AMD EPYC 7763 64-Core Processor                
                             │ base-bench.txt │            pr-bench.txt            │
                             │     sec/op     │   sec/op     vs base               │
MainParse_jsonl-4                33.98m ± 19%   32.04m ± 3%  -5.72% (p=0.000 n=10)
MainParse_jsonl_eachkey-4        41.95m ±  0%   43.77m ± 1%  +4.33% (p=0.000 n=10)
MainParse_parse_and_output-4     35.55m ±  1%   33.37m ± 1%  -6.13% (p=0.000 n=10)
geomean                          37.01m         36.03m       -2.62%

                             │ base-bench.txt │            pr-bench.txt             │
                             │      B/op      │     B/op      vs base               │
MainParse_jsonl-4                4.642Mi ± 0%   4.642Mi ± 0%  +0.00% (p=0.001 n=10)
MainParse_jsonl_eachkey-4        4.642Mi ± 0%   4.642Mi ± 0%  +0.00% (p=0.007 n=10)
MainParse_parse_and_output-4     5.412Mi ± 0%   5.412Mi ± 0%  +0.00% (p=0.014 n=10)
geomean                          4.885Mi        4.886Mi       +0.00%

                             │ base-bench.txt │            pr-bench.txt            │
                             │   allocs/op    │  allocs/op   vs base               │
MainParse_jsonl-4                 100.4k ± 0%   100.4k ± 0%       ~ (p=0.337 n=10)
MainParse_jsonl_eachkey-4         100.4k ± 0%   100.4k ± 0%       ~ (p=0.474 n=10)
MainParse_parse_and_output-4      100.5k ± 0%   100.5k ± 0%       ~ (p=1.000 n=10)
geomean                           100.4k        100.4k       +0.00%

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 6cb309a

@github-actions

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

No code suggestions found for the PR.

@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 5847c97

Comment thread README.md Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 389e307

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@sonarqubecloud

Copy link
Copy Markdown

Comment thread cmd/mackerel-plugin-jsonl/reader.go
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 4c9fb5f

@github-actions

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

No code suggestions found for the PR.

@kazeburo
kazeburo merged commit a9500dd into main Oct 11, 2026
4 checks passed
@kazeburo
kazeburo deleted the feat/multiply branch October 11, 2026 11:51
@github-actions github-actions Bot mentioned this pull request Oct 11, 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