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

Path Normalization #3

Closed
phnx47 opened this issue Aug 13, 2018 · 11 comments
Closed

Path Normalization #3

phnx47 opened this issue Aug 13, 2018 · 11 comments
Assignees

Comments

@phnx47
Copy link
Member

phnx47 commented Aug 13, 2018

Add 'Path Normalization' with regex.

Dirty version: https://github.com/PrometheusClientNet/Prometheus.Client.HttpRequestDurations/pull/1/files

@phnx47 phnx47 self-assigned this Aug 13, 2018
@zihotki
Copy link

zihotki commented Sep 19, 2018

Another option could be to use Route (api/cities/{id}) instead of a Path (api/cities/2342) and avoid using RegExes if possible. This way it'll be also easier to detect wildcard routes "/api/search/{*searchString}". What do you think?

@phnx47
Copy link
Member Author

phnx47 commented Sep 19, 2018

I will write prototype for test. And then I'll think about how best to work with it.

@dmeenhuis
Copy link

Any progress with this? I'm trying to get metrics for request durations that also include paths, but it would be a lot more useful if there is a way to normalize ID values.

@phnx47
Copy link
Member Author

phnx47 commented Jan 16, 2019

@dmeenhuis I will come back to this issue soon.

Thank you!

@dmeenhuis
Copy link

Thanks! I managed to fork this repo and incorporate the changes in the linked PR and for now that seems to work fine, although it contains a minor bug for the integer normalization (it doesn't properly deal with integer values at the end of the path, where there is no trailing slash).

Would love to see an update from your end, thanks for this very useful library so far 👍

@zihotki
Copy link

zihotki commented Jan 22, 2019

Thanks! I managed to fork this repo and incorporate the changes in the linked PR and for now...

@dmeenhuis Is it me or you didn't push or create a PR? I can't find it in this repo

@dmeenhuis
Copy link

It's not you, I didn't actually push the changes yet. Can do that tomorrow if you like.

@phnx47
Copy link
Member Author

phnx47 commented Jan 25, 2019

I finished work with this:prom-client-net/prom-client#18

Now, I can come back to this issue

phnx47 added a commit that referenced this issue Jan 27, 2019
phnx47 added a commit that referenced this issue Jan 27, 2019
phnx47 added a commit that referenced this issue Jan 29, 2019
phnx47 added a commit that referenced this issue Jan 29, 2019
@phnx47
Copy link
Member Author

phnx47 commented Feb 2, 2019

Released. 1.1.0

@phnx47
Copy link
Member Author

phnx47 commented Feb 2, 2019

Regex examples

@phnx47
Copy link
Member Author

phnx47 commented Feb 2, 2019

If something does not work, then reopen.

@phnx47 phnx47 closed this as completed Feb 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants