metrics: transition all metrics to using metricsgen generated constructors. (port of #8488) #9178
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ctors. (#8488) This pull request completes the change to the `metricsgen` metrics. It adds `go generate` directives to all of the files containing the `Metrics` structs. Using the outputs of `metricsdiff` between these generated metrics and `master`, we can see that there is not a diff between the two sets of metrics when run locally. ``` [william@sidewinder] tendermint[wb/metrics-gen-transition]:. ◆ ./scripts/metricsgen/metricsdiff/metricsdiff metrics_master metrics_generated [william@sidewinder] tendermint[wb/metrics-gen-transition]:. ◆ ``` This change also adds parsing for a `metrics:` key in a field comment. If a comment line begins with `//metrics:` the rest of the line is interpreted to be the metric help text. Additionally, a bug where lists of labels were not properly quoted in the `metricsgen` rendered output was fixed.
tychoish
approved these changes
Aug 6, 2022
thanethomson
approved these changes
Aug 6, 2022
williambanfield
added
the
S:automerge
3 tasks
samricotta
pushed a commit
that referenced
this pull request
Aug 9, 2022
…uctors. (port of #8488) (#9178) This pull request completes the change to the `metricsgen` metrics. It adds `go generate` directives to all of the files containing the `Metrics` structs. Using the outputs of `metricsdiff` between these generated metrics and `main`, we can see that there is a minimal diff between the two sets of metrics when run locally. The diff here stems from removal of the word 'message' which was done in v0.36+ and is ultimately a better phrasing. This metric has not yet been released, so this phrasing is preferred. ``` ./metricsdiff old new Metric changes: +++ tendermint_consensus_full_prevote_delay +++ tendermint_consensus_quorum_prevote_delay --- tendermint_consensus_full_prevote_message_delay --- tendermint_consensus_quorum_prevote_message_delay ``` This change also adds parsing for a `metrics:` key in a field comment. If a comment line begins with `//metrics:` the rest of the line is interpreted to be the metric help text. Additionally, a bug where lists of labels were not properly quoted in the `metricsgen` rendered output was fixed. In my view, docs and tests are not needed for this internal only change. --- #### PR checklist - [ ] Tests written/updated, or no tests needed - [ ] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [ ] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed Ports #8488 to main
samricotta
pushed a commit
that referenced
this pull request
Aug 10, 2022
…uctors. (port of #8488) (#9178) This pull request completes the change to the `metricsgen` metrics. It adds `go generate` directives to all of the files containing the `Metrics` structs. Using the outputs of `metricsdiff` between these generated metrics and `main`, we can see that there is a minimal diff between the two sets of metrics when run locally. The diff here stems from removal of the word 'message' which was done in v0.36+ and is ultimately a better phrasing. This metric has not yet been released, so this phrasing is preferred. ``` ./metricsdiff old new Metric changes: +++ tendermint_consensus_full_prevote_delay +++ tendermint_consensus_quorum_prevote_delay --- tendermint_consensus_full_prevote_message_delay --- tendermint_consensus_quorum_prevote_message_delay ``` This change also adds parsing for a `metrics:` key in a field comment. If a comment line begins with `//metrics:` the rest of the line is interpreted to be the metric help text. Additionally, a bug where lists of labels were not properly quoted in the `metricsgen` rendered output was fixed. In my view, docs and tests are not needed for this internal only change. --- #### PR checklist - [ ] Tests written/updated, or no tests needed - [ ] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [ ] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed Ports #8488 to main
samricotta
pushed a commit
that referenced
this pull request
Aug 11, 2022
…uctors. (port of #8488) (#9178) This pull request completes the change to the `metricsgen` metrics. It adds `go generate` directives to all of the files containing the `Metrics` structs. Using the outputs of `metricsdiff` between these generated metrics and `main`, we can see that there is a minimal diff between the two sets of metrics when run locally. The diff here stems from removal of the word 'message' which was done in v0.36+ and is ultimately a better phrasing. This metric has not yet been released, so this phrasing is preferred. ``` ./metricsdiff old new Metric changes: +++ tendermint_consensus_full_prevote_delay +++ tendermint_consensus_quorum_prevote_delay --- tendermint_consensus_full_prevote_message_delay --- tendermint_consensus_quorum_prevote_message_delay ``` This change also adds parsing for a `metrics:` key in a field comment. If a comment line begins with `//metrics:` the rest of the line is interpreted to be the metric help text. Additionally, a bug where lists of labels were not properly quoted in the `metricsgen` rendered output was fixed. In my view, docs and tests are not needed for this internal only change. --- #### PR checklist - [ ] Tests written/updated, or no tests needed - [ ] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [ ] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed Ports #8488 to main
samricotta
pushed a commit
that referenced
this pull request
Aug 11, 2022
…uctors. (port of #8488) (#9178) This pull request completes the change to the `metricsgen` metrics. It adds `go generate` directives to all of the files containing the `Metrics` structs. Using the outputs of `metricsdiff` between these generated metrics and `main`, we can see that there is a minimal diff between the two sets of metrics when run locally. The diff here stems from removal of the word 'message' which was done in v0.36+ and is ultimately a better phrasing. This metric has not yet been released, so this phrasing is preferred. ``` ./metricsdiff old new Metric changes: +++ tendermint_consensus_full_prevote_delay +++ tendermint_consensus_quorum_prevote_delay --- tendermint_consensus_full_prevote_message_delay --- tendermint_consensus_quorum_prevote_message_delay ``` This change also adds parsing for a `metrics:` key in a field comment. If a comment line begins with `//metrics:` the rest of the line is interpreted to be the metric help text. Additionally, a bug where lists of labels were not properly quoted in the `metricsgen` rendered output was fixed. In my view, docs and tests are not needed for this internal only change. --- #### PR checklist - [ ] Tests written/updated, or no tests needed - [ ] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [ ] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed Ports #8488 to main
samricotta
pushed a commit
that referenced
this pull request
Aug 12, 2022
…uctors. (port of #8488) (#9178) This pull request completes the change to the `metricsgen` metrics. It adds `go generate` directives to all of the files containing the `Metrics` structs. Using the outputs of `metricsdiff` between these generated metrics and `main`, we can see that there is a minimal diff between the two sets of metrics when run locally. The diff here stems from removal of the word 'message' which was done in v0.36+ and is ultimately a better phrasing. This metric has not yet been released, so this phrasing is preferred. ``` ./metricsdiff old new Metric changes: +++ tendermint_consensus_full_prevote_delay +++ tendermint_consensus_quorum_prevote_delay --- tendermint_consensus_full_prevote_message_delay --- tendermint_consensus_quorum_prevote_message_delay ``` This change also adds parsing for a `metrics:` key in a field comment. If a comment line begins with `//metrics:` the rest of the line is interpreted to be the metric help text. Additionally, a bug where lists of labels were not properly quoted in the `metricsgen` rendered output was fixed. In my view, docs and tests are not needed for this internal only change. --- #### PR checklist - [ ] Tests written/updated, or no tests needed - [ ] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [ ] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed Ports #8488 to main
samricotta
pushed a commit
that referenced
this pull request
Aug 16, 2022
…uctors. (port of #8488) (#9178) This pull request completes the change to the `metricsgen` metrics. It adds `go generate` directives to all of the files containing the `Metrics` structs. Using the outputs of `metricsdiff` between these generated metrics and `main`, we can see that there is a minimal diff between the two sets of metrics when run locally. The diff here stems from removal of the word 'message' which was done in v0.36+ and is ultimately a better phrasing. This metric has not yet been released, so this phrasing is preferred. ``` ./metricsdiff old new Metric changes: +++ tendermint_consensus_full_prevote_delay +++ tendermint_consensus_quorum_prevote_delay --- tendermint_consensus_full_prevote_message_delay --- tendermint_consensus_quorum_prevote_message_delay ``` This change also adds parsing for a `metrics:` key in a field comment. If a comment line begins with `//metrics:` the rest of the line is interpreted to be the metric help text. Additionally, a bug where lists of labels were not properly quoted in the `metricsgen` rendered output was fixed. In my view, docs and tests are not needed for this internal only change. --- #### PR checklist - [ ] Tests written/updated, or no tests needed - [ ] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [ ] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed Ports #8488 to main
3 tasks
cason
added a commit
to cometbft/cometbft
that referenced
this pull request
Jan 24, 2024
Part of #1731. The original PR precedes tendermint/tendermint#8488, a PR that adopted `metricsgen` for metric generation. This PR was forward ported to this repository as in tendermint/tendermint#9178. Some changes were added to the forward-ported PR to match the behaviour and description of the added metric with this follow-up change. --- #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> Co-authored-by: Sergio Mena <sergio@informal.systems>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request completes the change to the
metricsgenmetrics. It addsgo generatedirectives to all of the files containing theMetricsstructs.Using the outputs of
metricsdiffbetween these generated metrics andmain, we can see that there is a minimal diff between the two sets of metrics when run locally. The diff here stems from removal of the word 'message' which was done in v0.36+ and is ultimately a better phrasing. This metric has not yet been released, so this phrasing is preferred.This change also adds parsing for a
metrics:key in a field comment. If a comment line begins with//metrics:the rest of the line is interpreted to be the metric help text. Additionally, a bug where lists of labels were not properly quoted in themetricsgenrendered output was fixed.In my view, docs and tests are not needed for this internal only change.
PR checklist
CHANGELOG_PENDING.mdupdated, or no changelog entry neededdocs/) and code comments, or nodocumentation updates needed
Ports #8488 to main