feat(regions,formance): wire global.monitoring.support#391
feat(regions,formance): wire global.monitoring.support#391sofiasimdianova wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (12)
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
WalkthroughThe root Helm chart table and both chart README badges were updated to new Formance and Regions versions. The Formance and Regions READMEs also document ChangesHelm README updates
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
64b34d2 to
0949b7b
Compare
NumaryBot
left a comment
There was a problem hiding this comment.
🛑 Changes requested — automated review
The new support monitoring defaults can leave the opt-in feature inert for chart-created stacks because the default selector does not match labels emitted by the chart.
0949b7b to
35bf4ae
Compare
NumaryBot
left a comment
There was a problem hiding this comment.
🛑 Changes requested — automated review
The new stack label can make otherwise valid stack names render invalid Kubernetes resources. This should be fixed before considering the patch correct.
35bf4ae to
0a9a454
Compare
NumaryBot
left a comment
There was a problem hiding this comment.
✅ Approve — automated review
The changes add the new monitoring support values through the formance and regions charts and add the stack label needed for selection. I did not find a discrete regression in the modified files.
No findings.
| support: | ||
| # -- Enable Formance support telemetry forwarding (opt-in) | ||
| # @section -- Monitoring configuration | ||
| enabled: false |
There was a problem hiding this comment.
Thanks for the extra context that the operator chart already has the support template on 3.11.3-dev and that global.* values propagate into the operator subchart automatically. The remaining issue is the release/lock coupling: this branch renders with operator 3.11.2, where that template is absent, so global.monitoring.support.enabled=true is still a no-op for anyone installing this chart before the next operator release. Please either bump/lock the operator dependency to a released version that contains the OTLP support template before merging, or document the minimum operator release/sequence so users do not enable a non-functional option.
0a9a454 to
a09a235
Compare
✅ Approve — automated reviewThe PR wires No findings. |
NumaryBot
left a comment
There was a problem hiding this comment.
NumaryBot posted 3 new inline findings.
Summary: #391 (comment)
a09a235 to
a0466e7
Compare
NumaryBot
left a comment
There was a problem hiding this comment.
NumaryBot posted 3 new inline findings.
Summary: #391 (comment)
a0466e7 to
ed72201
Compare
NumaryBot
left a comment
There was a problem hiding this comment.
NumaryBot posted 1 new inline finding.
Summary: #391 (comment)
ed72201 to
282927e
Compare
NumaryBot
left a comment
There was a problem hiding this comment.
NumaryBot posted 4 new inline findings.
Summary: #391 (comment)
282927e to
a629547
Compare
NumaryBot
left a comment
There was a problem hiding this comment.
NumaryBot review complete: no remaining inline findings.
Resolved 3 stale NumaryBot review threads (3 fixed, 0 outdated).
Summary: #391 (comment)
No description provided.