Skip to content

feat(telemetry): expose a check for collisions with built-in server routes - #285

Open
LubabaKhalid wants to merge 1 commit into
cloudflare:mainfrom
LubabaKhalid:feat/telemetry-route-collision-check
Open

LubabaKhalid wants to merge 1 commit into
cloudflare:mainfrom
LubabaKhalid:feat/telemetry-route-collision-check

Conversation

@LubabaKhalid

Copy link
Copy Markdown

Built-in telemetry server routes are inserted before custom routes, so a custom route on the same path and method as a built-in one is silently ignored. This is easy to hit by accident, and a built-in route added in a later release can start shadowing an existing custom route without any warning.

This adds TelemetryServerRoute::collides_with_built_in_route, which returns true if the route collides with a built-in route for any of its methods. Services can call it at startup and fail or warn as they see fit:

for route in &custom_routes {
    if route.collides_with_built_in_route() {
        anyhow::bail!("telemetry route `{}` collides with a built-in route", route.path);
    }
}

The check builds the built-in routes for the enabled features and tries to insert the route into a copy of each method's router, so it follows the same conflict rules as the server itself, with and without telemetry-server-pattern-routing. The parameter escaping that Routes::set did inline moved into a small helper so both use the same path handling.

Testing

  • New test covering collisions (GET /health, GET /metrics), the same path on a method the built-in route does not handle (POST /health), and a path with no built-in route.
  • cargo test -p foundations --test telemetry_server with and without telemetry-server-pattern-routing.
  • cargo clippy -p foundations --all-targets with the CI lint flags, for both feature sets.
  • rustfmt --check --edition 2024.

Closes #200

…outes

Built-in telemetry server routes are inserted before custom routes, so a
custom route on the same path and method is silently ignored. Add
`TelemetryServerRoute::collides_with_built_in_route` so services can
detect this at startup.

The check inserts the route into a copy of each built-in router, using
the same path handling as the server, so it follows the same conflict
rules with and without `telemetry-server-pattern-routing`.
@LubabaKhalid

Copy link
Copy Markdown
Author

@TheJokr this adds the built-in route collision check you suggested. Could you take a look when you have a moment? Thanks!

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.

Expose a check for collisions with built-in telemetry server routes

1 participant