-
Notifications
You must be signed in to change notification settings - Fork 131
chore(k8s-infra): bump the otel-collector version to 0.139.0 #790
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughConsolidates OTLP endpoint selection to prefer configured Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Helm as Helm render
participant Template as _config.tpl
participant Env as ENV (OTEL_EXPORTER_OTLP_ENDPOINT)
participant Exporter as OTLP exporter
rect rgb(250,250,255)
Note over Template,Env: Endpoint & TLS resolution
Helm->>Template: render with values.presets.selfTelemetry?
alt selfTelemetry.endpoint set
Template->>Template: strip scheme from endpoint
Template->>Exporter: endpoint = selfTelemetry.endpoint (no scheme)
Template->>Exporter: insecure = selfTelemetry.insecure
Template->>Exporter: insecure_skip_verify = selfTelemetry.insecure_skip_verify
else endpoint not provided
Template->>Env: reference ${env:OTEL_EXPORTER_OTLP_ENDPOINT}
Env-->>Template: env endpoint (kept as ${env:...})
Template->>Exporter: endpoint = ${env:OTEL_EXPORTER_OTLP_ENDPOINT}
Template->>Exporter: insecure = otelInsecure / insecure_skip_verify = otelInsecureSkipVerify
end
end
rect rgb(245,255,245)
Note over Template,Exporter: Header update
Template->>Exporter: set header "signoz-ingestion-key": <api-key>
Exporter-->>Helm: emit OTLP exporter block
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (4)📓 Common learnings📚 Learning: 2024-10-09T08:24:52.293ZApplied to files:
📚 Learning: 2024-10-16T19:01:15.514ZApplied to files:
📚 Learning: 2024-11-01T05:29:30.302ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
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 |
7580ed2 to
e0c5651
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
charts/k8s-infra/values.yaml (1)
1-1: Chart.yaml version was not incremented despite substantial changes.The Chart.yaml version remains
0.14.2(unchanged from the previous commit), but the PR includes:
- Major otel-collector version bump (0.109.0 → 0.139.0)
- Configuration changes (removal of metrics address from telemetry service)
Increment the patch version to
0.14.3incharts/k8s-infra/Chart.yamlto reflect these changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
charts/k8s-infra/templates/_config.tpl(3 hunks)charts/k8s-infra/tests/otel-agent_autogke_kubelet_test.yaml(0 hunks)charts/k8s-infra/tests/otel-agent_debug_exporter_test.yaml(0 hunks)charts/k8s-infra/tests/otel-agent_host_metrics_test.yaml(0 hunks)charts/k8s-infra/tests/otel-agent_log_collection_test.yaml(0 hunks)charts/k8s-infra/tests/otel-agent_logs_collection_mulitline_test.yaml(3 hunks)charts/k8s-infra/tests/otel-agent_otlp_exporter_test.yaml(0 hunks)charts/k8s-infra/tests/otel-agent_self_telemetry_enable_otlp_test.yaml(3 hunks)charts/k8s-infra/tests/otel-agent_self_telemetry_test.yaml(3 hunks)charts/k8s-infra/tests/otel-deployment_debug_exporter_test.yaml(0 hunks)charts/k8s-infra/tests/otel-deployment_prometheus_test.yaml(0 hunks)charts/k8s-infra/tests/otel-deployment_self_telemetry_test.yaml(2 hunks)charts/k8s-infra/values.yaml(5 hunks)
💤 Files with no reviewable changes (7)
- charts/k8s-infra/tests/otel-agent_autogke_kubelet_test.yaml
- charts/k8s-infra/tests/otel-agent_debug_exporter_test.yaml
- charts/k8s-infra/tests/otel-deployment_debug_exporter_test.yaml
- charts/k8s-infra/tests/otel-agent_otlp_exporter_test.yaml
- charts/k8s-infra/tests/otel-agent_host_metrics_test.yaml
- charts/k8s-infra/tests/otel-agent_log_collection_test.yaml
- charts/k8s-infra/tests/otel-deployment_prometheus_test.yaml
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: grandwizard28
Repo: SigNoz/charts PR: 501
File: charts/k8s-infra/Chart.yaml:5-5
Timestamp: 2024-10-09T08:24:52.293Z
Learning: When updating the version in `charts/k8s-infra/Chart.yaml`, ensure that the version increment reflects the scope of changes made. For minor changes, increment the patch version (e.g., from `0.11.10` to `0.11.11`).
Learnt from: grandwizard28
Repo: SigNoz/charts PR: 501
File: charts/k8s-infra/Chart.yaml:5-5
Timestamp: 2024-09-17T09:57:12.685Z
Learning: When updating the version in `charts/k8s-infra/Chart.yaml`, ensure that the version increment reflects the scope of changes made. For minor changes, increment the patch version (e.g., from `0.11.10` to `0.11.11`).
📚 Learning: 2025-01-17T05:22:11.248Z
Learnt from: nityanandagohain
Repo: SigNoz/charts PR: 595
File: charts/k8s-infra/tests/otel-agent_self_telemetry_enable_otlp_test.yaml:9-10
Timestamp: 2025-01-17T05:22:11.248Z
Learning: In test files like `*_test.yaml`, simplified endpoint formats without HTTPS protocol are acceptable as they are used for testing and validation purposes only.
Applied to files:
charts/k8s-infra/tests/otel-agent_logs_collection_mulitline_test.yamlcharts/k8s-infra/tests/otel-agent_self_telemetry_test.yaml
📚 Learning: 2024-09-17T09:57:12.685Z
Learnt from: grandwizard28
Repo: SigNoz/charts PR: 501
File: charts/k8s-infra/Chart.yaml:5-5
Timestamp: 2024-09-17T09:57:12.685Z
Learning: When updating the version in `charts/k8s-infra/Chart.yaml`, ensure that the version increment reflects the scope of changes made. For minor changes, increment the patch version (e.g., from `0.11.10` to `0.11.11`).
Applied to files:
charts/k8s-infra/values.yaml
📚 Learning: 2024-10-16T19:01:15.514Z
Learnt from: grandwizard28
Repo: SigNoz/charts PR: 0
File: :0-0
Timestamp: 2024-10-16T19:01:15.514Z
Learning: When updating the version in `charts/k8s-infra/Chart.yaml`, use version `0.11.11` instead of `0.11.20` as per the user's guidance.
Applied to files:
charts/k8s-infra/values.yaml
🪛 GitHub Actions: Lint Charts
charts/k8s-infra/values.yaml
[error] 137-137: Trailing spaces detected (trailing-spaces).
🪛 GitHub Check: lint-chart
charts/k8s-infra/values.yaml
[failure] 137-137:
137:122 [trailing-spaces] trailing spaces
[failure] 1010-1010:
1010:126 [trailing-spaces] trailing spaces
[failure] 1404-1404:
1404:126 [trailing-spaces] trailing spaces
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test-chart
🔇 Additional comments (9)
charts/k8s-infra/tests/otel-agent_logs_collection_mulitline_test.yaml (1)
35-35: Consistent endpoint and header updates across test cases.The self-telemetry exporter configuration has been correctly updated across all three test cases with:
- Simplified endpoint format (
test-endpoint:4317without scheme) — appropriate for test validation- Standardized header key (
signoz-ingestion-key) — consistent with PR objectives- Proper TLS configuration with insecure settings for testing
These changes align well with the broader self-telemetry standardization in this PR.
Also applies to: 45-45, 47-49
charts/k8s-infra/tests/otel-agent_self_telemetry_test.yaml (1)
49-51: LGTM! Self-telemetry endpoint and header updates are consistent.The endpoint format changes (removing
http://prefix) and header rename (signoz-access-token→signoz-ingestion-key) align with the template refactoring in_config.tpl. The simplified endpoint format is acceptable in test files.Also applies to: 110-112, 127-129
charts/k8s-infra/tests/otel-deployment_self_telemetry_test.yaml (1)
74-77: LGTM! Self-telemetry configuration correctly updated.The endpoint now uses the environment variable directly without the
http://prefix, and the header has been standardized tosignoz-ingestion-key. The explicitinsecure: truesetting is appropriate for the test scenario.Also applies to: 91-94
charts/k8s-infra/tests/otel-agent_self_telemetry_enable_otlp_test.yaml (1)
58-60: LGTM! Dual exporter test configuration updated correctly.The test properly validates self-telemetry alongside the regular OTLP exporter. Endpoint and header changes are consistent with the template refactoring.
Also applies to: 140-142, 157-159
charts/k8s-infra/templates/_config.tpl (3)
220-225: LGTM! Simplified endpoint configuration logic.The consolidated endpoint selection (provided endpoint or environment variable) and conditional TLS settings improve maintainability. The header standardization to
signoz-ingestion-keyis consistent across all self-telemetry configurations.
241-245: LGTM! Self-traces configuration matches the standardized pattern.The endpoint and header changes are consistent with the self-telemetry exporter configuration.
265-269: LGTM! Self-metrics configuration follows the same pattern.The configuration is consistent with the other self-telemetry sections.
charts/k8s-infra/values.yaml (2)
590-590: LGTM! OtelAgent image version correctly updated.The version bump from
0.109.0to0.139.0aligns with the PR objective.
1072-1072: LGTM! OtelDeployment image version correctly updated.The version bump from
0.109.0to0.139.0is consistent with the otelAgent update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/k8s-infra/values.yaml (1)
1009-1013: Refactor: Remove or consolidate duplicate documentation comments.Documentation comments explaining self-telemetry are now duplicated in three places:
- Lines 137-139 (presets.selfTelemetry section)
- Lines 1009-1013 (otelAgent service section)
- Lines 1403-1407 (otelDeployment service section)
These comments are identical and create maintenance overhead. Consider:
- Keeping the documentation only in the presets section, or
- Making references in the service sections point back to presets (e.g., "See presets.selfTelemetry for details")
This reduces duplication and improves maintainability.
Also applies to: 1403-1407
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
charts/k8s-infra/values.yaml(5 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: grandwizard28
Repo: SigNoz/charts PR: 501
File: charts/k8s-infra/Chart.yaml:5-5
Timestamp: 2024-10-09T08:24:52.293Z
Learning: When updating the version in `charts/k8s-infra/Chart.yaml`, ensure that the version increment reflects the scope of changes made. For minor changes, increment the patch version (e.g., from `0.11.10` to `0.11.11`).
Learnt from: grandwizard28
Repo: SigNoz/charts PR: 501
File: charts/k8s-infra/Chart.yaml:5-5
Timestamp: 2024-09-17T09:57:12.685Z
Learning: When updating the version in `charts/k8s-infra/Chart.yaml`, ensure that the version increment reflects the scope of changes made. For minor changes, increment the patch version (e.g., from `0.11.10` to `0.11.11`).
Learnt from: grandwizard28
Repo: SigNoz/charts PR: 0
File: :0-0
Timestamp: 2024-10-16T19:01:15.514Z
Learning: When updating the version in `charts/k8s-infra/Chart.yaml`, use version `0.11.11` instead of `0.11.20` as per the user's guidance.
Learnt from: grandwizard28
Repo: SigNoz/charts PR: 0
File: :0-0
Timestamp: 2024-09-17T09:58:12.154Z
Learning: When updating the version in `charts/k8s-infra/Chart.yaml`, use version `0.11.11` instead of `0.11.20` as per the user's guidance.
📚 Learning: 2024-10-09T08:24:52.293Z
Learnt from: grandwizard28
Repo: SigNoz/charts PR: 501
File: charts/k8s-infra/Chart.yaml:5-5
Timestamp: 2024-10-09T08:24:52.293Z
Learning: When updating the version in `charts/k8s-infra/Chart.yaml`, ensure that the version increment reflects the scope of changes made. For minor changes, increment the patch version (e.g., from `0.11.10` to `0.11.11`).
Applied to files:
charts/k8s-infra/values.yaml
📚 Learning: 2024-10-16T19:01:15.514Z
Learnt from: grandwizard28
Repo: SigNoz/charts PR: 0
File: :0-0
Timestamp: 2024-10-16T19:01:15.514Z
Learning: When updating the version in `charts/k8s-infra/Chart.yaml`, use version `0.11.11` instead of `0.11.20` as per the user's guidance.
Applied to files:
charts/k8s-infra/values.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test-chart
🔇 Additional comments (3)
charts/k8s-infra/values.yaml (3)
590-590: Image tag upgrade to 0.139.0 looks good.Both OtelAgent and OtelDeployment image tags have been correctly updated from 0.109.0 to 0.139.0, aligning with the PR objective. Ensure the new image version has been tested in your environment and is compatible with your deployment.
Also applies to: 1072-1072
141-177: Self-telemetry presets structure is well-designed.The new
presets.selfTelemetryconfiguration block provides a clean abstraction for sending collector internal telemetry, with sensible defaults (all telemetry disabled by default) and flexibility for endpoint/API key configuration. This enables self-monitoring without cluttering the main agent/deployment configs.
137-139: I can see the trailing spaces have been fixed - all three lines end cleanly with no trailing whitespace before the line terminator ($). Let me verify the remaining concerns:Trailing spaces have been fixed — verification successful.
The script output confirms that lines 137, 1010, and 1404 no longer contain trailing whitespace. The
cat -Aoutput shows each line ending with$(the line terminator) with no trailing spaces before it, resolving the critical linting failures flagged in previous reviews.The image tag update to 0.139.0 is a valid, recently released version of OpenTelemetry Collector (released November 4, 2025), making this a straightforward patch version bump for the chart.
Related: https://github.com/SigNoz/platform-pod/issues/1304
Summary by CodeRabbit
Chores
New Features