consul
consul copied to clipboard
hashicorp
Fix incorrect name and doc for kv_entries metric
The name of the metric as registered with the metrics library to provide the help string, was incorrect compared with the actual code that sets the metric value - bring them into sync.
Also, the help message was incorrect. Rather than copy the help message from telemetry.mdx, which was correct, but felt a bit unnatural in the way it was worded, update both of them to a new wording.
Link to actual code that sets the metric value, for comparison: https://github.com/hashicorp/consul/blob/a407d378af120741ef9fa8745bc54fa2db56644f/agent/consul/usagemetrics/usagemetrics_oss.go#L73
PR that originally introduced the metric, and the discrepancies fixed here: #11090
Thanks adam! Looks straightfoward to me as well, i'll bring this up and hopefully we can just approve the merge tomorrow when the team meets to discuss the prometheus issues
I think this is going to experience a merge conflict with my other open PR touching telemetry.mdx - #13516.
Could you get that other PR - which has been approved already - merged in, so that I can resolve the conflict?
@maxb
Just merged the last one so you're free to add changes.
Also an update on your PRs: I had to push back discussions on all your changes since last week was our annual european conference. Will hopefully get you back feedback shortly.
Sure, I appreciate the complicated stuff is going to take a while to work through :-)
Meanwhile, this little 3-liner now has conflicts resolved.
Please could I get a pr/no-metrics-test label for this PR?
Please could I get a pr/no-changelog label for this PR, unless you think even something this minor warrants a changelog entry?
Hmm, actually, it looks like there is an error in a previous merge, let me fix that...
Hi @maxb we're having an issue with some integration tests not running on forked repos, and I can't override that.
We're looking into how to address this so that the integration tests run and we can merge your PR.
@maxb Could you pull in the latest changes from the main branch? That should allow for the integration tests to run
@maxb Could you pull in the latest changes from the main branch? That should allow for the integration tests to run
Done - all CircleCI checks green now.