-
Notifications
You must be signed in to change notification settings - Fork 14.4k
KAFKA-19207: Move ForwardingManagerMetrics and ForwardingManagerMetricsTest to server module #19574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…csTest to server module Signed-off-by: PoAn Yang <[email protected]>
*/ | ||
private final LatencyHistogram remoteTimeMsHist; | ||
|
||
public final MetricName queueLengthName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we avoid using public member?
METRIC_GROUP_NAME, | ||
"The current number of RPCs that are waiting in the broker's forwarding manager queue, waiting to be sent to the controller." | ||
); | ||
metrics.addMetric(queueLengthName, new FuncGauge<>(now -> queueLength.get())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is FuncGauge
necessary?
Signed-off-by: PoAn Yang <[email protected]>
@chia7712 I addressed all comments. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FrankYang0529 thanks for this patch. overall LGTM
public final MetricName latencyP999Name; | ||
|
||
|
||
public LatencyHistogram(Metrics metrics, String name, String group, long maxLatency) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please use private
constructor?
this.latencyP999Name = metrics.metricName(name + ".p999", group); | ||
|
||
sensor.add(new Percentiles( | ||
4000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please add a static constant for it?
Signed-off-by: PoAn Yang <[email protected]>
…sTest to server module (apache#19574) 1. Move `ForwardingManagerMetrics` and `ForwardingManagerMetricsTest` to server module. 2. Rewrite them in Java. Reviewers: Chia-Ping Tsai <[email protected]>
ForwardingManagerMetrics
andForwardingManagerMetricsTest
toserver module.
Reviewers: Chia-Ping Tsai [email protected]