Backward compatibility for Prometheus metrics names #4955
Conversation
jutaro
force-pushed
the
jutaro/metrics-mapping
branch
3 times, most recently
from
ace06c3 to
cc2e142
Compare
jutaro
requested review from
Jimbo4350,
newhoggy,
coot,
a team,
mgmeier and
fmaste
as code owners
| # Just an example for metrics compatibility mapping | ||
| # metricsComp = { | ||
| # "Mempool.TxsInMempool" = "Mempool.TxsInMempool.Mapped"; | ||
| # "ChainDB.SlotNum" = "ChainDB.SlotNum.Mapped"; |
There was a problem hiding this comment.
@jutaro, just one thing -- can we change names in this example to make it more clear -- what metric already exists, and what will be created? If I didn't have the context, I'd be a bit confused by it.
Also, another question -- can we flip the order?
It's a bit weird that the created metrics are on the right of what reads like an assignment operator.
There was a problem hiding this comment.
What names to you think make it clearer. I thought adding the ".Mapped" postfix makes it clear.
There was a problem hiding this comment.
It is a mapping from current name to new name. I think this is pretty straightforward, and reversing it would not make it clearer in my view ?!?
There was a problem hiding this comment.
It all hinges on how you read the assignment operator; its meaning for ("X" = "Y") being
"X" has alias "Y"
right?
Without knowing how the resulting Map Text Text is applied in code, it's not straightforward; it would be nice to have that clarification in the comment, for someone just wanting to adjust the .nix service definition.
There was a problem hiding this comment.
There is a lot of mathematical baggage, where we define:
- something on the left
- to be equal to something on the right
..which, on the surface might feel like it's imperative PL bullshit, but it actually runs deeper.
Can you think of a single definition where the "new thing" appears on the right?
There was a problem hiding this comment.
There are further reasons:
- maps are asymmetric -- the thing on the left is unique, the thing on the right not so
- it makes sense to allow multiple aliases for the same source metric
- it doesn't make sense to have a single alias to be related to several source metrics
The status quo allows the strange thing and disallows the silly-but-meaningful thing.
cardano-tracer/src/Cardano/Tracer/Handlers/Metrics/Prometheus.hs
Outdated
Show resolved
Hide resolved
| Nothing -> p' : accu | ||
| Just rep -> p' : (rep,mv) : accu) | ||
| [] | ||
| metricsList |
There was a problem hiding this comment.
Has metricsList to be ordered somehow (e.g. identical mvs must be subsequent)?
I was wondering about the fold...
There was a problem hiding this comment.
Only one mapping per entry is supported, and then order doesn't matter.
| # Just an example for metrics compatibility mapping | ||
| # metricsComp = { | ||
| # "Mempool.TxsInMempool" = "Mempool.TxsInMempool.Mapped"; | ||
| # "ChainDB.SlotNum" = "ChainDB.SlotNum.Mapped"; |
There was a problem hiding this comment.
It all hinges on how you read the assignment operator; its meaning for ("X" = "Y") being
"X" has alias "Y"
right?
Without knowing how the resulting Map Text Text is applied in code, it's not straightforward; it would be nice to have that clarification in the comment, for someone just wanting to adjust the .nix service definition.
jutaro
force-pushed
the
jutaro/metrics-mapping
branch
3 times, most recently
from
0de686a to
964bafd
Compare
jutaro
force-pushed
the
jutaro/metrics-mapping
branch
from
5c36b7a to
a30f5ca
Compare
|
bors r+ |
4955: Backward compatibility for Prometheus metrics names r=jutaro a=jutaro
With configuration entries
metricsComp = {
"a" = "b";
...
};
metrics with name a as well appear with name b in Prometheus. The names in the configuration has to be specified as in the original message, and not as in the Prometheus export (Where e.g. a '.' becomes an '_').
Co-authored-by: Yupanqui <[email protected]>
|
This PR was included in a batch that successfully built, but then failed to merge into master. It will not be retried. Additional information: {"message":"Validation Failed","documentation_url":"https://docs.github.com/articles/about-protected-branches"} |
With configuration entries
metricsComp = {
"a" = "b";
...
};
metrics with name a as well appear with name b in Prometheus. The names in the configuration has to be specified as in the original message, and not as in the Prometheus export (Where e.g. a '.' becomes an '_').