Add standalone metrics service to Payjoin-service by zealsham · Pull Request #1296 · payjoin/rust-payjoin

zealsham · 2026-01-23T11:22:07Z

This PR addresses #1284.

it introduces Prometheus metric collection as a standalone service for payjoin-service
This Pr will also implement most of the metrics discussed here https://github.com/orgs/payjoin/discussions/775)

Pull Request Checklist

Please confirm the following before requesting review:

I have disclosed my use of
AI
in the body of this PR.
I have read CONTRIBUTING.md and rebased my branch to produce hygienic commits.

coveralls · 2026-01-23T11:28:37Z

Pull Request Test Coverage Report for Build 21691748627

Details

138 of 157 (87.9%) changed or added relevant lines in 7 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.4%) to 83.649%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
payjoin-directory/src/main.rs	0	1	0.0%
payjoin-service/src/metrics.rs	34	35	97.14%
payjoin-service/src/config.rs	0	4	0.0%
payjoin-service/src/lib.rs	69	82	84.15%

Totals
Change from base Build 21675282596:	0.4%
Covered Lines:	10232
Relevant Lines:	12232

💛 - Coveralls

spacebear21

Thanks for the quick PR! Besides the review comments below I have two other requests:

Let's squash the two commits so that the first one doesn't fail CI
IMO there are two additional metrics described in #1284 that would be really valuable to add here. Both of these are give us a much better overview of actual usage:
- number of entries in DB (v1 vs v2)
- unique shortid counters for a given time period. Extra points for the ability to select a time frame.

spacebear21 · 2026-01-24T21:05:56Z

+        .layer(axum::middleware::from_fn_with_state(metrics.clone(), track_metrics))
+        .layer(axum::middleware::from_fn_with_state(metrics.clone(), track_connections))


axum docs recommend using ServiceBuilder to apply multiple middleware at once: https://docs.rs/axum/latest/axum/middleware/index.html#applying-multiple-middleware

spacebear21 · 2026-01-24T21:22:17Z

+        let status_type = match status_code {
+            200..=299 => "success",
+            300..=399 => "redirect",
+            400..=499 => "client_error",
+            500..=599 => "server_error",
+            _ => "unknown",
+        };


This is lossy, we might as well save the exact status code and grouping could always be done when "encoding" at read time.

spacebear21 · 2026-01-24T21:23:44Z

+
+        // Total number of connections seen so far
+        let total_connections =
+            IntCounter::new("Total_connections", "Total number of connections")?;


All tracked metrics should be defined in constants instead of hardcoded. Also this one is capitalized, they should be consistently lower snake case. In general we should do our best to follow Prometheus naming best practices

spacebear21 · 2026-01-24T21:30:42Z

+                    Ok(body) => {
+                        let response = Response::builder()
+                            .status(StatusCode::OK)
+                            .header("Content-Type", "text/plain; version=0.0.4; charset=utf-8")


nit: use https://docs.rs/http/latest/http/header/constant.CONTENT_TYPE.html

spacebear21 · 2026-01-24T21:34:28Z

+    match (method, path) {
+        // Ohttp endpoints
+        ("POST", "/.well-known/ohttp-gateway") => "ohttp_gateway",
+        ("GET", "/.well-known/ohttp-gateway") => "ohttp_gateway_get",
+        ("POST", "/") => "ohttp_gateway",
+        ("GET", "/ohttp-keys") => "ohttp_keys",
+
+        ("GET", "/health") => "health",
+        ("GET", "/") => "directory_home",
+        ("GET", "/metrics") => "metrics",
+
+        _ => "unknown",
+    }
+    .to_string()


In the same vein as https://github.com/payjoin/rust-payjoin/pull/1296/changes#r2724621450, we could store the whole path and method for losslessness, and group or sort things when reading instead of when writing.

This also makes me wonder if there's a way to correlate different metrics? For example say we are tracking lots of 400s and they are coming from "GET /ohttp-keys" requests, is there a way to identify that this is the problematic path? How might we achieve that?

since there's only a small number of paths each endpoint could be counted separately

time series databases should generally be able to aggregate such information but for convenience we could also have combined counters/histograms

that said, for the specific example you gave is more of a logging/observability concern than a metrics one, i.e. knowing that there are errors to look into makes sense as a metric but if you actually want to figure out the cause you probably want more than just the endpoint but the complete error

the tracing crate can emit structured logs with spans intact using opentracing, it shouldn't take much work to make that available... then with various log searching tools you can see aggregate error counts and break them down by the other request properties

nothingmuch

i think it would be better to only serve metrics on a separate interface until we've determined whether there's a good reason to serve specific ones publicly

secondly, the manual routing logic and tower service impl could probably be replaced with axum's router and response handling, wouldn't that be simpler? if that's already planned for a followup PR i think i would prefer if it was done here since this introduces some new code to translate from axum's concepts to the lower level code

zealsham · 2026-01-25T12:40:31Z

Thanks for the review @spacebear21 , i'll implement the changes

number of entries in DB (v1 vs v2)

my intended implementation is create a metrics reporter trait in the directory and have it report DB entries to the metrics service . do you have an opinion about this ?

spacebear21 · 2026-01-27T10:56:22Z

my intended implementation is create a metrics reporter trait in the directory and have it report DB entries to the metrics service . do you have an opinion about this ?

Hmm, on second thought perhaps this one should be left for a follow-up PR since it involves adding stuff to the directory. I'm wondering if it would make more sense to address this sub-item from #941 first:

DB should also be a tower::Service, using tower::{timeout,limit::rate}

zealsham · 2026-01-27T13:56:01Z

my intended implementation is create a metrics reporter trait in the directory and have it report DB entries to the metrics service . do you have an opinion about this ?

Hmm, on second thought perhaps this one should be left for a follow-up PR since it involves adding stuff to the directory. I'm wondering if it would make more sense to address this sub-item from #941 first:

DB should also be a tower::Service, using tower::{timeout,limit::rate}

i'll open a seperate pr for this, and then the db_metrics can be worked on after db-service get merged

spacebear21

cACK, I have minor implementation and code change requests but this looks good to merge otherwise.

This pr addresses payjoin#1284 and wil be built on to reflect the discussion on https://github.com/orgs/payjoin/discussions/775 Request passes through the axum middleware and metrics is collected. This currently tracks http request by endpoint type, method and status. and also tracks active and total connections. Remove metrics collection from Payjoin-directory The metrics server is exposed on different interface fix broken test fix broken test fix broken test Fix broken test dude to lock file fix format issue fix format issue Refactor metrics Servie

zealsham · 2026-02-05T12:25:51Z

cACK, I have minor implementation and code change requests but this looks good to merge otherwise.

implemented

spacebear21

tACK 2a0bffe tested metrics counters locally with basic GET connections to the directory homepage

zealsham marked this pull request as draft January 23, 2026 11:22

zealsham force-pushed the metrics-service branch 2 times, most recently from 52a17f3 to 192c5b5 Compare January 24, 2026 11:38

zealsham changed the title ~~WIP:Add standalone metrics service to Payjoin-service~~ Add standalone metrics service to Payjoin-service Jan 24, 2026

zealsham marked this pull request as ready for review January 24, 2026 12:52

zealsham requested review from DanGould, nothingmuch and spacebear21 January 24, 2026 12:52

spacebear21 requested changes Jan 24, 2026

View reviewed changes

nothingmuch reviewed Jan 24, 2026

View reviewed changes

zealsham force-pushed the metrics-service branch 7 times, most recently from b4df0a5 to a5fd921 Compare February 2, 2026 15:48

spacebear21 mentioned this pull request Feb 3, 2026

feat: implement ohttp gateway middleware #1306

Open

2 tasks

zealsham force-pushed the metrics-service branch from 34ab05c to 70f2f5c Compare February 4, 2026 09:22

spacebear21 requested changes Feb 4, 2026

View reviewed changes

Comment thread payjoin-service/src/config.rs Outdated

Comment thread payjoin-service/src/config.rs Outdated

Comment thread payjoin-service/src/metrics.rs

Comment thread payjoin-service/src/metrics.rs Outdated

spacebear21 mentioned this pull request Feb 4, 2026

Payjoin Mailroom Tracking Issue #941

Open

29 tasks

zealsham force-pushed the metrics-service branch from 70f2f5c to 2a0bffe Compare February 4, 2026 23:01

spacebear21 approved these changes Feb 5, 2026

View reviewed changes

spacebear21 merged commit 9c765dd into payjoin:master Feb 5, 2026
13 checks passed

spacebear21 mentioned this pull request Feb 12, 2026

Aggregated metrics for payjoin-service (with native OTLP) #1327

Merged

2 tasks

		.layer(axum::middleware::from_fn_with_state(metrics.clone(), track_metrics))
		.layer(axum::middleware::from_fn_with_state(metrics.clone(), track_connections))

Conversation

zealsham commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 21691748627

Details

💛 - Coveralls

Uh oh!

spacebear21 left a comment

Choose a reason for hiding this comment

Uh oh!

spacebear21 Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

spacebear21 Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

spacebear21 Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

spacebear21 Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

spacebear21 Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

nothingmuch Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

nothingmuch left a comment

Choose a reason for hiding this comment

Uh oh!

zealsham commented Jan 25, 2026

Uh oh!

spacebear21 commented Jan 27, 2026

Uh oh!

zealsham commented Jan 27, 2026

Uh oh!

spacebear21 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zealsham commented Feb 5, 2026

Uh oh!

spacebear21 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zealsham commented Jan 23, 2026 •

edited

Loading

coveralls commented Jan 23, 2026 •

edited

Loading