Skip to content

feat(telemetry): expose a native prometheus scrape endpoint - #2

Closed
utsab345 wants to merge 18 commits into
mainfrom
feature/prometheus-metrics-3191
Closed

utsab345 wants to merge 18 commits into
mainfrom
feature/prometheus-metrics-3191

Conversation

@utsab345

@utsab345 utsab345 commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Description

Implements The-PR-Agent#3191: an opt-in native Prometheus scrape endpoint for the OpenTelemetry command telemetry.

Telemetry stays disabled by default and the exporter is only active when OTEL.EXPORTER_TYPE = "prometheus" — nothing is exposed or changed out of the box. In that mode the pr_agent.commands counter (and any future gauge instruments) is rendered as GET /metrics on the gunicorn-served webhook apps: github_app, gitlab_webhook, azuredevops_server_webhook, and gitea_app.

Gunicorn runs with preload_app = True, so counters must aggregate correctly across workers. The exporter translates each worker's DELTA aggregates into prometheus_client metrics on the worker side, writes them to per-pid state files under a shared PROMETHEUS_MULTIPROC_DIR, and /metrics merges every worker's file with a MultiProcessCollector at scrape time. gunicorn provisions the state directory in when_ready (before the first fork) and deregisters workers in child_exit, so counters stay accurate through worker churn.

  1. Dependency change (flagged as requested). This PR adds prometheus-client==0.21.1 to the base dependency set and no other runtime dependency.
  2. Why not opentelemetry-exporter-prometheus? The maintainer's suggested approach named that package, but its PrometheusMetricReader documents no multiprocessing support, and the only published versions are 0.x pre-releases. Since multiprocess aggregation under gunicorn is the core requirement of the issue, expecting a MetricExporter to plug into the existing PeriodicExportingMetricReader (my earlier suggestion in the issue comments was based on an inaccurate assumption about that package), I implemented a small in-house bridge on prometheus_client directly. It is ~120 lines, metric-naming and label rules match the OpenMetrics format, and it reuses the existing PeriodicExportingMetricReader + push-on-release flow. Happy to revisit if maintainers prefer a different dependency strategy.
  3. The endpoint is mounted only when the prometheus exporter is selected, so a default deployment exposes no new surface and requires no collector configuration.
  4. The exporter is command-metrics only: spans are not exported in this mode (a startup warning makes that explicit). The reader-fork lifecycle gap noted as point 5 in the issue is a separate bug and is intentionally left out of this PR.
  5. A single-process deployment (plain uvicorn, no gunicorn) works without the state directory and serves its own in-process registry.

Configuration

[otel]
is_enabled = true
exporter_type = "prometheus"          # "console" | "otlp" | "prometheus" | "none"
prometheus_multiproc_dir = "/tmp/pr-agent-prometheus"  # shared, writable by every worker

The directory can be mounted as a volume for durability; when_ready creates it when missing. Example scrape config:

scrape_configs:
  - job_name: pr-agent
    static_configs:
      - targets: ["pr-agent:3000"]

How this was verified

  1. Unit tests translate an OTel counter into the Prometheus text format through the real MeterProvider + PeriodicExportingMetricReader path, including name sanitization (pr_agent.commands → pr_agent_commands_total) and label-key sanitization (provider.name → provider_name).
  2. A multiprocess end-to-end test runs the exporter in two fresh interpreter processes (as gunicorn workers would) and asserts /metrics merges both workers' state files.
  3. The exporter-type validation, tracer/meter agreement, config provisioning of the state dir, and shipped defaults are covered in the telemetry tests.
  4. A test verifies the module can be imported without importing prometheus_client (gunicorn master safety under preload_app).
  5. Full unit suite: 4459 passed;ruff and pre-commit hooks clean.

closes The-PR-Agent#3191

shashankvarma499 and others added 18 commits September 8, 2026 13:39
…-Agent#3201)

Signed-off-by: Shashank Varma <324153016+shashankvarma499@users.noreply.github.com>
Co-authored-by: Aurora <aurora9c69543e@atomicmail.ai>
…-Agent#3163)

Co-authored-by: Alex Tumanov <6143578+oleksii-tumanov@users.noreply.github.com>
…ent#3219)

Co-authored-by: Aurora <aurora9c69543e@atomicmail.ai>
Adds an opt-in 'prometheus' OTEL.EXPORTER_TYPE that renders the command
counter as a native GET /metrics endpoint on the gunicorn-served webhook
apps (github_app, gitlab_webhook, azuredevops_server_webhook, gitea_app).

The exporter is meters-only and bridges the SDK's DELTA counter aggregates
into prometheus_client metrics on the worker side. gunicorn provisions a
shared PROMETHEUS_MULTIPROC_DIR before forking workers and deregisters them
on child_exit, so a MultiProcessCollector merges every worker's values at
scrape time. The endpoint is only mounted when this exporter is selected,
so nothing is exposed by default.

Dependency: adds prometheus-client==0.21.1. opentelemetry-exporter-prometheus
was explicitly avoided because its PrometheusMetricReader documents no
multiprocessing support and it only ships as a 0.x pre-release.

Refs: The-PR-Agent#3191
Comment on lines +65 to +66
"counter = provider.get_meter('mp-test').create_counter("
"'pr_agent.commands', unit='{command}', description='PR-Agent commands executed')",
@utsab345 utsab345 closed this Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose a Prometheus-scrapable metrics endpoint for multiprocess workers