Emit emitter/buffers/allocated/delta and emitter/buffers/failed/delta #6425
Conversation
gianm
left a comment
gianm
left a comment
There was a problem hiding this comment.
LGTM. There is one thing that is technically incompatible, but I'm not sure it matters, since the old behavior was never documented. I will leave this open in case anyone else is interested in commenting.
| { | ||
| int newTotalAllocatedBuffers = httpPostEmitter.getTotalAllocatedBuffers(); | ||
| int allocatedBuffersDelta = newTotalAllocatedBuffers - lastTotalAllocatedBuffers; | ||
| emitter.emit(builder.build("emitter/buffers/allocated/total", newTotalAllocatedBuffers)); |
There was a problem hiding this comment.
This isn't compatible with the old name emitter/buffers/totalAllocated, but I think it's ok because we never documented the old names anyway.
|
Got that, then it looks good to me. |
nishantmonu51
left a comment
nishantmonu51
left a comment
There was a problem hiding this comment.
I am in favor of removing total in this PR as it anyways changes the name of metrics in an incompatible way and it does not make much sense to have totals when we are emitting deltas.
|
I removed |
5 participants
@gianm gauge
emitter/buffers/failedandemitter/buffers/totalAllocatedmetrics proved to be very inconvenient in practice: they are not additive across a fleet of nodes, are randomly reset when a JVM restarts (that may be completely independent from metrics emitting issues). Regarding problem identification,emitter/buffers/droppedserves this well.Currently in this PR I added
/deltavs./totalmetrics, but if there are no objections I would remove/totaland so alignemitter/buffers/allocatedand/failedwith/droppedand/events/emittedwhich are already delta-only.