Skip to content

fix(demos): harden the OTel benchmark demo after review - #24

Draft
Ladas wants to merge 1 commit into
praxis-proxy:mainfrom
Ladas:feat/otel-benchmark-followup
Draft

fix(demos): harden the OTel benchmark demo after review#24
Ladas wants to merge 1 commit into
praxis-proxy:mainfrom
Ladas:feat/otel-benchmark-followup

Conversation

@Ladas

@Ladas Ladas commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Why

#13 merged at the state that was pushed; a review pass afterwards found the
issues below. Each is small, each is verified on a live cluster, and none of
them changes what the benchmark measures — they change whether what it
reports can be trusted, and whether the demo still works when a second KIND
cluster is running beside it.

Correctness

  • forge exec steps now name the kubectl context. forge scopes
    manifest, helm and wait steps to the cluster but runs exec with the
    ambient context (execute_exec takes no kube_context, unlike its
    siblings). With a second KIND cluster up, kubectl create namespace otel
    and the dashboard ConfigMaps land on whichever cluster was created last,
    while the properly scoped steps that follow target the right one.
  • Both traced configs sample at 0.1. ai-otel-full.yaml had it;
    otel-full.yaml did not, and the core scenario runs at 2000 RPS — the
    larger of the two firehoses. Unsampled is not the shape the overhead
    question is about.
  • A memory_limiter runs first in the collector pipeline. The container
    is capped at 512Mi and nothing shed load before it: under exporter
    backpressure the queue grew until the kernel killed the collector and every
    buffered span went with it. Refusals now land in
    otelcol_processor_refused_spans, charted beside the export failures.
    Verified: Memory limiter configured in the collector log, pod stable at 0
    restarts, counter present in Prometheus.
  • The Span Export Failures panel filters to exporter="otlp/tempo". The
    pipeline also has a debug exporter, so the unfiltered sum() counted
    every span twice and a debug-exporter hiccup read as a Tempo failure. Both
    exporters confirmed reporting on the live cluster.
  • praxis-ai is pinned to the commit tagged v0.3.0, not to the tag. A
    tag can be force-moved upstream; a commit cannot. The exactness lived only
    in Cargo.lock before this.

Removals

  • The ai-extended scenario referenced ai-extended-baseline.yaml and
    ai-extended-otel-full.yaml, which do not exist, so selecting it aborted on
    the first kubectl create under set -euo pipefail. report.sh had no
    branch for its result prefix either.
  • The per-run kubectl top pod snapshot. No stack installs
    metrics-server and KIND does not ship it, so the call always failed into
    || true and wrote nine empty *-resources.txt files across a 3×3 run.
    The dashboards already plot CPU and memory from cAdvisor.
  • Two comments describing a [patch.crates-io] table that the 0.5.4 bump
    removed — one in Cargo.toml, one in deny.toml, both still explaining a
    patch to praxis main "for otel features not yet in v0.5.3".

Tooling

  • make container FEATURES=otel plumbs the build-arg through, so the
    repo's own tooling can build the image the demos ask for. Both demo READMEs
    hand-rolled docker build --build-arg lines because it could not.
  • A cargo git cache mount alongside the registry one. The ai dependency
    is a git source, so it was re-cloned on every build.
  • shellcheck covers demos/*/scripts/*.sh in make lint and CI, which
    it never did — the glob was hack/*.sh .hooks/pre-commit. The first run
    found a masked exit status in report.sh (SC2155), fixed here.

Test plan

  • make lint, actionlint, shellcheck, taplo, cargo check --locked -p praxis-experimental-server --features otel all clean
  • markdownlint clean against a clean checkout of this branch
  • praxis-forge config validate accepts the edited forge.yaml
  • Collector applied to a live KIND cluster: rollout succeeds, limiter
    configured, otel-collector Prometheus target up
  • Dashboards re-applied; Grafana serves the filtered queries
  • 43 non-exempt added lines, well inside the size limit

@Ladas
Ladas marked this pull request as draft September 4, 2026 09:18
@Ladas
Ladas force-pushed the feat/otel-benchmark-followup branch from aa473b9 to 0ec8139 Compare September 4, 2026 13:50
Follow-ups to praxis-proxy#13, found by a review pass after it merged. Each one is a
defect with an observable consequence; nothing here changes what the
benchmark measures.

Broken or dead:

- The `ai-extended` scenario referenced two config files that do not
  exist, so selecting it aborted on the first `kubectl create` under
  `set -euo pipefail`, and report.sh had no branch for its result prefix.
- The per-run `kubectl top pod` snapshot always failed into `|| true`,
  because no stack installs metrics-server and KIND does not ship it. It
  wrote nine empty `*-resources.txt` files per 3x3 run; the dashboards
  already plot CPU and memory from cAdvisor.
- `export BRANCH=$(...)` in report.sh masked the command's exit status
  (SC2155), which shellcheck never saw because the lint target only
  covered `hack/` and `.hooks/`.
- Two comments described a `[patch.crates-io]` table that the 0.5.4 bump
  had already removed.
- The prerequisites told you to install forge from a feature branch.
  praxis-proxy/forge#16 has merged, so `extraPortMappings` is in main.

Observability of the trace pipeline:

- A `memory_limiter` now runs first in the collector pipeline. The
  container is capped at 512Mi and nothing shed load before it, so under
  exporter backpressure the queue grew until the kernel killed the
  collector and every buffered span went with it. Refusals land in
  `otelcol_processor_refused_spans`, charted beside the export failures.
- The Span Export Failures panel filters to `exporter="otlp/tempo"`. The
  pipeline also has a `debug` exporter, so the unfiltered sum counted
  every span twice and a debug-exporter hiccup read as a Tempo failure.

Supply chain and build:

- praxis-ai is pinned to the commit tagged v0.3.0 rather than to the tag,
  which is what deny.toml's comment already claimed. A tag can be
  force-moved upstream; a commit cannot.
- `make container FEATURES=otel` plumbs the build-arg through, so the
  repo's own tooling can build the image the demo READMEs ask for instead
  of the hand-rolled `docker build` lines they carry today.
- A cargo git cache mount alongside the registry one: the ai dependency is
  a git source and was re-cloned on every image build.

Signed-off-by: Ladislav Smola <lsmola@redhat.com>
@Ladas
Ladas force-pushed the feat/otel-benchmark-followup branch from 0ec8139 to a2ac800 Compare September 7, 2026 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant