[Feature] Add shared OpenTelemetry producer config#297
Open
royischoss wants to merge 1 commit into
Open
Conversation
- values.yaml — top-level telemetry: block with "" default for enabled (inherits collector state) - templates/config/mlrun-env-configmap.yaml — resolution logic + safety override + endpoint derivation - README.md — opt-in docs with truth table and external-endpoint example - Chart.yaml — bumped to 0.11.0-rc.38
davesh0812
reviewed
May 25, 2026
Contributor
davesh0812
left a comment
There was a problem hiding this comment.
Looks very good overall.
please add some unit tests if you want you can use those tests I added in mlefi :
- test_telemetry_default_inherits_collector_disabled — default values, expect MLRUN_TELEMETRY__ENABLED: "false", no endpoint key.
- test_telemetry_inherits_collector_enabled — --set opentelemetry.collector.enabled=true, expect ENABLED: "true" + in-cluster endpoint.
- test_telemetry_external_endpoint — --set telemetry.enabled=true --set telemetry.otlpEndpoint=external.com:4317 --set opentelemetry.collector.enabled=false, expect endpoint passes through.
- test_telemetry_safety_force_disable — --set telemetry.enabled=true with collector off and no endpoint, expect ENABLED: "false".
- test_telemetry_headers_secret_emitted_only_when_enabled — headersSecretName set but enabled=false, expect no HEADERS_SECRET_NAME key.
| <!-- These are Claude-specific behavioral instructions on HOW to respond. AGENTS.md covers WHAT the codebase contains. --> | ||
|
|
||
| - Values changes: show `--set` flags or a patch values file overlay, not edits to `values.yaml` directly, unless there is a change with the default value that should be reflected in `values.yaml` (e.g. a new component's `enabled` flag) | ||
| -it Values changes: show `--set` flags or a patch values file overlay, not edits to `values.yaml` directly, unless there is a change with the default value that should be reflected in `values.yaml` (e.g. a new component's `enabled` flag) |
Contributor
There was a problem hiding this comment.
Suggested change
| -it Values changes: show `--set` flags or a patch values file overlay, not edits to `values.yaml` directly, unless there is a change with the default value that should be reflected in `values.yaml` (e.g. a new component's `enabled` flag) | |
| - Values changes: show `--set` flags or a patch values file overlay, not edits to `values.yaml` directly, unless there is a change with the default value that should be reflected in `values.yaml` (e.g. a new component's `enabled` flag) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Description
Adds a top-level
telemetry:block to the umbrella chart so any service in the bundle can read the same OpenTelemetry producer-side config. Today it wiresMLRUN_TELEMETRY__*env vars into mlrun-api via the existingmlrun-common-envConfigMap; Nuclio and other services are left for follow-up.Mirrors the producer side of the MLRun PR (mlrun/mlrun#9668) and the analogous mlefi PR (IG4-2293, #532) on the IG4 side.
🛠️ Changes Made
values.yamltelemetry:block (enabled/otlpEndpoint/insecure/headersSecretName), sibling tomlrun:/nuclio:templates/config/mlrun-env-configmap.yamlMLRUN_TELEMETRY__*keys with resolution + safety logicREADME.mdChart.yaml0.11.0-rc.38✅ Checklist
charts/mlrun-ce/Chart.yaml.🧪 Testing
Live cluster verification on
app1.vmdev232ig4.lab.iguaz.io:- Test 1 (no OTel):
mlrun-common-envConfigMap showsMLRUN_TELEMETRY__ENABLED: "false"- Test 2 (OTel enabled): ConfigMap shows
ENABLED: "true",OTLP_ENDPOINT: otel-collector.mlrun.svc.cluster.local:4317- Probe pod with same
envFrom: configMapRef: mlrun-common-envreceives all three env vars at runtime🔗 References
🚨 Breaking Changes?
🔍️ Additional Notes