Skip to content

fix(loadtest): eliminate harness false-positives across 30+ admin endpoints#5456

Open
Lang-Akshay wants to merge 3 commits into
mainfrom
dashing-feels
Open

fix(loadtest): eliminate harness false-positives across 30+ admin endpoints#5456
Lang-Akshay wants to merge 3 commits into
mainfrom
dashing-feels

Conversation

@Lang-Akshay

@Lang-Akshay Lang-Akshay commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Converted 30+ admin endpoints from _validate_json_response to _validate_status — these endpoints return HTML (not JSON) when MCPGATEWAY_ADMIN_API_ENABLED=false, causing false-positive CatchResponseError failures in load tests
  • Added SOFT_JSONRPC_ERROR_CODES = frozenset({-32000}) constant and tolerates JSON-RPC -32000 errors in _validate_jsonrpc_response-32000 is expected when MCP backends are unavailable
  • POST /auth/email/admin/users now accepts 400 alongside 403/409/422 — server returns 400 for email validation and password policy failures
  • /admin/events switched from _validate_json_response to _validate_status — endpoint returns SSE (text/event-stream), not JSON
  • GET /prompts/{id} now accepts 422 as a valid response — gateway maps PromptError (missing required args) to HTTP 422
  • User-creating tasks use uuid.uuid4().hex (128-bit) instead of uuid.uuid4().hex[:8] (32-bit) for email addresses, eliminating birthday collisions at ~542 RPS over 10 minutes
  • SSE virtual server registration clears associated_resources: [] to remove ambiguous resource routing caused by both HTTP and SSE servers registering the same resource URIs

Affected admin endpoint groups (all converted to _validate_status):

  • Observability: heatmap, timeseries, top-errors, top-volume, top-slow, tools/usage, tools/performance
  • Performance: cache, history, system, requests
  • Plugins: list, stats, detail
  • Search: servers, gateways, resources, prompts, users, tools
  • IDs: tools/ids, servers/ids
  • System: mcp-registry/servers, metrics/reset, well-known, grpc

The two genuine gateway bugs surfaced by this investigation are tracked separately in #5453 (TaskGroup crash on prompts/get) and #5454 (tools/call returning -32603).

Closes #5321

@jonpspri jonpspri added MUST P1: Non-negotiable, critical requirements without which the product is non-functional or unsafe release-time-findings issues discovered during release testing. labels Jul 1, 2026
Fix four categories of false-positive failures causing ~7.6% error rate
in `make load-test-cli` runs despite no real gateway misbehavior:

- Use `_validate_status` for /admin/events (SSE endpoint, not JSON)
- Allow 422 for GET /prompts/{id} (valid PromptError response)
- Use full uuid4().hex (128-bit) for user emails to prevent birthday
  collisions at ~542 RPS sustained over 10 minutes
- Clear associated_resources from SSE virtual server to eliminate
  ambiguous resource routing errors

Real gateway bugs (TaskGroup crash, -32603 tool error) tracked in #5453
and #5454.

Closes #5321

Signed-off-by: Lang-Akshay <akshay.shinde26@ibm.com>
@gandhipratik203

Copy link
Copy Markdown
Collaborator

PR fixes 4 load-test false positives (SSE endpoint wrongly validated as JSON, missing 422 case for prompt errors, 32-bit UUID collisions at high RPS, and ambiguous dual-registered resources). Might need a rebase on the latest main. Looks good otherwise.

@Lang-Akshay Lang-Akshay changed the title fix(loadtest): eliminate harness false-positives in 10-minute load test fix(loadtest): eliminate harness false-positives across 30+ admin endpoints Jul 3, 2026
@gandhipratik203

Copy link
Copy Markdown
Collaborator

Log form the latest run. run.log This is singificant improvement given that it is a best effort task given the environment based variations.

@gandhipratik203 gandhipratik203 self-requested a review July 3, 2026 16:16
gandhipratik203
gandhipratik203 previously approved these changes Jul 3, 2026

@gandhipratik203 gandhipratik203 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM!

Signed-off-by: Lang-Akshay <ashinde266@gmail.com>
@ja8zyjits

Copy link
Copy Markdown
Collaborator

⚠️ Warnings (Non Blocking)

Documentation Mismatch

  • Load-test docs describe AdminUIUser as always participating with fixed weight, but implementation now disables admin/UI-heavy user classes when MCPGATEWAY_UI_ENABLED=false or MCPGATEWAY_ADMIN_API_ENABLED=false
    • File: docs/docs/testing/performance.md:369
    • Code: tests/loadtest/locustfile.py:1114, 2828, 4215, 4518
    • Fix: Update load-test docs to note several admin-facing Locust classes are feature-flag aware and may run with weight 0 when Admin UI or Admin API disabled

🔧 Issues

DRY Violation

  • Weight calculation pattern duplicated across 15+ user classes
    • Files: tests/loadtest/locustfile.py (multiple locations)
    • Pattern: weight = 1 if ADMIN_API_ENABLED else 0 and weight = 1 if (ADMIN_UI_ENABLED and ADMIN_API_ENABLED) else 0
    • Classes: AdminUIUser, ObservabilityUser, AdminObservabilityExtendedUser, AdminPerformanceExtendedUser, AdminPluginsUser, AdminSystemExtendedUser, AdminSectionsUser, AdminSearchUser, AdminCacheConfigUser, AdminHTMXPartialsUser, AdminGrpcUser, AdminLoginLogoutUser, AdminLogsExtendedUser, AdminSupportBundleUser, AdminEntityDetailUser, AdminMetricsResetUser, AdminTeamsMembershipUser, AdminResourcesTestUser, AdminDetailReadExtendedUser, AdminGrpcCRUDUser, AdminHTMXEntityOpsUser, AdminMCPRegistryOpsUser, AdminHTMXEntityCRUDUser, AdminUsersOpsUser, AdminTeamsHTMXOpsUser
    • Fix: Extract to helper function _get_admin_weight(requires_ui=False, requires_api=True) that returns appropriate weight
    • Benefit: Single source of truth, easier to adjust weights globally

…asses to prevent false failures

Signed-off-by: Lang-Akshay <akshay.shinde26@ibm.com>
@Lang-Akshay

Copy link
Copy Markdown
Collaborator Author

Thanks for the review @ja8zyjits

  • ✅ Fix 1 docs/docs/testing/performance.md: Corrected AdminUIUser weight to 3, added note block listing all feature-flag-aware classes by requirement type
  • ✅ Fix 2 tests/loadtest/locustfile.py: Added _get_admin_weight(requires_ui, requires_api, base) helper after the flag declarations; replaced all 26 duplicated conditionals with helper calls

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

Labels

MUST P1: Non-negotiable, critical requirements without which the product is non-functional or unsafe release-time-findings issues discovered during release testing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TESTING]: Load test should pass without errors

4 participants