Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5072eb4543
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ate its config against the pinned image Add an `otel-ebpf-profiler` golden case built from the `opentelemetry-ebpf-profiler` sub-chart, i.e. the configuration the wizard generates. The existing `ebpf-profiler` case only exercises the legacy `coralogix-ebpf-profiler` sub-chart, so the path that crash-looped in 0.0.333 had no coverage. Add check-collector-config-validity.sh, wired into the helm-golden-render job. For every golden render it extracts the collector config from each ConfigMap and, for configs that declare the `profiling` receiver, runs `otelcol validate` inside the exact profiler image the chart renders, so the check follows every dependency bump with no extra maintenance. Golden diffs cannot catch an upstream config-key removal on their own: the rendered YAML stays byte-identical while the binary starts rejecting it.
350e795 to
1e7a02a
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e7a02ae6e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| esac | ||
| done < <(grep -oE '\$\{env:[A-Za-z_][A-Za-z0-9_]*\}' "$config" | sed "s/\${env://; s/}//" | sort -u) | ||
|
|
||
| CX_COLLECTOR_CONFIG="$(cat "$config")" docker run --rm --entrypoint sh "${env_args[@]}" "$image" -c ' |
There was a problem hiding this comment.
When the new workflow reaches this command, Docker overrides the profiler image's entrypoint with sh, but the official collector release image is built as a minimal shell-less runtime, so the container exits with an executable-not-found error before mkdir or otelcol validate can run. This makes the required helm-golden-render job fail for every PR; run the collector binary directly and supply the service-account fixture through a mounted directory or another container rather than executing shell setup inside this image.
Useful? React with 👍 / 👎.
2 participants
Closes the test gaps that let the profiler crash ship. The crash itself is fixed upstream in coralogix/opentelemetry-helm-charts#497, which landed here via #988 (opentelemetry-collector 0.136.4, chart 0.0.335) — so this PR is now test coverage only, on top of that.
Background
Profiling was broken in otel-integration 0.0.333: the chart rendered
tracers: allon theprofilingreceiver, a key removed from the profiler in v0.156.0, so every profiler pod crash-looped with'config.Config' has invalid keys: tracers. 0.0.333 (#984) is where the image moved to 0.156.0.Two gaps let it through
ebpf-profilercase usesvalues-ebpf-profiler.yaml, which enables the legacycoralogix-ebpf-profilersub-chart — not theopentelemetry-ebpf-profilersub-chart the wizard emits and that users actually run.Changes
otel-ebpf-profilerbuilt fromvalues-otel-ebpf-profiler.yaml, i.e. the wizard configuration from the report (otlpExporter to the node agent, resourceDetection, fleetManagement without supervisor,samplesPerSecond: 20). The legacyebpf-profilercase is left in place; it can be dropped when that sub-chart is retired..github/scripts/check-collector-config-validity.sh, wired into thehelm-golden-renderjob. For every golden render it extracts the collector config from each ConfigMap and, for configs that declare theprofilingreceiver, runsotelcol validateagainst them. Details:ghcr.io/…/opentelemetry-collector-ebpf-profiler:0.156.0, read out of the golden), so the check follows every dependency bump with no extra maintenance and needs no separate binary download.validateunmarshals the config and builds the pipelines — no eBPF probes, no privileges, no cluster. It is the same code path that produced the crash message in production.${env:…}references are stubbed, sincevalidateresolves them. The config is passed in via--config=env:…rather than a bind mount, so it behaves the same in CI and on a laptop whose Docker VM doesn't share$TMPDIR.eksresource detector constructs a Kubernetes client; a stub service-account token is written inside the container so it can. Nothing reads it and no request is made.profilingconfig is found in the goldens, so this coverage cannot silently disappear again.Validation
Both checks run clean locally, and the new one was verified in both directions:
check-helm-golden-renders.sh— clean, including the new case.check-collector-config-validity.sh—OK otel-ebpf-profiler/…configmap_agent_yaml.tracers: allinto the golden makes it fail with exactly the production error:'config.Config' has invalid keys: tracers.shellcheck(v0.10.0, as CI pins) clean on both scripts;helmlog validateandmake check-docsclean.