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

topdown: Use an underscore instead of a dot in http.send metric name. #2695

Merged

Conversation

koponen-styra
Copy link
Contributor

Currently the recorded metric name is "timer_rego_builtin_http.send_ns" which seems inconsistent.

This is not backwards compatible but before even trying to address that, I thought I should raise the PR to understand is the naming intentional.

Signed-off-by: Teemu Koponen koponen@styra.com

@tsandall
Copy link
Member

It's consistent in the sense that _ was used to separate categories and . wasn't treated as a special character. It sounds like . causes issues for some implementations that consume these metrics so this change seems fine to me. This metric was only added in v0.23 and it's unlikely that anyone is depending on it.

@koponen-styra could you add a section to the CHANGELOG.md file under Unreleased:

### Backwards Compatibility

* Renamed `timer_rego_builtin_http.send_ns` to `timer_rego_builtin_http_send_ns` to avoid issues with periods in metric keys.

Currently the recorded metric name is
"timer_rego_builtin_http.send_ns" which causes issues for some
implementations.

Signed-off-by: Teemu Koponen <koponen@styra.com>
@tsandall tsandall merged commit 5433a93 into open-policy-agent:master Sep 15, 2020
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.

2 participants