agents: fix http_client percentile calculation

[santigimeno]

p50 and p99 where flipped.

Improved test/agents/test-grpc-metrics.mjs to make sure we don't regress.

Fixes: #423

Summary by CodeRabbit

• Bug Fixes

  • Fixed HTTP client percentile mapping so median and p99 latency are reported to the correct metrics.

• Tests

  • Strengthened metrics tests: validate p99 ≥ p50, emit multiple HTTP traces before asserting, run tests against HTTP targets, and simplified agent configuration by removing an obsolete interval setting.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be716e6 and 369ce34.

📒 Files selected for processing (2)

• agents/otlp/src/otlp_common.cc
• test/agents/test-grpc-metrics.mjs

---

Walkthrough

Corrects HTTP client percentile assignments by swapping the 0.5 and 0.99 quantile mappings and updates tests to validate p99 ≥ p50, target HTTP in TestClient, and emit HTTP traces before metrics collection.

Changes

Cohort / File(s)	Summary
Percentile Mapping Fix agents/otlp/src/otlp_common.cc	Swapped HTTP client summary quantile assignments: 0.5 now maps to stor.http_client_median, 0.99 maps to stor.http_client99_ptile.
Test Validation & Config test/agents/test-grpc-metrics.mjs	Added assertion that p99 ≥ p50 when quantileValues present; set TestClient to use HTTP (['-t','http']); removed interval from config(); emit 10 HTTP traces before awaiting metrics; removed NSOLID_INTERVAL from test env configs.

Sequence Diagram(s)

(Skipped — changes are a bug fix and test updates without new multi-component control flow.)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• agents: fix http_client percentile calculation #424 — Makes the same p50/p99 swap in agents/otlp/src/otlp_common.cc and updates the same test to validate p99 ≥ p50.

Suggested reviewers

• EHortua
• RafaelGSS

Poem

🐰 I hopped through metrics with a careful twitch,
Swapped p50 and p99 without a glitch.
I sent ten traces, then watched values align,
Now medians hum true and percentiles shine. ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 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 main fix: correcting the flipped p50 and p99 percentile values in the http_client metric calculation.
Linked Issues check	✅ Passed	The code changes directly address issue #423 by swapping the quantile mapping to fix the p50/p99 flip, and tests are enhanced to prevent regression.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the http_client percentile calculation issue; test modifications are appropriately scoped to validate the fix.
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 (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch santi/fix_http_client_ptile

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

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

[coderabbitai]

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 `@test/agents/test-grpc-metrics.mjs`:
- Around line 243-245: The warm-up loop calls the async TestClient.trace method
(child.trace) without awaiting, causing flaky metrics snapshots; update the loop
that currently invokes child.trace 10 times to await each call (e.g., use "await
child.trace('http')" inside the for-loop or map to Promise.all with awaited
results) so traces complete before metrics are sampled, ensuring deterministic
test behavior.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ad2641 and 39c100e.

📒 Files selected for processing (2)

• agents/otlp/src/otlp_common.cc
• test/agents/test-grpc-metrics.mjs

[coderabbitai]

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 `@test/agents/test-grpc-metrics.mjs`:
- Around line 199-200: The assertion comparing p99 and p50 can run on summaries
with no data and fail because quantileValues[].value may be undefined; update
the check in the test that uses dataPoint.quantileValues to first verify both
quantileValues[0].value and quantileValues[1].value are present (or only run the
comparison for the warmed-up metric name "httpClient") before asserting
assert.ok(dataPoint.quantileValues[0].value >=
dataPoint.quantileValues[1].value, ...); follow the same truthy-guarding pattern
used around validateNumber to skip the comparison when values are falsy.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 39c100e and be716e6.

📒 Files selected for processing (2)

• agents/otlp/src/otlp_common.cc
• test/agents/test-grpc-metrics.mjs

🚧 Files skipped from review as they are similar to previous changes (1)

• agents/otlp/src/otlp_common.cc