Skip to content

fix: bound SQLite observability metric scans - #6390

Open
jstar0 wants to merge 2 commits into
IBM:mainfrom
jstar0:fix/6286-sqlite-observability-metrics
Open

fix: bound SQLite observability metric scans#6390
jstar0 wants to merge 2 commits into
IBM:mainfrom
jstar0:fix/6286-sqlite-observability-metrics

Conversation

@jstar0

@jstar0 jstar0 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

The public observability metrics endpoints currently use a Python fallback on SQLite that calls .all() for every trace in the requested window. A 168-hour request can therefore materialize the full raw window in the API process.

This change:

  • Aggregates SQLite execution counts in SQL while preserving the existing UTC bucket contract.
  • Computes SQLite p50/p95/p99 with window functions and linear interpolation equivalent to PostgreSQL percentile_cont, without loading every duration into Python.
  • Reuses the existing observability TTL cache with query-specific keys for metric type, look-back window, and interval.
  • Assigns /observability/metrics/* and /v1/observability/metrics/* to an explicit rate-limit tier instead of the default fallback.

The PostgreSQL path and the unsupported-dialect Python fallback remain unchanged.

Closes #6286

Verification

  • make test with proxy environment variables cleared: 22533 passed, 870 skipped, 2 xfailed.
  • Focused observability, cache, and rate-limit tests: 100% passed.
  • Observability service/router SQL tests: 114 passed.
  • Admin observability SQL tests: 27 passed.
  • ruff check on changed production and focused test files: passed.
  • ruff format --check on changed files: passed.
  • git diff --check: passed.
  • SQLite query-plan check confirmed the start_time index is used for the bounded time filter.

Signed-off-by: King Star <mcxin.y@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses unbounded memory growth on SQLite deployments for the public observability metrics endpoints by moving aggregation work into SQLite SQL (instead of Python .all() materialization), adding short-TTL caching keyed by query shape, and explicitly classifying these endpoints under a dedicated rate-limit tier.

Changes:

  • Add SQLite-native SQL implementations for metrics timeseries and latency percentiles (window functions + interpolation) to avoid Python-side full-window materialization.
  • Reuse AdminStatsCache for metrics results with query-specific cache keys (metric + hours + interval) and add test coverage for cache key isolation.
  • Add an explicit OBSERVABILITY_METRICS rate-limit tier for /observability/metrics/* and /v1/observability/metrics/* and validate tier selection in middleware tests.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
mcpgateway/services/observability_service.py Adds SQLite SQL aggregation paths for timeseries and percentiles; keeps PostgreSQL and generic fallback intact.
mcpgateway/routers/observability.py Adds per-query caching for metrics endpoints using AdminStatsCache keyed by metric/hours/interval.
mcpgateway/cache/admin_stats_cache.py Extends observability stats cache API to accept an optional query-specific cache key.
mcpgateway/middleware/rate_limit_middleware.py Introduces OBSERVABILITY_METRICS tier pattern to explicitly classify metrics endpoints.
tests/unit/mcpgateway/routers/test_observability_metrics.py Adds fixtures to isolate cache across tests and adds a cache-key isolation test.
tests/unit/mcpgateway/cache/test_admin_stats_cache.py Verifies the new optional cache_key parameter works for in-memory cache paths.
tests/unit/mcpgateway/middleware/test_rate_limit_middleware.py Adds coverage ensuring metrics paths map to the new rate-limit tier.
Suppressed comments (1)

mcpgateway/services/observability_service.py:1977

  • Same compatibility concern here: unixepoch() is not present in older SQLite versions, so this percentile query can error at runtime. strftime('%s', start_time) is widely supported and avoids a hard SQLite version dependency.
                CAST(unixepoch(start_time) / :interval_seconds AS INTEGER) * :interval_seconds AS bucket_epoch,

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread mcpgateway/services/observability_service.py Outdated
Signed-off-by: King Star <mcxin.y@gmail.com>
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.

[BUG]: Unbounded full-window scan in the SQLite observability metrics fallback

2 participants