Refactoring and metric wrapper unification#313
Conversation
Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
…n core) Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
…s to allow for that) Signed-off-by: Aarni Koskela <akx@iki.fi>
Also gets rid of the LabelWrapper class; all top-level metric classes can now act in both labeled and unlabeled modes Fixes prometheus#210 Signed-off-by: Aarni Koskela <akx@iki.fi>
Signed-off-by: Aarni Koskela <akx@iki.fi>
|
I'm in the middle of adding various openmetrics-related stuff, so I'll look at this properly once that's all released. One question from a quick peek is why you need two classes to replace label wrapper. |
|
With this class structure, one can do g = Gauge(..., ['label1', 'label2'])
g.labels('v1', 'v2').set(123)but g = Gauge(..., ['label1', 'label2'])
g.labels('v1', 'v2').labels('v3', 'v4').set(123)is impossible, since If we want to allow |
|
Hmm, I'm seeing deadlocks in multiproc mode in the GC collector (which is actually understandable, since GC callbacks could get triggered at any point) with this PR. @brian-brazil, do you think changing Haven't yet tested whether |
|
I suspect it does have the same problem. @tcolgate |
|
Nope, just reproed it with 0.4.0. Opening a ticket. |
|
Closing as superseded by #344. |
2 participants
This PR does a couple things. (If you want, I can split these into separate PRs.)
🔧 Firstly, it refactors the 1200+-line
core.pyinto separate modules for each functionality. This does not affect imports for client code; all the sameprometheus_client-provided non-underscored names are re-exported from that file. Some previously underscored names were brought into the light.✨ Secondly (and this is what prompted me to really do this work), it fixes #210 by removing the factory function magic decorator
MetricWrapper; as it was before, say, an invocationGauge(...)could either return aGaugeobject, as you'd expect, or a_LabelWrapperif you'd passed in a list of label names. This tripped up code completion (and tbqh, me :) ) and static analysis.Instead, the six metric wrappers (Counter, Enum, Gauge, Histogram, Info, Summary) are now plain old classes (though with a slightly arcane diamond-shaped inheritance hierarchy that lets them be used both with and without
.labels()).👓 Thirdly, there was a mostly mechanical
autopep8pass to placatetox flake8errors. (That check isn't run by CI, it seems?)No test code was changed for this (aside from the removal of the
__wrapped__attribute as there is nothing to wrap anymore), so I would say this is backwards API compatible (with the exception of client code that might have done checks likeisinstance(x, _LabelWrapper)).I'm glad to answer any questions and/or modify things as needed! :)