feat: expose OTEL_CONFIG in Helm chart values.yaml, disabled by default - #20898
feat: expose OTEL_CONFIG in Helm chart values.yaml, disabled by default#20898uzairhameed wants to merge 2 commits into
Conversation
|
Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, you can find updates in the #github-notifications channel in the community Slack. |
📝 WalkthroughWalkthroughThe Helm chart adds an optional ChangesOpenTelemetry configuration
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@install/kubernetes/helm/meshery/values.yaml`:
- Around line 21-23: Update the environment variable rendering in the deployment
template to pipe each value through Helm’s quote function, including
OTEL_CONFIG, so multiline configurations remain a single valid environment value
and the default empty string is preserved. Validate the rendered chart with helm
template.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 773ba0cd-face-4868-ba13-ff2ff5a754a6
📒 Files selected for processing (1)
install/kubernetes/helm/meshery/values.yaml
There was a problem hiding this comment.
Pull request overview
This PR exposes Meshery Server’s OTEL_CONFIG environment variable through the Meshery Helm chart’s values.yaml, keeping tracing disabled by default (empty value) unless explicitly configured by the user.
Changes:
- Add
OTEL_CONFIGto the chart’senv:values with an empty default. - Document intent in
values.yamlcomments (tracing enabled only when configured).
Signed-off-by: Uzair Hameed <[email protected]>
a9785ef to
7d263e7
Compare
|
Commit SHA: END-TO-END TESTS
📦 Test Result Summary
⌛ Duration: 3 minutes and 59 seconds Overall Result: 👍 All tests passed. [Show/Hide] Test Result Details
🔗 Relationship Tests [Show/Hide] Relationship Test Details (18 tests)
|
Fixes: meshery/meshkit#894 (partial — exposes tracing config via Helm)
Summary
Adds
OTEL_CONFIGto the Helm chart'svalues.yaml, defaulted to emptyso tracing stays disabled unless a user explicitly configures a collector
endpoint.
Why
Acceptance criterion 3 in meshery/meshkit#894 asked for this deployment
option to be exposed in Meshery's Helm chart. It wasn't previously present
— Helm deployments had no supported way to enable tracing at all.
Testing
Confirmed
values.yamlcorrectly passes through to the container's envvars via the existing
deployment.yamltemplate.Note
Also updates
templates/deployment.yamlto quote env values(
{{ $val | quote }}), sinceOTEL_CONFIGcan hold a multiline YAMLblock which would otherwise render as invalid YAML.
Related
This is one part of a larger fix for meshery/meshkit#894. The other part
— correcting
tracing.go's disable logic and suppressing repeatedexport-failure logs — is tracked separately in meshery/meshkit#1070