This repository has been archived by the owner on Jul 31, 2023. It is now read-only.
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.
Include the const labels in
baseMetric.upsertEntry
in the same way asbaseMetric.entryForValues
.The "Derived" form of metrics use an
UpsertEntry
method to provide the function that supplies the metric value as well as the label values. These methods usebaseMetric.upsertEntry
to do the work, but that method was ignoring any const labels set byWithConstLabel
.Commit c31d268 added const labels to metrics and updated
baseMetric.entryForValues
to add the const labels, butbaseMetric.upsertEntry
was not similarly updated.This causes
UpsertEntry
to return anerrKeyValueMismatch
error ("must supply the same number of label values as keys used to construct this metric") when called unless the values for the const labels were passed toUpsertEntry
. That kind of defeats the purpose of const labels.Add a test case for const labels on Int64DerivedGauge. It is not necessary to add test cases for all the derived metrics as they all use the same
baseMetric
implementation.