Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aggregate reporting for an Express default route #112

Open
kennsippell opened this issue Jun 28, 2023 · 2 comments
Open

Aggregate reporting for an Express default route #112

kennsippell opened this issue Jun 28, 2023 · 2 comments

Comments

@kennsippell
Copy link

kennsippell commented Jun 28, 2023

I have a "default route" in my application which proxies requests to another service I think this is a somewhat common pattern -a default route in Express. Something like:

app.get("*", (req, res) => {
  res.send("DEFAULT ROUTE");
});

When I do this, it is causing some noisey routes in monitoring because each unique path which triggers this route is reported and aggregated separely. For example a request to /does-not-exist.txt is reported as route does-not-exist.txt*.

I'm thinking to resolve this by special casing the '*' route and reporting this in Prometheus under the simple consistent value * (no additional path information).

An alternative solution (if this is not a common case encountered for others), would be to provide a public interface which allows me to define my own mapping from route to string).

@kobik
Copy link
Collaborator

kobik commented Jun 28, 2023

Hi @kennsippell,

Thanks for reporting the issue.
I believe that using wildcard is a valid use-case and should be supported out-the-box, just as we do with variable path params.

As I currently don't have the capacity for handling it myself, I'd gladly accept PRs.

@kennsippell
Copy link
Author

Happy to contribute. I threw together #113.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants