Abstract HTTP Path Pattern in Metrics

[kolloch]

Description of the feature

For the request metrics, I see two labels that roughly do what I want: http.url and http.path_pattern.

http.url is to specific and includes the concrete URL including IDs "resolved" in the pattern. I cannot use it, since it will increase the cardinality of the metric drastically (and therefore increase our bills too much). Basically, a separate time series is stored for all label combinations in most metric systems that I know.

The http.path_pattern sounds like what I want but only includes the route prefix at the point where I mounted the middleware, not of the nested route.

The corresponding watched/subscriber routes are mounting like this:

Route::new().nest(
                    opt.public_base_url.path(), // = /private/nx-release/control/
                    Route::new()
                        .nest("/api", api_service)
                        .at("badges/subscriber/:subscription_id", get(subscriber_badge))
                        .at("badges/watched/:git_ref_id", get(watched_badge))
                        .at("request_dump", post(request_dump))
                        .nest("/", ui)
                        .data(opt.clone())
                        // ... more data(..) ...
                        .data(callback_server.clone())
                        .with(poem::middleware::Tracing)
                        .with(middleware::AddRequestId)
                        .with(OpenTelemetryMetrics::new()) // <- adding the instrumentation to capture all routes

I'd love /private/nx-release/control/badges/subscriber/:subscription_id as the http_pattern (or another label).

Existing Code

This is were the http.path_pattern is added.

https://github.com/poem-web/poem/blob/master/poem/src/middleware/opentelemetry_metrics.rs#L82-L85

        if let Some(path_pattern) = req.data::<PathPattern>() {
            const HTTP_PATH_PATTERN: Key = Key::from_static_str("http.path_pattern");
            labels.push(HTTP_PATH_PATTERN.string(path_pattern.0.to_string()));
        }

PathPattern is a simple string container:

#[derive(Debug)]
#[allow(unreachable_pub)]
pub struct PathPattern(pub Arc<str>);

It is set here:

poem/poem/src/route/router.rs

Lines 319 to 328 in 4692e5b

	match (req.data::<PathPattern>(), req.data::<PathPrefix>()) {
	(Some(parent), Some(prefix)) => req.set_data(PathPattern(
	format!("{}{}", parent.0, &pattern[prefix.0..]).into(),
	)),
	(None, Some(prefix)) => req.set_data(PathPattern(pattern[prefix.0..].into())),
	(None, None) => req.set_data(PathPattern(pattern)),
	(Some(parent), None) => {
	req.set_data(PathPattern(format!("{}{}", parent.0, pattern).into()))
	}
	}

                match (req.data::<PathPattern>(), req.data::<PathPrefix>()) {
                    (Some(parent), Some(prefix)) => req.set_data(PathPattern(
                        format!("{}{}", parent.0, &pattern[prefix.0..]).into(),
                    )),
                    (None, Some(prefix)) => req.set_data(PathPattern(pattern[prefix.0..].into())),
                    (None, None) => req.set_data(PathPattern(pattern)),
                    (Some(parent), None) => {
                        req.set_data(PathPattern(format!("{}{}", parent.0, pattern).into()))
                    }
                }

Weirdly, there is test code that pretty much asserts the behavior that I want:

poem/poem/src/route/router.rs

Lines 499 to 563 in 4692e5b

	async fn path_pattern() {
	let app = Route::new()
	.at(
	"/a/:id",
	make_sync(|req| req.data::<PathPattern>().unwrap().0.to_string()),
	)
	.nest(
	"/nest",
	Route::new().at(
	"/c/:id",
	make_sync(|req| req.data::<PathPattern>().unwrap().0.to_string()),
	),
	)
	.nest(
	"/nest1",
	Route::new().nest(
	"/nest2",
	Route::new().at(
	"/:id",
	make_sync(|req| req.data::<PathPattern>().unwrap().0.to_string()),
	),
	),
	)
	.nest_no_strip(
	"/nest_no_strip",
	Route::new().at(
	"/nest_no_strip/d/:id",
	make_sync(|req| req.data::<PathPattern>().unwrap().0.to_string()),
	),
	)
	.nest_no_strip(
	"/nest_no_strip1",
	Route::new().nest_no_strip(
	"/nest_no_strip1/nest_no_strip2",
	Route::new().at(
	"/nest_no_strip1/nest_no_strip2/:id",
	make_sync(|req| req.data::<PathPattern>().unwrap().0.to_string()),
	),
	),
	);
	let cli = TestClient::new(app);
	cli.get("/a/10").send().await.assert_text("/a/:id").await;
	cli.get("/nest/c/10")
	.send()
	.await
	.assert_text("/nest/c/:id")
	.await;
	cli.get("/nest_no_strip/d/99")
	.send()
	.await
	.assert_text("/nest_no_strip/d/:id")
	.await;
	cli.get("/nest1/nest2/10")
	.send()
	.await
	.assert_text("/nest1/nest2/:id")
	.await;
	cli.get("/nest_no_strip1/nest_no_strip2/10")
	.send()
	.await
	.assert_text("/nest_no_strip1/nest_no_strip2/:id")
	.await;
	}

    #[tokio::test]
    async fn path_pattern() {
        let app = Route::new()
            .at(
                "/a/:id",
                make_sync(|req| req.data::<PathPattern>().unwrap().0.to_string()),
            )
            .nest(
                "/nest",
                Route::new().at(
                    "/c/:id",
                    make_sync(|req| req.data::<PathPattern>().unwrap().0.to_string()),
                ),
            )
            .nest(
                "/nest1",
                Route::new().nest(
                    "/nest2",
                    Route::new().at(
                        "/:id",
                        make_sync(|req| req.data::<PathPattern>().unwrap().0.to_string()),
                    ),
                ),
            )
            .nest_no_strip(
                "/nest_no_strip",
                Route::new().at(
                    "/nest_no_strip/d/:id",
                    make_sync(|req| req.data::<PathPattern>().unwrap().0.to_string()),
                ),
            )
            .nest_no_strip(
                "/nest_no_strip1",
                Route::new().nest_no_strip(
                    "/nest_no_strip1/nest_no_strip2",
                    Route::new().at(
                        "/nest_no_strip1/nest_no_strip2/:id",
                        make_sync(|req| req.data::<PathPattern>().unwrap().0.to_string()),
                    ),
                ),
            );

        let cli = TestClient::new(app);

        cli.get("/a/10").send().await.assert_text("/a/:id").await;
        cli.get("/nest/c/10")
            .send()
            .await
            .assert_text("/nest/c/:id")
            .await;
        cli.get("/nest_no_strip/d/99")
            .send()
            .await
            .assert_text("/nest_no_strip/d/:id")
            .await;
        cli.get("/nest1/nest2/10")
            .send()
            .await
            .assert_text("/nest1/nest2/:id")
            .await;
        cli.get("/nest_no_strip1/nest_no_strip2/10")
            .send()
            .await
            .assert_text("/nest_no_strip1/nest_no_strip2/:id")
            .await;
    }

I wonder what goes wrong in my case?

[kolloch]

Ah, I assume that the middleware just "takes" the PathPattern at the level it is added.

The test gets the PathPattern inside the the http call itself so it has all the nesting. This is different from what the middleware does if you "mount" it globally.

Can you somehow add it nested to all routes without adding it manually to all leaf routes?