refactor(API): deprecate non-descriptor metric creation methods #154
Conversation
incertum
force-pushed
the
deprecation/help-unit-required-future
branch
from
ac36981 to
0babf92
Compare
|
Additional note: We should avoid redirecting to the public APIs that have now been marked as deprecated. I wanted to first ask for your opinion on this @ktoso . Happy to make these additional changes. |
I've pushed a suggestion. |
ktoso
left a comment
ktoso
left a comment
There was a problem hiding this comment.
Looks good, AFAICS this keeps source compat
Deprecates all metric creation methods that accept loose string parameters for name and help text.
This standardizes metric creation on the MetricNameDescriptor type, which requires a metricName, unitName and helpText for all metrics. This improves compliance with official Prometheus client library guidelines ("A MetricFamily MUST have a name, HELP, TYPE, and UNIT metadata.").
The deprecated methods will be removed in a future major version (see swift-server#145).
Signed-off-by: Melissa Kilby <[email protected]>
Signed-off-by: Melissa Kilby <[email protected]>
incertum
force-pushed
the
deprecation/help-unit-required-future
branch
2 times, most recently
from
5e3e2d5 to
c7bbfdd
Compare
Signed-off-by: Melissa Kilby <[email protected]>
incertum
force-pushed
the
deprecation/help-unit-required-future
branch
from
c7bbfdd to
f1c267b
Compare
|
Please re re-review the latest minor fixes re CI failures, thanks! @FranzBusch @ktoso |
|
Can we avoid removing the warnings as errors setting? Are the deprecation warnings in the tests? If so you an mark the tests as deprecated which will silence those warnings. |
Signed-off-by: Melissa Kilby <[email protected]>
incertum
force-pushed
the
deprecation/help-unit-required-future
branch
from
f1c267b to
e48116e
Compare
While it silenced it locally |
Mark tests as deprecated (for now). They will be refactored in the future. Co-authored-by: Franz Busch <[email protected]> Signed-off-by: Melissa Kilby <[email protected]>
incertum
force-pushed
the
deprecation/help-unit-required-future
branch
from
e48116e to
f70db0f
Compare
It looks like only 5.10 and 6.0 are producing these warnings. Can we instead disable |
incertum
force-pushed
the
deprecation/help-unit-required-future
branch
from
ee9de4a to
e0dc338
Compare
Signed-off-by: Melissa Kilby <[email protected]>
incertum
force-pushed
the
deprecation/help-unit-required-future
branch
from
e0dc338 to
d53cfd7
Compare
I have updated the override matrix (it wasn't yet configured for 6.0 and 6.1). It turns out that while having I would propose to defer this and unblock this PR for now. |
|
I'll go ahead and merge this. If we believe that additional refinements are beneficial, we can open a follow-up PR. |
3 participants
Deprecates all metric creation methods that accept loose string parameters for name and help text.
This standardizes metric creation on the MetricNameDescriptor type, which requires a metricName, unitName and helpText for all metrics. This improves compliance with official Prometheus client library guidelines ("A MetricFamily MUST have a name, HELP, TYPE, and UNIT metadata.").
The deprecated methods will be removed in a future major version (see #145).
CC @ktoso, thank you!