Skip to content

fix(metrics): /api/v1/query/arrow counts errors but not requests or successes #801

Description

@xe-nvdk

Summary

POST /api/v1/query/arrow increments arc_query_errors_total on failure but never increments arc_query_requests_total or arc_query_success_total. The Arrow endpoint therefore counts its failures and none of its attempts, which makes the obvious error-rate expression wrong — and unbounded — for any Arrow-heavy workload.

Repro (verified live, current main 5f8412e)

Standalone Arc, local storage, auth off. Two Arrow queries: one valid, one syntactically invalid.

BEFORE:
arc_query_requests_total 4
arc_query_success_total 1
arc_query_errors_total 1

$ curl -XPOST /api/v1/query/arrow -d '{"sql":"SELECT count(*) FROM cpu"}'   -> HTTP 200
$ curl -XPOST /api/v1/query/arrow -d '{"sql":"SELECT FROM WHERE"}'          -> HTTP 500

AFTER:
arc_query_requests_total 4      <- unchanged, both queries invisible
arc_query_success_total 1       <- unchanged, the successful query invisible
arc_query_errors_total 2        <- +1, only the failure counted

Root cause

Counts of the three helpers by file:

File IncQueryRequests IncQuerySuccess IncQueryErrors
internal/api/query.go 2 5 42
internal/api/query_arrow.go 0 0 13

internal/api/query.go:1527 increments requests at entry to the JSON path. internal/api/query_arrow.go#executeQueryArrow (registered at internal/api/query_arrow.go:958) has no equivalent; it only calls m.IncQueryErrors() on its failure branches (query_arrow.go:386, 395, 417, 427, 436, 453, 472, 479, 493, 521, …).

Impact

This is a monitoring correctness bug, not a data bug, but it breaks the first dashboard anyone builds:

  1. Error rate is inflated, and can exceed 1. rate(arc_query_errors_total[5m]) / rate(arc_query_requests_total[5m]) counts Arrow failures in the numerator but no Arrow traffic in the denominator. A pure-Arrow deployment drives the numerator up while the denominator stays flat at whatever the JSON path did — so the ratio diverges, and is a divide-by-zero if no JSON queries ever ran.
  2. Arrow QPS is unobservable. There is no counter that moves on a successful Arrow query, so throughput on the high-performance path cannot be graphed from /metrics at all. arc_http_requests_total moves, but it aggregates every endpoint including writes and health checks.
  3. arc_query_success_total + arc_query_errors_total != arc_query_requests_total in general, so none of the three can be safely used as a denominator.

Arrow is the path performance-sensitive clients are steered to, so this is likely under-counting the majority of query traffic in exactly the deployments most likely to be monitored closely.

Suggested fix

Add m.IncQueryRequests() at entry to executeQueryArrow, and m.IncQuerySuccess() on the success path, mirroring internal/api/query.go. Worth a quick audit of the other query entry points at the same time (GET /api/v1/query/:measurement, and the msgpack query response path) to confirm they are wired.

If the intended contract is instead "requests = JSON only", that needs to be stated in the metric HELP text, because the current name implies otherwise.

Test plan

  • Unit/integration test asserting arc_query_requests_total increments for POST /api/v1/query/arrow
  • Test asserting arc_query_success_total increments on a successful Arrow query
  • Assert the invariant success + errors == requests across both endpoints after a mixed workload
  • Regression tests must FAIL pre-fix (revert-run-restore)

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions