feat(product-console): add dynamic OTEL host injection support#1156
Merged
ferr3ira-gabriel merged 1 commit intodevelopfrom Apr 1, 2026
Merged
Conversation
|
Caution Review failedPull request was closed or merged during review Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe Helm chart adds an Sequence Diagram(s)sequenceDiagram
participant Helm as Helm template
participant K8sAPI as Kubernetes API
participant Pod as Pod/Container
participant OTEL as OpenTelemetry Collector
Helm->>K8sAPI: Render Deployment (check ENABLE_TELEMETRY vs .Values.otel)
alt validation fails
Helm-->>K8sAPI: Fail rendering with error
else validation passes
Helm->>K8sAPI: Apply Deployment with conditional env vars
K8sAPI->>Pod: Schedule Pod and start container
alt otel.host provided
K8sAPI->>Pod: Inject OTEL_HOST and OTEL_URL_* from otel.host:otel.port
Pod->>OTEL: Send metrics/traces/logs to OTEL_URL_* (host:port)
else use hostIP
Pod->>Pod: Resolve HOST_IP from status.hostIP
K8sAPI->>Pod: Inject OTEL_HOST as $(HOST_IP) and OTEL_URL_* using $(HOST_IP):port
Pod->>OTEL: Send metrics/traces/logs to OTEL_URL_* ($(HOST_IP):port)
end
end
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/product-console/templates/deployment.yaml`:
- Around line 53-67: The env: line is rendered even when .Values.otel.enabled is
false, producing an invalid env: null; wrap the whole env block with the
conditional instead of just its items: move the {{- if .Values.otel.enabled }} /
{{- end }} to surround the env: key and its indented OTEL_* entries (including
the HOST_IP fieldRef and OTEL_URL_* values) so that env: is omitted entirely
when otel is disabled.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4837aa67-cc9d-4c4e-8b16-18a1666c8f92
📒 Files selected for processing (2)
charts/product-console/templates/deployment.yamlcharts/product-console/values.yaml
ferr3ira-gabriel
force-pushed
the
feat/product-console-otel-host-injection
branch
from
d9bc657 to
128bbd5
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/product-console/templates/deployment.yaml`:
- Around line 53-67: The deployment template can allow ENABLE_TELEMETRY=true
while no exporter endpoints are set (OTEL_HOST/OTEL_URL_*), so add a fail-fast
check in the pod start-up: when configmap.ENABLE_TELEMETRY is "true" and both
.Values.otel.enabled and .Values.otel.external are false, exit with a clear
error. Implement this by adding a small initContainer or an entrypoint script
invoked in the Deployment template (referencing OTEL_HOST, OTEL_URL_METRICS,
OTEL_URL_TRACES, OTEL_URL_LOGS, and configmap.ENABLE_TELEMETRY) that validates
the variables and fails the pod start if endpoints are missing, returning a
descriptive error message about telemetry being enabled without configured
exporters.
In `@charts/product-console/values.yaml`:
- Around line 155-156: The comment above ENABLE_TELEMETRY mentions only
"otel.enabled=true" for dynamic injection of OTEL_HOST and OTEL_URL_*; update
that comment to also mention that "otel.external=true" triggers the same env
injection. Edit the existing note text to say: when otel.enabled=true or
otel.external=true, OTEL_HOST and OTEL_URL_* are injected dynamically from
HOST_IP so maintainers see both flags that cause injection.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d620ec2f-46a9-4664-a201-5f9fe7aca4e5
📒 Files selected for processing (2)
charts/product-console/templates/deployment.yamlcharts/product-console/values.yaml
ferr3ira-gabriel
force-pushed
the
feat/product-console-otel-host-injection
branch
from
128bbd5 to
7ee7ad5
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
charts/product-console/values.yaml (1)
157-157:⚠️ Potential issue | 🟡 MinorUpdate the OTEL note to include
otel.external=true.Line 157 mentions only
otel.enabled=true, but deployment env injection also occurs whenotel.external=true.Suggested patch
- # Note: When otel.enabled=true, OTEL_HOST and OTEL_URL_* are injected dynamically from HOST_IP + # Note: When otel.enabled=true or otel.external=true, OTEL_HOST and OTEL_URL_* are injected dynamically🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/product-console/values.yaml` at line 157, Update the inline comment that currently references only otel.enabled to also mention otel.external; specifically, modify the note that reads "When otel.enabled=true, OTEL_HOST and OTEL_URL_* are injected dynamically from HOST_IP" so it states that injection happens when either otel.enabled=true or otel.external=true, keeping the reference to OTEL_HOST, OTEL_URL_* and HOST_IP intact to clarify both flags trigger dynamic env injection.charts/product-console/templates/deployment.yaml (1)
53-78:⚠️ Potential issue | 🟠 MajorAdd a template fail-fast for telemetry toggle mismatch.
Line 53 currently allows rendering with
configmap.ENABLE_TELEMETRY="true"while bothotel.enabledandotel.externalare false, leaving OTEL exporter endpoints unset.Suggested patch
+ {{- $telemetryEnabled := eq (toString (default "false" .Values.configmap.ENABLE_TELEMETRY)) "true" -}} + {{- if and $telemetryEnabled (not (or .Values.otel.enabled .Values.otel.external)) -}} + {{- fail "configmap.ENABLE_TELEMETRY=true requires otel.enabled=true or otel.external=true" -}} + {{- end }} {{- if or .Values.otel.enabled .Values.otel.external }} env: {{- if .Values.otel.host }} - name: OTEL_HOST#!/bin/bash set -euo pipefail chart="charts/product-console" # Current behavior check: should render successfully but with telemetry=true and no OTEL env block. helm template pc "$chart" \ --set configmap.ENABLE_TELEMETRY=true \ --set otel.enabled=false \ --set otel.external=false >/tmp/pc.yaml echo "Rendered with ENABLE_TELEMETRY=true and otel flags false." echo "Expected after fix: helm template fails with explicit message." rg -n -C2 'ENABLE_TELEMETRY|OTEL_URL_|OTEL_HOST|^\s*env:' /tmp/pc.yaml || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/product-console/templates/deployment.yaml` around lines 53 - 78, Add a Helm template guard in the deployment.yaml before the OTEL env block that fails rendering when telemetry is enabled in the configmap but no OTEL exporter is configured: check configmap.ENABLE_TELEMETRY (use toString .Values.configmap.ENABLE_TELEMETRY to normalize) and if it equals "true" while both .Values.otel.enabled and .Values.otel.external are false, call the Helm fail function with a clear message; reference the existing OTEL env block and the symbols .Values.otel.enabled, .Values.otel.external and .Values.configmap.ENABLE_TELEMETRY so the template aborts instead of rendering empty OTEL endpoints.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@charts/product-console/templates/deployment.yaml`:
- Around line 53-78: Add a Helm template guard in the deployment.yaml before the
OTEL env block that fails rendering when telemetry is enabled in the configmap
but no OTEL exporter is configured: check configmap.ENABLE_TELEMETRY (use
toString .Values.configmap.ENABLE_TELEMETRY to normalize) and if it equals
"true" while both .Values.otel.enabled and .Values.otel.external are false, call
the Helm fail function with a clear message; reference the existing OTEL env
block and the symbols .Values.otel.enabled, .Values.otel.external and
.Values.configmap.ENABLE_TELEMETRY so the template aborts instead of rendering
empty OTEL endpoints.
In `@charts/product-console/values.yaml`:
- Line 157: Update the inline comment that currently references only
otel.enabled to also mention otel.external; specifically, modify the note that
reads "When otel.enabled=true, OTEL_HOST and OTEL_URL_* are injected dynamically
from HOST_IP" so it states that injection happens when either otel.enabled=true
or otel.external=true, keeping the reference to OTEL_HOST, OTEL_URL_* and
HOST_IP intact to clarify both flags trigger dynamic env injection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 15fc0f7d-9713-42df-8ad2-11315768d3f5
📒 Files selected for processing (2)
charts/product-console/templates/deployment.yamlcharts/product-console/values.yaml
ferr3ira-gabriel
force-pushed
the
feat/product-console-otel-host-injection
branch
from
7ee7ad5 to
630312e
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
charts/product-console/values.yaml (1)
157-157:⚠️ Potential issue | 🟡 MinorUpdate the OTEL note to include
otel.external=true.The comment documents only
otel.enabled=true, but env injection is also triggered byotel.external=true(see deployment template line 56:{{- if or .Values.otel.enabled .Values.otel.external }}).Suggested fix
- # Note: When otel.enabled=true, OTEL_HOST and OTEL_URL_* are injected dynamically from HOST_IP + # Note: When otel.enabled=true or otel.external=true, OTEL_HOST and OTEL_URL_* are injected dynamically🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/product-console/values.yaml` at line 157, The comment about dynamic injection currently only mentions otel.enabled=true but the injection is also triggered when otel.external=true; update the note in values.yaml to state that OTEL_HOST and OTEL_URL_* are injected when either otel.enabled=true or otel.external=true (i.e., reflect the template condition that checks or .Values.otel.enabled .Values.otel.external), so readers know both flags cause env injection.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@charts/product-console/values.yaml`:
- Line 157: The comment about dynamic injection currently only mentions
otel.enabled=true but the injection is also triggered when otel.external=true;
update the note in values.yaml to state that OTEL_HOST and OTEL_URL_* are
injected when either otel.enabled=true or otel.external=true (i.e., reflect the
template condition that checks or .Values.otel.enabled .Values.otel.external),
so readers know both flags cause env injection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7af849b7-9da8-4e8a-bce3-234e52ff1e29
📒 Files selected for processing (2)
charts/product-console/templates/deployment.yamlcharts/product-console/values.yaml
Add support for dynamic OpenTelemetry configuration. When otel.enabled=true OR otel.external=true, the chart injects OTEL_URL_* env vars: - If otel.host is set: uses static host (e.g., ClusterIP Service) - If otel.host is empty: injects HOST_IP from pod's node (for DaemonSet with hostPort) Also adds fail-fast validation: if ENABLE_TELEMETRY is 'true' but neither otel.enabled nor otel.external is set, helm template fails with a clear error. This matches the pattern used by midaz chart and supports both: - External OTEL collector with ClusterIP Service - External OTEL collector DaemonSet with hostPort
ferr3ira-gabriel
force-pushed
the
feat/product-console-otel-host-injection
branch
from
630312e to
4f473e9
Compare
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
1 participant
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.
Summary
Add support for dynamic OpenTelemetry configuration using the node's HOST_IP for product-console helm chart.
Problem
The OTEL collector in firmino runs as a DaemonSet with hostPorts (no ClusterIP Service), so apps need to connect via the node's IP address. The current product-console chart only supports static configmap values.
Solution
When
otel.enabled=true, the chart now injects:HOST_IPfrom pod'sstatus.hostIP(fieldRef)OTEL_HOST=$(HOST_IP)OTEL_URL_METRICS=http://$(HOST_IP):<port>/v1/metricsOTEL_URL_TRACES=http://$(HOST_IP):<port>/v1/tracesOTEL_URL_LOGS=http://$(HOST_IP):<port>/v1/logsThis matches the pattern used by other services (midaz, flowker, etc.).
Usage
Testing
otel.enabled=trueand verify env vars are injected correctly