[connector/servicegraph] Fix histogram metrics miss unit #34511
Conversation
|
I find the change straightforward and LGTM. I'm more concerned about the situations around this being a breaking change vs. when it isn't. I'm leaning towards this being a breaking change as it is now, so it should either be declared as breaking, or non-breaking if we first have this as opt-in. Then it can be moved to opt-out later on. |
|
@t00mas I believe it will not introduce breaking change if the user uses however, if the user uses |
There was a problem hiding this comment.
LGTM. Codeowners, could you take a look? cc: @jpkrohling @mapno @JaredTan95
|
cc @mapno |
|
@jpkrohling please merge #34933 first |
|
Merged that one. |
Frapschen
force-pushed
the
fix-hisgogram-metrics-miss-unit
branch
from
dec85a1 to
e670ec9
Compare
Signed-off-by: Murphy Chen <[email protected]>
Frapschen
force-pushed
the
fix-hisgogram-metrics-miss-unit
branch
from
e670ec9 to
f9aaee7
Compare
|
@jpkrohling Ready to merge |
atoulme
added
the
ready to merge
…try#34511) I found the servicegraph histogram metrics missing unit. This PR will add a proper unit to metrics like [spanmetrics connector does.]( metric.SetUnit(p.config.Histogram.Unit.String())) Signed-off-by: Murphy Chen <[email protected]>
Description:
I found the servicegraph histogram metrics missing unit.
This PR will add a proper unit to metrics like spanmetrics connector does.
Link to tracking Issue:
Testing:
Documentation: