Benchmark: grafana PR 90045

[celmis-codereviewer]

Benchmark reproduction of grafana#90045

[celmis-codereviewer]

✅ APPROVED — no blocking findings

Full findings and scope are in the review summary comment on this pull request — one persistent comment, updated in place on every run.

[celmis-codereviewer]

❌ CHANGES REQUESTED — blocking findings

Full findings and scope are in the review summary comment on this pull request — one persistent comment, updated in place on every run.

[celmis-codereviewer]

Why: When d.Storage.Create returns an error on line 43, line 45 calls recordLegacyDuration instead of recordStorageDuration, recording Storage operation failures and duration under Legacy metrics.

🟠 recordLegacyDuration called instead of recordStorageDuration on Storage.Create failure

When d.Storage.Create returns an error, line 45 records the error duration using d.recordLegacyDuration rather than d.recordStorageDuration. This causes failures in the primary Storage engine to be misattributed to Legacy metrics.

Also at line 125.

Suggested change

	d.recordLegacyDuration(true, mode3Str, options.Kind, method, startStorage)
	d.recordStorageDuration(true, mode3Str, options.Kind, method, startStorage)

_{agent: defect · rule: defect.copy-paste · confidence: 0.95}

[celmis-codereviewer]

Why: When d.Delete initializes context on line 96, d.Log is passed to klog.NewContext instead of log, stripping the method, kind, and name fields added on line 95 from the context logger.

🟠 Base logger passed to klog.NewContext instead of contextual logger in Delete

On line 95, log is created with contextual key-value pairs (name, kind, method). However, line 96 attaches d.Log to the context instead of log. Any downstream code retrieving the logger from ctx via klog.FromContext(ctx) will miss these contextual log fields.

Suggested change

	log := d.Log.WithValues("name", name, "kind", options.Kind, "method", method)
	ctx = klog.NewContext(ctx, log)

_{agent: defect · rule: defect.wrong-variable · confidence: 0.95}

[celmis-codereviewer]

Why: When d.Storage.Delete completes successfully, line 105 passes the object name parameter instead of options.Kind to recordStorageDuration, polluting metric labels with individual resource instance names.

🟠 Object name passed as kind parameter to recordStorageDuration

On line 105, d.recordStorageDuration is called with name as the third parameter instead of options.Kind. In all other metric tracking calls, the third parameter is the resource kind. Passing the object instance name causes high-cardinality metric label values.

Suggested change

	}
	d.recordStorageDuration(false, mode3Str, options.Kind, method, startStorage)

_{agent: defect · rule: defect.wrong-argument · confidence: 0.95}

[celmis-codereviewer]

Why: When the goroutine handling legacy collection deletion completes on line 159, line 160 calls recordStorageDuration instead of recordLegacyDuration, recording Legacy operations under Storage metrics.

🟠 recordStorageDuration called instead of recordLegacyDuration for Legacy.DeleteCollection

In the background goroutine for DeleteCollection, d.Legacy.DeleteCollection is executed on line 159, but line 160 records the result using d.recordStorageDuration instead of d.recordLegacyDuration.

Suggested change

	d.recordLegacyDuration(err != nil, mode3Str, options.Kind, method, startLegacy)

_{agent: defect · rule: defect.copy-paste · confidence: 0.95}

[celmis-codereviewer]

🤖 Code Review for PR #16

⚙ ADJUSTED — graph context partial (2 of 5 changed files): 3 of 5 changed files have no symbols in the index; 3 of them are not in that checkout at all (pkg/apiserver/rest/dualwriter_mode1_test.go, pkg/apiserver/rest/dualwriter_mode3.go, pkg/apiserver/rest/dualwriter_mode3_test.go) — this PR's base is older than the indexed revision, so those files were renamed or deleted before it and no re-index can bring them back; there is nothing to fix.

❌ CHANGES REQUESTED — blocking findings

Findings

• 🟠 Error: 4

Scope

• Files changed: 5
• Lines: +524 / -131

Performance

• Analysis time: 66.7s · agents: structural, cve, contract, security, defect · tokens: 59,422/17,235

---

_{Powered by Code Analyzer · context: tree-sitter graph + structural, cve, contract, security, defect}