Skip to content

feat(performance-test-server): add OTel OTLP export overhead scenario#15

Open
intech wants to merge 1 commit into
mainfrom
feat/otel-export-benchmark-scenario
Open

feat(performance-test-server): add OTel OTLP export overhead scenario#15
intech wants to merge 1 commit into
mainfrom
feat/otel-export-benchmark-scenario

Conversation

@intech

@intech intech commented Apr 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds 6th server configuration (port 8085) with full OTel tracing + metrics and OTLP gRPC exporter enabled
  • Adds OTel collector docker-compose service (profile: otel-export)
  • Adds k6 scenario otel-export-overhead.js measuring p50/p95/p99 latency delta vs baseline under 100 VUs / ~5 min sustained load
  • Updates README with expected overhead ranges and links to upstream OTel-JS issues

Motivation

Currently we cannot answer "how much CPU does @connectum/otel cost in production" — this blocks informed decisions on OTel SDK version pins and exporter configuration tuning.

The existing interceptor-overhead.js scenario runs the OTel interceptor with the provider uninitialized (no-op spans/metrics), which only measures interceptor wiring cost. This PR fills the gap by running a real BatchSpanProcessor + @opentelemetry/otlp-transformer + OTLP/gRPC pipeline against a local collector.

Scenario is fully opt-in:

  • Port 8085 is bound only when OTEL_EXPORT_ENABLED=1
  • otel-collector service lives under the otel-export docker-compose profile
  • All existing scenarios and default compose runs are untouched

Smoke test (local)

  • docker compose --profile otel-export up -d otel-collector → collector healthy
  • Server started with OTEL_EXPORT_ENABLED=1 and real OTLP endpoint → 6 servers bound including :8085
  • 10 SayHello requests at :8085 → collector shows spans with rpc.system=connect_rpc, rpc.service=greeter.v1.GreeterService, rpc.method=SayHello
  • Quick 20-sample latency comparison: baseline p95 ~1.87ms, otel-export p95 ~2.25ms (+20%, in line with the expected-overhead table in the README)

Test plan

  • pnpm install succeeds with refreshed lockfile
  • pnpm typecheck passes
  • docker compose --profile otel-export up --build brings up server + otel-collector with all health checks green
  • All 6 servers start when OTEL_EXPORT_ENABLED=1
  • k6 otel-export-overhead.js runs for ~5 minutes and produces JSON results in k6/results/otel-export-overhead.json
  • p95 overhead from baseline (port 8081) → otel-export (port 8085) documented in run output / CI bench tracking
  • Existing k6-interceptor-overhead and k6-basic-load scenarios still pass

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added port 8085 for OpenTelemetry Protocol export functionality
    • New Docker Compose profile (otel-export) enabling optional export benchmarking scenarios
    • Added configuration options via environment variables for controlling export behavior
    • New performance measurement script for comparing baseline versus export configurations
  • Documentation

    • Updated README documenting new port, environment variables, and benchmarking instructions

Adds a 6th server configuration (port 8085) with full interceptor chain and
a real OTLP/gRPC exporter pointed at a local OTel collector, plus a k6
scenario that measures p50/p95/p99 latency and throughput delta between the
baseline (port 8081) and otel-export configurations under 100 VUs sustained
load.

Motivation: the existing interceptor-overhead scenario runs the OTel
interceptor with the provider uninitialized (no-op spans/metrics), so it
cannot answer "what is the CPU cost of actually shipping spans in
production". This closes that gap and gives us an end-to-end harness for
validating future @opentelemetry/otlp-transformer bumps (R1.2).

The OTel scenario is opt-in:
  - port 8085 is bound only when OTEL_EXPORT_ENABLED=1
  - the otel-collector service lives under the "otel-export" docker-compose
    profile, so default compose runs are unaffected
  - all other servers/ports and existing scenarios work unchanged

Files:
  - src/index.ts: +6th server, conditional on OTEL_EXPORT_ENABLED, eager
    initProvider + graceful shutdownProvider
  - k6/otel-export-overhead.js: new scenario (100 VUs, ~5 min, shuffled
    baseline vs otel-export calls, JSON summary via K6_OUT)
  - docker-compose.yml: otel-collector service (profile: otel-export),
    k6-otel-export runner, OTEL_* env var surface with production-ish
    BatchSpanProcessor defaults
  - otel-collector-config.yaml: OTLP gRPC+HTTP receivers -> debug exporter
    (the goal is export-side CPU measurement, not backend write throughput)
  - Dockerfile: EXPOSE 8085
  - README.md: scenario docs, env-var reference, expected overhead table,
    links to upstream issues (#6221, #6225, #6390, #6570)

Smoke test: 10 requests to :8085 verified spans arrive at collector with
correct rpc.system / rpc.service / rpc.method attributes. Quick 20-sample
comparison on the same host: baseline p95 ~1.87ms, otel-export p95 ~2.25ms
(+20%), in line with the expected-overhead range documented in the README.

pnpm-lock.yaml is updated to resolve pre-existing drift between manifest
(1.0.0-rc.10) and lockfile (1.0.0-rc.7) introduced by the earlier rc.10
bump; install now succeeds with --frozen-lockfile.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Apr 19, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR extends the performance-test-server to benchmark OpenTelemetry OTLP export overhead by adding a new port 8085 server instance, OTel Collector infrastructure, environment-based configuration, and a K6 benchmark script that compares baseline latency/throughput against end-to-end OTel export behavior.

Changes

Cohort / File(s) Summary
Docker & Orchestration
performance-test-server/Dockerfile, performance-test-server/docker-compose.yml
Added port 8085 to Dockerfile EXPOSE. Extended docker-compose to include OTel environment variables on server service (OTEL_EXPORT_ENABLED, OTEL_SERVICE_NAME, OTEL_EXPORTER, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_BSP tuning parameters). Introduced otel-collector and k6-otel-export services under otel-export profile with health checks and dependency declarations.
Configuration & Ignores
performance-test-server/otel-collector-config.yaml, performance-test-server/k6/results/.gitignore
Added OTel Collector YAML config defining gRPC (4317) and HTTP (4318) receivers, batch processor (1s timeout, 1024 batch size), debug exporter, and three telemetry pipelines (traces, metrics, logs). Added .gitignore to exclude JSON result files while tracking the ignore configuration itself.
Documentation & Test Script
performance-test-server/README.md, performance-test-server/k6/otel-export-overhead.js
Expanded README with new "OTel OTLP Export Overhead" test section documenting port 8085, otel-export Docker Compose profile, OTEL_* controls, and k6 output path. Added new k6 script implementing ramping-VU scenario (ramp to 100 VUs, 4-min hold, ramp down) with dual-endpoint comparison (baseline 8081 vs OTel-export 8085), custom Trend/Rate metrics, p95/success-rate thresholds, and health check setup/teardown logic.
Server Initialization
performance-test-server/src/index.ts
Updated server startup to conditionally initialize OTel provider (initProvider/shutdownProvider) when OTEL_EXPORT_ENABLED=1, dynamically build servers array to include port 8085 with OTel export options, and register graceful shutdown to flush OTel provider.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • feat: Docker benchmarks + examples cleanup #5: Modifies overlapping performance-test-server artifacts (Dockerfile, docker-compose, README, src/index.ts, k6 scripts) to extend benchmarking and server ports; this PR specifically adds port 8085/OTEL export and runtime OTel provider initialization/shutdown logic.

Poem

🐰 Port 8085 hops into the light,
OTel spans trace through the night,
Collector gathers, K6 runs the test,
Overhead measured—now let's see the best! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: adding a new OTel OTLP export overhead benchmarking scenario with a 6th server configuration, k6 test script, and collector infrastructure.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/otel-export-benchmark-scenario

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the type:feature New feature or enhancement request label Apr 19, 2026
@intech intech self-assigned this Apr 19, 2026
@intech

intech commented Apr 19, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Apr 19, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
performance-test-server/src/index.ts (1)

192-230: ⚠️ Potential issue | 🟠 Major

Update port labels to reflect that ports 8084/8080 use real OTLP exporter when OTEL_EXPORT_ENABLED=1.

When OTEL_EXPORT_ENABLED=1, the initProvider() call (line 198) sets up a process-global OTel provider with real OTLP exporters. All three createOtelInterceptor() instances (ports 8084, 8080, 8085) share this global provider and will export traces/metrics to the configured collector, making the "no-op exporter" labels on ports 8084 and 8080 inaccurate. This misleads users and can skew the interceptor-overhead benchmark if it runs in the same compose invocation.

Either update the log labels (lines 227–228) to indicate real export is active, or isolate the export scenario to port 8085 alone by deferring initProvider() until after 8084/8080 serve requests in a separate process.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@performance-test-server/src/index.ts` around lines 192 - 230, The port labels
are inaccurate when otelExportEnabled is true because initProvider() activates a
real OTLP exporter used by all createOtelInterceptor() instances; update the
printed labels in the server summary (the console.log lines that print the port
table for 8084 and 8080) to reflect that they use the real OTLP exporter when
otelExportEnabled is true (e.g., conditionally print "OTel (tracing + metrics) —
real OTLP exporter" for 8084 and "Full chain (all interceptors) — real OTLP
exporter" for 8080), using the otelExportEnabled flag to decide which label to
show.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@performance-test-server/docker-compose.yml`:
- Around line 93-96: The server healthcheck currently only verifies port 8080
but the k6 job (command: run /scripts/otel-export-overhead.js) needs port 8085
when OTEL_EXPORT_ENABLED=1; update the server healthcheck (or add a new
healthcheck probe used by docker-compose) to verify port 8085 as well when
OTEL_EXPORT_ENABLED is set, or make the k6 service depend_on a new healthcheck
target that checks both ports; specifically modify the server healthcheck block
(and/or the depends_on entry referenced by the k6 service) so it conditionally
checks port 8085 when OTEL_EXPORT_ENABLED=1 to ensure the benchmark waits for
8085 to be bound before starting.
- Around line 46-51: The current healthcheck using test: ["CMD",
"/otelcol-contrib", "components"] doesn't verify OTLP receiver readiness (and
the scratch-based otel image has no curl/wget), so replace it with one of the
viable options: either build a custom collector image that installs curl and
change the healthcheck to curl -f http://localhost:13133/ or implement a
separate tiny sidecar healthcheck service (e.g., an Alpine container with curl)
that probes http://collector:13133/health (or the receiver endpoint) and signals
readiness via a shared volume or by using depends_on with condition:
service_healthy for k6-otel-export; alternatively add a comment documenting that
the current check only validates binary availability and not receiver readiness.
Update the docker-compose healthcheck entry (symbol: healthcheck and test) and
any k6-otel-export startup gating (symbol: service_healthy) accordingly.

In `@performance-test-server/k6/otel-export-overhead.js`:
- Around line 130-163: The current test harness builds a paired testCases array
and always executes both entries per iteration (see testCases and the for...of
loop calling testCase.run()), which couples baseline and OTEL_EXPORT request
counts so you cannot measure independent throughput delta; either change the
design to run each config in separate scenarios/runs (e.g., split into two
independent loops/scenarios that call callService(BASELINE_PORT, ...) and
callService(OTEL_EXPORT_PORT, ...) separately so
baselineDuration/baselineSuccess and otelExportDuration/otelExportSuccess are
accumulated independently), or remove any claim about measuring throughput delta
and document that this test only reports latency deltas. Ensure the Fisher‑Yates
shuffle and sleep placement are adjusted accordingly if you split runs so each
scenario preserves jitter reduction and batching characteristics.

In `@performance-test-server/README.md`:
- Around line 102-105: The docker compose command in the README starts
unprofiled services too; change the example to explicitly target the
k6-otel-export service by adding its service name to the up command (i.e., run
docker compose --profile otel-export up k6-otel-export --build
--abort-on-container-exit with OTEL_EXPORT_ENABLED=1) so the default benchmark
(k6-interceptor-overhead) does not run concurrently and skew results.

---

Outside diff comments:
In `@performance-test-server/src/index.ts`:
- Around line 192-230: The port labels are inaccurate when otelExportEnabled is
true because initProvider() activates a real OTLP exporter used by all
createOtelInterceptor() instances; update the printed labels in the server
summary (the console.log lines that print the port table for 8084 and 8080) to
reflect that they use the real OTLP exporter when otelExportEnabled is true
(e.g., conditionally print "OTel (tracing + metrics) — real OTLP exporter" for
8084 and "Full chain (all interceptors) — real OTLP exporter" for 8080), using
the otelExportEnabled flag to decide which label to show.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9df8157e-8d18-456b-a548-553430b360c6

📥 Commits

Reviewing files that changed from the base of the PR and between dd1c27c and d636756.

⛔ Files ignored due to path filters (1)
  • performance-test-server/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (7)
  • performance-test-server/Dockerfile
  • performance-test-server/README.md
  • performance-test-server/docker-compose.yml
  • performance-test-server/k6/otel-export-overhead.js
  • performance-test-server/k6/results/.gitignore
  • performance-test-server/otel-collector-config.yaml
  • performance-test-server/src/index.ts

Comment on lines +46 to +51
healthcheck:
test: ["CMD", "/otelcol-contrib", "components"]
interval: 5s
timeout: 3s
retries: 5
start_period: 5s

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect whether the collector image has a usable HTTP client for healthchecks.
docker run --rm --entrypoint sh otel/opentelemetry-collector-contrib:0.120.0 -c 'command -v wget || command -v curl || command -v nc || true'

Repository: Connectum-Framework/examples

Length of output: 118


🏁 Script executed:

cat performance-test-server/docker-compose.yml

Repository: Connectum-Framework/examples

Length of output: 4198


🌐 Web query:

OTel opentelemetry-collector-contrib 0.120.0 health check endpoint tools available

💡 Result:

In opentelemetry-collector-contrib version 0.120.0, the primary tool for health checks is the Health Check Extension (extension/healthcheckextension), which exposes an HTTP endpoint for probing the Collector's status. This is suitable for liveness/readiness probes in Kubernetes. Key details: - Default endpoint: localhost:13133 (configurable via 'endpoint' parameter, e.g., 0.0.0.0:13133 for external access). - Default path: / (configurable). - Response: 200 OK if healthy, 503 Service Unavailable if unhealthy. Custom response bodies possible (e.g., healthy: "OK", unhealthy: "FAILED"). - Configuration example: extensions: health_check: endpoint: 0.0.0.0:13133 path: /health Feature gate 'extension.healthcheck.useComponentStatus' (default: false) enables new component status-based reporting for migration. Additional tools: - zPages Extension (extension/zpagesextension): Provides debugging endpoints including servicez (overall health/version/uptime) and extensionz (extension status). Default endpoint: localhost:55679. Routes: /debug/servicez, /debug/extensionz. - Internal telemetry: Prometheus metrics at :8888/metrics (configurable), including process CPU/memory/uptime. Health Check V2 (healthcheckv2extension) exists but is in 'development' stability, not recommended for production. No version-specific changes for 0.120.0 in health checks per release notes; standard extension available in contrib distribution.

Citations:


🏁 Script executed:

#!/bin/bash
# Search for health_check extension configuration in the repo
rg "health_check" --type yaml

Repository: Connectum-Framework/examples

Length of output: 341


🏁 Script executed:

cat performance-test-server/otel-collector-config.yaml

Repository: Connectum-Framework/examples

Length of output: 1387


🌐 Web query:

otel/opentelemetry-collector-contrib 0.120.0 alpine image wget curl busybox tools available

💡 Result:

The official otel/opentelemetry-collector-contrib 0.120.0 Docker image is based on scratch, not a full Alpine image with package manager tools. It copies only ca-certificates from an alpine:prep stage and the otelcontribcol binary. Therefore, standard Alpine tools like wget, curl, and busybox are NOT available in the runtime image. The image is designed to be minimal for production use.

Citations:


🌐 Web query:

otelcol otel-collector-contrib healthcheck command CLI built-in health check command

💡 Result:

The standard OpenTelemetry Collector binaries (otelcol and otelcol-contrib) do not have a built-in CLI "healthcheck" subcommand. The primary health check mechanism is the "health_check" extension (or healthcheckv2), which exposes an HTTP endpoint (default: 0.0.0.0:13133) for external probing when configured and enabled in the service section. CLI subcommands include "validate" (for config validation), "components" (list available components), and "version". No healthcheck CLI command is documented in official sources. A proposal exists to add "otelcol-contrib healthcheck" for Docker HEALTHCHECK self-checks (Issue #40517, open as of 2026), but it is not implemented in standard distributions. AWS Distro for OpenTelemetry (ADOT) includes a separate "/healthcheck" binary for container health checks, but this is not in upstream otelcol-contrib. For container health checks, common practices use external tools like curl or grpcurl against the HTTP/gRPC health endpoint, or custom images adding such tools.

Citations:


🏁 Script executed:

#!/bin/bash
# Check if there's any healthcheck-related functionality in the otel collector binary itself
rg -i "healthcheck|health.?check" performance-test-server/ --type yaml --type json --type md

Repository: Connectum-Framework/examples

Length of output: 1088


Collector healthcheck doesn't validate the running collector—use a viable alternative.

/otelcol-contrib components succeeds even if the health_check extension (configured on :13133) is not ready. Since k6-otel-export gates on service_healthy, it can start before the collector's OTLP receivers are operational.

However, the proposed wget solution will not work: the official otel/opentelemetry-collector-contrib:0.120.0 image is scratch-based and has no shell, wget, or curl.

Viable options:

  1. Custom image with curl/wget — Build a Dockerfile that adds curl to the contrib image.
  2. Pre-flight dependency check — Use a separate healthcheck service container (e.g., Alpine with curl) that probes :13133 and reports to a shared volume.
  3. Document the trade-off — Accept that healthcheck validates binary availability, not receiver readiness, and document the startup sequence in comments.

The core issue is real and worth addressing given the k6 dependency, but the proposed fix requires adapting to the image constraints.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@performance-test-server/docker-compose.yml` around lines 46 - 51, The current
healthcheck using test: ["CMD", "/otelcol-contrib", "components"] doesn't verify
OTLP receiver readiness (and the scratch-based otel image has no curl/wget), so
replace it with one of the viable options: either build a custom collector image
that installs curl and change the healthcheck to curl -f http://localhost:13133/
or implement a separate tiny sidecar healthcheck service (e.g., an Alpine
container with curl) that probes http://collector:13133/health (or the receiver
endpoint) and signals readiness via a shared volume or by using depends_on with
condition: service_healthy for k6-otel-export; alternatively add a comment
documenting that the current check only validates binary availability and not
receiver readiness. Update the docker-compose healthcheck entry (symbol:
healthcheck and test) and any k6-otel-export startup gating (symbol:
service_healthy) accordingly.

Comment on lines +93 to +96
depends_on:
server: { condition: service_healthy }
otel-collector: { condition: service_healthy }
command: run /scripts/otel-export-overhead.js

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

server: service_healthy does not prove port 8085 is ready.

The server healthcheck only exercises port 8080, but this k6 service immediately requires port 8085. If 8085 is not bound yet or OTEL_EXPORT_ENABLED was missed, the benchmark starts and fails in setup. Consider making the server healthcheck include 8085 when OTEL_EXPORT_ENABLED=1.

🩺 Proposed direction
-        node -e "const h=require('node:http2'),c=h.connect('https://localhost:8080',
+        node -e "const h=require('node:http2');const ports=process.env.OTEL_EXPORT_ENABLED==='1'?[8080,8085]:[8080];
+        let pending=ports.length, failed=false;
+        for (const port of ports){const c=h.connect(`https://localhost:${port}`,
         {rejectUnauthorized:false});const r=c.request({':method':'POST',
         ':path':'/greeter.v1.GreeterService/SayHello','content-type':'application/json',
         'connect-protocol-version':'1'});r.end(JSON.stringify({name:'health'}));
-        let d='';r.on('data',x=>d+=x);r.on('end',()=>{c.close();process.exit(d?0:1)});
-        r.on('error',()=>{c.close();process.exit(1)});
-        setTimeout(()=>{c.close();process.exit(1)},3000)"
+        let d='';r.on('data',x=>d+=x);r.on('end',()=>{c.close();failed ||= !d;if(--pending===0)process.exit(failed?1:0)});
+        r.on('error',()=>{c.close();process.exit(1)});
+        setTimeout(()=>{c.close();process.exit(1)},3000)}"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@performance-test-server/docker-compose.yml` around lines 93 - 96, The server
healthcheck currently only verifies port 8080 but the k6 job (command: run
/scripts/otel-export-overhead.js) needs port 8085 when OTEL_EXPORT_ENABLED=1;
update the server healthcheck (or add a new healthcheck probe used by
docker-compose) to verify port 8085 as well when OTEL_EXPORT_ENABLED is set, or
make the k6 service depend_on a new healthcheck target that checks both ports;
specifically modify the server healthcheck block (and/or the depends_on entry
referenced by the k6 service) so it conditionally checks port 8085 when
OTEL_EXPORT_ENABLED=1 to ensure the benchmark waits for 8085 to be bound before
starting.

Comment on lines +130 to +163
export default function () {
// Alternate baseline / otel-export per iteration to average out JIT/GC
// jitter. Each iteration touches both configs once, matching the
// interceptor-overhead.js pattern.
const testCases = [
{
run() {
const { response, success } = callService(BASELINE_PORT, "baseline");
baselineDuration.add(response.timings.duration);
baselineSuccess.add(success);
},
},
{
run() {
const { response, success } = callService(OTEL_EXPORT_PORT, "otel_export");
otelExportDuration.add(response.timings.duration);
otelExportSuccess.add(success);
},
},
];

// Fisher-Yates shuffle — eliminate ordering bias.
for (let i = testCases.length - 1; i > 0; i--) {
const j = Math.floor(Math.random() * (i + 1));
[testCases[i], testCases[j]] = [testCases[j], testCases[i]];
}

for (const testCase of testCases) {
testCase.run();
}

// Small think time to keep the offered load realistic and to give
// BatchSpanProcessor room to batch exports rather than flush per-request.
sleep(0.05);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

This paired loop cannot measure throughput delta.

Each iteration always calls both configs once, so baseline and OTel-export request counts/rates are coupled. Latency deltas are still useful, but throughput delta is not independently measurable with this closed-loop paired design.

Either remove the throughput-delta claim or split the configs into independent scenarios/runs.

🛠️ Minimal doc/comment fix if this is latency-only
- *   Throughput (requests/sec) per config
- *   Export-overhead delta printed in teardown
+ *   p50/p95/p99 latency delta per config
+ *   Export-overhead latency delta printed in teardown
-    console.log("\n  Goal: measure p50/p95/p99 latency delta and throughput delta");
+    console.log("\n  Goal: measure p50/p95/p99 latency delta");
-    console.log("   - Compute relative = with_otel_export / baseline_no_otel");
+    console.log("   - Compute relative latency = with_otel_export / baseline_no_otel");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@performance-test-server/k6/otel-export-overhead.js` around lines 130 - 163,
The current test harness builds a paired testCases array and always executes
both entries per iteration (see testCases and the for...of loop calling
testCase.run()), which couples baseline and OTEL_EXPORT request counts so you
cannot measure independent throughput delta; either change the design to run
each config in separate scenarios/runs (e.g., split into two independent
loops/scenarios that call callService(BASELINE_PORT, ...) and
callService(OTEL_EXPORT_PORT, ...) separately so
baselineDuration/baselineSuccess and otelExportDuration/otelExportSuccess are
accumulated independently), or remove any claim about measuring throughput delta
and document that this test only reports latency deltas. Ensure the Fisher‑Yates
shuffle and sleep placement are adjusted accordingly if you split runs so each
scenario preserves jitter reduction and batching characteristics.

Comment on lines +102 to +105
```bash
OTEL_EXPORT_ENABLED=1 docker compose --profile otel-export up \
--build --abort-on-container-exit
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Target k6-otel-export explicitly to avoid running the default benchmark too.

docker compose --profile otel-export up starts all unprofiled services as well, so k6-interceptor-overhead can run concurrently and skew this benchmark. Specify the service name.

🐛 Proposed fix
 OTEL_EXPORT_ENABLED=1 docker compose --profile otel-export up \
-  --build --abort-on-container-exit
+  --build --abort-on-container-exit k6-otel-export
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
```bash
OTEL_EXPORT_ENABLED=1 docker compose --profile otel-export up \
--build --abort-on-container-exit
```
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@performance-test-server/README.md` around lines 102 - 105, The docker compose
command in the README starts unprofiled services too; change the example to
explicitly target the k6-otel-export service by adding its service name to the
up command (i.e., run docker compose --profile otel-export up k6-otel-export
--build --abort-on-container-exit with OTEL_EXPORT_ENABLED=1) so the default
benchmark (k6-interceptor-overhead) does not run concurrently and skew results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:feature New feature or enhancement request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant