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

OpenTelemetryMetrics should record matched route pattern only, rather than raw path #337

Closed
DCjanus opened this issue Jul 24, 2022 · 10 comments
Labels
enhancement New feature or request

Comments

@DCjanus
Copy link
Contributor

DCjanus commented Jul 24, 2022

For now, OpenTelemetryMetrics record request uri as metric label, which may cause serious problem, like OOM, in those case

  1. random path scan
  2. param in paths, like /users/:id

Which means, we should record matched route pattern only, in above cases, prometheus should record label /users/:id only, and ignore NotFound responses,

@DCjanus DCjanus added the enhancement New feature or request label Jul 24, 2022
@DCjanus DCjanus changed the title OpenTelemetryMetrics should record matched route name only, rather than raw path OpenTelemetryMetrics should record matched route pattern only, rather than raw path Jul 24, 2022
@DCjanus
Copy link
Contributor Author

DCjanus commented Jul 24, 2022

Unfortunately, for now, structure of Route may hard to impl this feature, I'm not sure how to resolve this issue.

@sunli829
Copy link
Collaborator

It's a very good proposal, and it's totally achievable, I'll add it later.

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Aug 25, 2022
@sunli829 sunli829 removed the Stale label Aug 25, 2022
@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Sep 25, 2022
@Christoph-AK
Copy link
Contributor

I think this is still worth implementing.

@github-actions github-actions bot removed the Stale label Sep 28, 2022
@DCjanus
Copy link
Contributor Author

DCjanus commented Oct 10, 2022

To achieve this goal, we have to re-design Route module, is there anyone would like to handle this, or post some web-framework-route module design to help us.

@sunli829
Copy link
Collaborator

I'll start working on this later.

@sunli829
Copy link
Collaborator

I added a path_pattern label to OpenTelemetryMetrics middleware in master branch, please help me test it.

sunli829 added a commit that referenced this issue Oct 11, 2022
@DCjanus
Copy link
Contributor Author

DCjanus commented Oct 15, 2022

is there any exmaple for path_pattern?I'm not sure how to use it

@sunli829
Copy link
Collaborator

You don't need to do anything, I've added the http.path_pattern label to the logs in the OpenTelemetryMetrics middleware.

if let Some(path_pattern) = req.data::<PathPattern>() {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants