Skip to content

feat(metrics): make built-in summary objectives configurable#505

Open
tiagomartines11 wants to merge 1 commit into
v2from
feat/configurable-summary-objectives
Open

feat(metrics): make built-in summary objectives configurable#505
tiagomartines11 wants to merge 1 commit into
v2from
feat/configurable-summary-objectives

Conversation

@tiagomartines11

Copy link
Copy Markdown
Member

What

Make the objectives (quantiles) of the two built-in Prometheus summary metrics configurable:

  • pitaya_handler_response_time_ns (ResponseTime)
  • pitaya_handler_handler_delay_ns (ProcessDelay)

Both were hard-coded to {0.7: 0.02, 0.95: 0.005, 0.99: 0.001}. This adds a new config key pitaya.metrics.prometheus.objectives so operators can drop unused percentiles (or emit none) without forking pitaya.

Why

Each summary emits, per label-set, one series per quantile plus _sum and _count. With the hard-coded three quantiles that is 5 series per label combo. On a high-cardinality handler (e.g. a connector with hundreds of route×status×code combos) the quantile series are ~half of the /metrics payload and push the endpoint well past the Datadog OpenMetrics default 2,000-series cap, which causes truncation and phantom counter spikes.

Client-side summary quantiles are per-pod and cannot be aggregated across pods, so teams that only chart 0.95/0.99 still pay for 0.7. Making objectives configurable lets them drop what they don't use.

How

  • New field PrometheusConfig.Objectives map[string]float64 (keyed by the quantile string, e.g. "0.95", for YAML/env friendliness), converted to map[float64]float64 when building SummaryOpts.
  • Applied to both built-in summaries via a shared newSummaryVec helper.
  • Backward compatible: when unset, the reporter falls back to the historical {0.7, 0.95, 0.99} — byte-for-byte identical to today.

Note on the default (important)

The option is intentionally not seeded with a viper default. pitaya's custom UnmarshalKey deep-merges seeded default map keys back into any override, so a seeded default would make partial overrides silently regain the dropped percentiles (verified: setting {0.95, 0.99} still produced {0.7, 0.95, 0.99}). Instead, "unset" resolves to nil, and buildObjectives(nil) supplies the historical default. This makes both "drop 0.7" and "emit no quantiles" actually work.

Config

pitaya:
  metrics:
    prometheus:
      objectives:
        "0.95": 0.005
        "0.99": 0.001   # 0.7 dropped
      # objectives: {}   # emit only _sum/_count, no quantile series
Config Result
unset historical {0.7, 0.95, 0.99}
{"0.95": .., "0.99": ..} exactly those (0.7 dropped)
{} (empty) only _sum/_count

Tests

  • metrics: buildObjectives (nil→default, custom, empty, invalid key) + scrape assertions that the registered summaries emit exactly the configured quantile="..." series (default / dropped / none), and always _sum/_count.
  • config: resolution through the real NewPitayaConfig viper path — unset→nil, partial-drop, empty-stays-empty (regression guard against re-seeding a default).

Scope

  • Custom summaries (spec.Summaries[].Objectives) unchanged — already configurable.
  • The ResponseTime histogram registration is untouched (out of scope).
  • No public constructor signature changes.

Docs

docs/configuration.rst documents the new key, default, YAML shape, and the client-side-quantiles-don't-aggregate caveat.

Add pitaya.metrics.prometheus.objectives to tune the quantiles emitted by
the built-in handler_response_time_ns and handler_handler_delay_ns
summaries, which were hard-coded to {0.7, 0.95, 0.99}. Operators can drop
unused percentiles (or emit none) to cut Prometheus series cardinality.

The map is keyed by the quantile string for YAML/env friendliness and
converted to map[float64]float64 when building SummaryOpts. The option is
intentionally not seeded with a viper default: a seeded default would be
deep-merged back into any partial override, so dropping a percentile via
config would silently fail. Unset resolves to nil, which the reporter maps
to the historical {0.7, 0.95, 0.99}, keeping default behavior identical.

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

Copy link
Copy Markdown

Coverage Report for CI Build 29434363368

Coverage increased (+0.5%) to 61.971%

Details

  • Coverage increased (+0.5%) from the base build.
  • Patch coverage: 21 uncovered changes across 1 file (28 of 49 lines covered, 57.14%).
  • 18 coverage regressions across 2 files.

Uncovered Changes

File Changed Covered %
metrics/prometheus_reporter.go 49 28 57.14%

Coverage Regressions

18 previously-covered lines in 2 files lost coverage.

File Lines Losing Coverage Coverage
app.go 12 67.76%
module.go 6 62.64%

Coverage Stats

Coverage Status
Relevant Lines: 8504
Covered Lines: 5270
Line Coverage: 61.97%
Coverage Strength: 0.68 hits per line

💛 - Coveralls

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.

2 participants