chore: add omitempty to bson/json tags for FilterPolicy and TLSPrivat…#3972
chore: add omitempty to bson/json tags for FilterPolicy and TLSPrivat…#3972Yogeshjindal wants to merge 6 commits intomainfrom
Conversation
…eKeyPath These fields serialize as empty strings in API responses when unset, causing test failures for the k8s-proxy config endpoint. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Code Review SummaryStatus: No Issues Found | Recommendation: Merge OverviewThis PR adds Changes Made:
Previous Issues (All Resolved)The previous review flagged concerns about pointer type changes (
Files Reviewed (2 files)
Reviewed by claude-4.5-opus-20251124 · 113,864 tokens |
🚀 Keploy Performance Test ResultsMulti-Run Validation: Tests run 3 times, pipeline fails only if 2+ runs show regression.
Thresholds: P50 < 5ms, P90 < 15ms, P99 < 70ms, RPS >= 100 (±1% tolerance), Error Rate < 1% ✅ Result: PASSED - Only 1 out of 3 runs failed (threshold: 2) P50, P90, and P99 percentiles naturally filter out outliers |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
pkg/models/testrun.go
Outdated
| Success int `json:"success" yaml:"success"` | ||
| Failure int `json:"failure" yaml:"failure"` | ||
| Obsolete int `json:"obsolete,omitempty" yaml:"obsolete,omitempty"` | ||
| Obsolete *int `json:"obsolete,omitempty" yaml:"obsolete,omitempty"` |
There was a problem hiding this comment.
CRITICAL: Changing Obsolete from int to *int causes compilation errors in 6+ locations that haven't been updated.
Affected locations:
pkg/service/report/report.go:174-obsolete += rep.Obsolete- cannot add*inttointpkg/service/report/report.go:205-obsolete: rep.Obsolete- assigning*inttointfieldpkg/service/replay/replay.go:1700-obsolete: testReport.Obsolete- assigning*inttointfieldpkg/service/replay/replay.go:1712-r.totalTestObsolete += testReport.Obsolete- cannot add*inttointpkg/service/replay/replay.go:1735-testReport.Obsolete > 0- cannot compare*inttointpkg/service/replay/replay.go:1759-testReport.Obsolete > 0- cannot compare*inttoint
Fix: Either revert to int type, or update all usages to dereference with nil checks, e.g.:
if testReport.Obsolete != nil && *testReport.Obsolete > 0 { ... }
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent unset config/model fields from serializing as empty strings in API responses (notably impacting the k8s-proxy config endpoint), by adjusting struct field tags/optionality.
Changes:
- Updated
models.Filter.FilterPolicyto be optional and tagged withomitemptyfor JSON/YAML/BSON. - Updated
config.Record.TLSPrivateKeyPathto be optional and tagged withomitemptyfor JSON/YAML/BSON. - Changed
models.TestReport.Obsoletefromintto*int.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/models/testrun.go | Changes TestReport.Obsolete to a pointer type, affecting how/when it serializes. |
| pkg/models/instrument.go | Makes FilterPolicy optional with omitempty tags (currently via pointer type). |
| config/config.go | Makes TLSPrivateKeyPath optional with omitempty tags (currently via pointer type). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
🚀 Keploy Performance Test ResultsMulti-Run Validation: Tests run 3 times, pipeline fails only if 2+ runs show regression.
Thresholds: P50 < 5ms, P90 < 15ms, P99 < 70ms, RPS >= 100 (±1% tolerance), Error Rate < 1% ✅ Result: PASSED - Only 1 out of 3 runs failed (threshold: 2) P50, P90, and P99 percentiles naturally filter out outliers |
🚀 Keploy Performance Test ResultsMulti-Run Validation: Tests run 3 times, pipeline fails only if 2+ runs show regression.
Thresholds: P50 < 5ms, P90 < 15ms, P99 < 70ms, RPS >= 100 (±1% tolerance), Error Rate < 1% ✅ Result: PASSED - Only 1 out of 3 runs failed (threshold: 2) P50, P90, and P99 percentiles naturally filter out outliers |
🚀 Keploy Performance Test ResultsMulti-Run Validation: Tests run 3 times, pipeline fails only if 2+ runs show regression.
Thresholds: P50 < 5ms, P90 < 15ms, P99 < 70ms, RPS >= 100 (±1% tolerance), Error Rate < 1% ✅ Result: PASSED - Only 1 out of 3 runs failed (threshold: 2) P50, P90, and P99 percentiles naturally filter out outliers |
…eKeyPath
These fields serialize as empty strings in API responses when unset, causing test failures for the k8s-proxy config endpoint.
Describe the changes that are made
Links & References
Closes: #[issue number that will be closed through this PR]
🔗 Related PRs
🐞 Related Issues
📄 Related Documents
What type of PR is this? (check all applicable)
Added e2e test pipeline?
Added comments for hard-to-understand areas?
Added to documentation?
Are there any sample code or steps to test the changes?
Self Review done?
Any relevant screenshots, recordings or logs?
🧠 Semantics for PR Title & Branch Name
Please ensure your PR title and branch name follow the Keploy semantics:
📌 PR Semantics Guide
📌 Branch Semantics Guide
Examples:
fix: patch MongoDB document update bugfeat/#1-login-flow(You may skip mentioning the issue number in the branch name if the change is small and the PR description clearly explains it.)Additional checklist: