Conversation
There was a problem hiding this comment.
can we please call this ingest/events/buffered
also please add this to docs
There was a problem hiding this comment.
Sure. Changed metric name and added into metric doc.
guobingkun
force-pushed
the
event_receiver_firehose_monitor
branch
3 times, most recently
from
83fb430 to
6f07a96
Compare
There was a problem hiding this comment.
I believe this metric will not get published by default, user would have to add that to druid.monitor.monitors property ?
There was a problem hiding this comment.
Yeah, you are right. Modified doc.
guobingkun
force-pushed
the
event_receiver_firehose_monitor
branch
2 times, most recently
from
0bd351d to
515eb31
Compare
|
LGTM |
guobingkun
force-pushed
the
event_receiver_firehose_monitor
branch
from
e0062a2 to
c4d3653
Compare
|
Is it possible to have a generic FirehoseMonitor which can be configured in runtime.properties and emits events related to the firehose in use e.g buffered/events, batch size etc for eventReceiverFirehose and for kafka firehose emit kafka related metrics like kafka backlog etc. |
|
@nishantmonu51 The metrics being exposed here are pretty specific to the |
guobingkun
force-pushed
the
event_receiver_firehose_monitor
branch
from
c4d3653 to
86d1e4d
Compare
|
@nishantmonu51 further thoughts? |
|
@cheddar, @guobingkun, by generic i meant is it possible to have the glue code for emitting metrics (like FirehoseRegister and Monitor) to be reUsable by multiple firehoses so that If we implement metrics for any other firehose we wont have to write similar things again and the logic for what metrics to be emitted can be part of the firehose itself and will be specific to them. |
|
btw, I am also fine with doing things as per the current impl and restructuring code when we need to emit metrics for other firehoses too. |
|
@nishantmonu51 It's possible to make a generic FirehoseRegister + FirehoseMonitor and let each firehose implement a FirehoseMetric. Though I prefer not to do it now since it's quite a different direction from what this PR was originally going, there will also be different concurrent issues need to be resolved if we do it in a generic way. |
|
👍 |
There was a problem hiding this comment.
Can you check return value and log if not found?
add an EventReceiverFirehoseMonitor so that we can monitor how many events have been queued in the EventReceiverFirehose and get a sense about whether the firehose is under too much pressure.
guobingkun
force-pushed
the
event_receiver_firehose_monitor
branch
from
86d1e4d to
c3b6fcc
Compare
|
bouncing for unrelated test failure. |
7 participants
add an EventReceiverFirehoseMonitor so that we can monitor how many
events have been queued in the EventReceiverFirehose and get a sense
about whether the firehose is under too much pressure.