Adding gitOpsModeEnabled and gitOpsModeExceptions to anonymous statistics payload - #44161
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
lucasmrod
changed the title
gitOpsModeEnabled and gitOpsModeExceptions to anonymous statistics payload
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds two GitOps-related fields to the anonymous usage statistics payload: Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
server/datastore/mysql/statistics_test.go (1)
105-108: Fragile expectation coupled to migration-dumped defaults.Asserting
["labels", "secrets"]here depends on migration20260323144117_AddGitOpsExceptionsToAppConfigbeing baked into the dumped test schema. A future migration that changes existing-install defaults will silently require this test to be updated. The inline comment documents the dependency, which is good; consider additionally computing the expected slice from the loadedAppConfig.GitOpsConfig.Exceptionsto make the test resilient to default changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/datastore/mysql/statistics_test.go` around lines 105 - 108, The test currently asserts a hard-coded expectation on stats.GitOpsModeExceptions (["labels","secrets"]) which is fragile; instead load the app config's default exceptions and compare against that dynamic value. Replace the hard-coded assertion with one that reads the expected slice from the loaded AppConfig (AppConfig.GitOpsConfig.Exceptions) and assert.Equal(t, expectedExceptions, stats.GitOpsModeExceptions), keeping the existing assertion that stats.GitOpsModeEnabled is false; reference the test's stats variable and the AppConfig.GitOpsConfig.Exceptions field when making the change.server/datastore/mysql/statistics.go (1)
340-354: Helper correctly produces a stable, non-nil slice.Order matches the field declaration order in
fleet.GitOpsExceptionsand the capacity hint of 3 is accurate. Returningmake([]string, 0, 3)ensures JSON serializes as[]when empty, which matches the intended contract documented onStatisticsPayload.GitOpsModeExceptions.Optional: this helper could live as a method on
fleet.GitOpsExceptionsinserver/fleet/app.gofor cohesion/reuse, but current placement is fine since it's only consumed here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/datastore/mysql/statistics.go` around lines 340 - 354, No change required: the gitOpsExceptionsList helper correctly returns a stable, non-nil slice for fleet.GitOpsExceptions and matches StatisticsPayload.GitOpsModeExceptions' contract; if you prefer to refactor for cohesion, move this function to a method on fleet.GitOpsExceptions (e.g., func (e fleet.GitOpsExceptions) List() []string) in server/fleet/app.go and update callers to use that method, otherwise leave gitOpsExceptionsList as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/datastore/mysql/statistics_test.go`:
- Around line 105-108: The test currently asserts a hard-coded expectation on
stats.GitOpsModeExceptions (["labels","secrets"]) which is fragile; instead load
the app config's default exceptions and compare against that dynamic value.
Replace the hard-coded assertion with one that reads the expected slice from the
loaded AppConfig (AppConfig.GitOpsConfig.Exceptions) and assert.Equal(t,
expectedExceptions, stats.GitOpsModeExceptions), keeping the existing assertion
that stats.GitOpsModeEnabled is false; reference the test's stats variable and
the AppConfig.GitOpsConfig.Exceptions field when making the change.
In `@server/datastore/mysql/statistics.go`:
- Around line 340-354: No change required: the gitOpsExceptionsList helper
correctly returns a stable, non-nil slice for fleet.GitOpsExceptions and matches
StatisticsPayload.GitOpsModeExceptions' contract; if you prefer to refactor for
cohesion, move this function to a method on fleet.GitOpsExceptions (e.g., func
(e fleet.GitOpsExceptions) List() []string) in server/fleet/app.go and update
callers to use that method, otherwise leave gitOpsExceptionsList as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cb2c6655-b4fe-4709-9e1e-496c6bd8e53a
📒 Files selected for processing (4)
changes/42240-add-gitops-statsserver/datastore/mysql/statistics.goserver/datastore/mysql/statistics_test.goserver/fleet/statistics.go
There was a problem hiding this comment.
Pull request overview
Adds GitOps mode status and configured exception sub-options to Fleet’s anonymous usage analytics payload, enabling product analytics to measure adoption of GitOps mode and its exception settings.
Changes:
- Extended
fleet.StatisticsPayloadwithgitOpsModeEnabledandgitOpsModeExceptions. - Populated the new fields from
AppConfig.GitOpsConfigin MySQL statistics computation. - Added automated datastore tests covering default, enabled/disabled, and empty-exception scenarios, plus a changes note.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| server/fleet/statistics.go | Adds new GitOps fields to the anonymous statistics payload struct. |
| server/datastore/mysql/statistics.go | Computes/sends GitOps mode enabled + exceptions list (stable ordering, non-nil slice). |
| server/datastore/mysql/statistics_test.go | Adds coverage for GitOps stats across default/modified app config states. |
| changes/42240-add-gitops-stats | Release note entry for the new analytics fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #44161 +/- ##
==========================================
+ Coverage 66.76% 66.78% +0.01%
==========================================
Files 2629 2627 -2
Lines 211215 211157 -58
Branches 9421 9421
==========================================
- Hits 141027 141021 -6
+ Misses 57361 57317 -44
+ Partials 12827 12819 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Related issue: Resolves #42240.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Summary by CodeRabbit
New Features
Tests