Copilot commented on code in PR #22664:
URL: https://github.com/apache/kafka/pull/22664#discussion_r3468263589
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetrics.java:
##########
@@ -442,6 +446,46 @@ public GroupCoordinatorMetrics(MetricsRegistry registry,
Metrics metrics) {
METRICS_GROUP,
"The total number of failed plugin.deleteTopology calls
(DeleteGroups and periodic cleanup combined)")));
+ Sensor streamsGroupTopologyDescriptionSetSuccessSensor =
+
metrics.sensor(STREAMS_GROUP_TOPOLOGY_DESCRIPTION_SET_SUCCESS_SENSOR_NAME);
+ streamsGroupTopologyDescriptionSetSuccessSensor.add(new Meter(
+
metrics.metricName("streams-group-topology-description-set-success-rate",
+ METRICS_GROUP,
+ "The rate of successful plugin.setTopology calls"),
+
metrics.metricName("streams-group-topology-description-set-success-count",
+ METRICS_GROUP,
+ "The total number of successful plugin.setTopology calls")));
+
+ Sensor streamsGroupTopologyDescriptionSetErrorSensor =
+
metrics.sensor(STREAMS_GROUP_TOPOLOGY_DESCRIPTION_SET_ERROR_SENSOR_NAME);
+ streamsGroupTopologyDescriptionSetErrorSensor.add(new Meter(
+
metrics.metricName("streams-group-topology-description-set-error-rate",
+ METRICS_GROUP,
+ "The rate of failed plugin.setTopology calls (any failure:
permanent, transient, or otherwise)"),
+
metrics.metricName("streams-group-topology-description-set-error-count",
+ METRICS_GROUP,
+ "The total number of failed plugin.setTopology calls (any
failure: permanent, transient, or otherwise)")));
+
+ Sensor streamsGroupTopologyDescriptionGetSuccessSensor =
+
metrics.sensor(STREAMS_GROUP_TOPOLOGY_DESCRIPTION_GET_SUCCESS_SENSOR_NAME);
+ streamsGroupTopologyDescriptionGetSuccessSensor.add(new Meter(
+
metrics.metricName("streams-group-topology-description-get-success-rate",
+ METRICS_GROUP,
+ "The rate of successful plugin.getTopology calls (a call
returning null counts as success)"),
+
metrics.metricName("streams-group-topology-description-get-success-count",
+ METRICS_GROUP,
+ "The total number of successful plugin.getTopology calls (a
call returning null counts as success)")));
+
+ Sensor streamsGroupTopologyDescriptionGetErrorSensor =
+
metrics.sensor(STREAMS_GROUP_TOPOLOGY_DESCRIPTION_GET_ERROR_SENSOR_NAME);
+ streamsGroupTopologyDescriptionGetErrorSensor.add(new Meter(
+
metrics.metricName("streams-group-topology-description-get-error-rate",
+ METRICS_GROUP,
+ "The rate of failed plugin.getTopology calls"),
Review Comment:
The get-error meter description says this metric counts failed
*plugin.getTopology* calls, but
`StreamsGroupTopologyDescriptionManager.applyGetTopologyOutcome` also records
this sensor when response conversion throws. To avoid misleading operators,
update the description to reflect that this metric covers any getTopology
operation resulting in an ERROR response (including conversion failures / SPI
contract violations), or stop recording conversion failures against this sensor.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetrics.java:
##########
@@ -442,6 +446,46 @@ public GroupCoordinatorMetrics(MetricsRegistry registry,
Metrics metrics) {
METRICS_GROUP,
"The total number of failed plugin.deleteTopology calls
(DeleteGroups and periodic cleanup combined)")));
+ Sensor streamsGroupTopologyDescriptionSetSuccessSensor =
+
metrics.sensor(STREAMS_GROUP_TOPOLOGY_DESCRIPTION_SET_SUCCESS_SENSOR_NAME);
+ streamsGroupTopologyDescriptionSetSuccessSensor.add(new Meter(
+
metrics.metricName("streams-group-topology-description-set-success-rate",
+ METRICS_GROUP,
+ "The rate of successful plugin.setTopology calls"),
+
metrics.metricName("streams-group-topology-description-set-success-count",
+ METRICS_GROUP,
+ "The total number of successful plugin.setTopology calls")));
+
+ Sensor streamsGroupTopologyDescriptionSetErrorSensor =
+
metrics.sensor(STREAMS_GROUP_TOPOLOGY_DESCRIPTION_SET_ERROR_SENSOR_NAME);
+ streamsGroupTopologyDescriptionSetErrorSensor.add(new Meter(
+
metrics.metricName("streams-group-topology-description-set-error-rate",
+ METRICS_GROUP,
+ "The rate of failed plugin.setTopology calls (any failure:
permanent, transient, or otherwise)"),
+
metrics.metricName("streams-group-topology-description-set-error-count",
+ METRICS_GROUP,
+ "The total number of failed plugin.setTopology calls (any
failure: permanent, transient, or otherwise)")));
+
+ Sensor streamsGroupTopologyDescriptionGetSuccessSensor =
+
metrics.sensor(STREAMS_GROUP_TOPOLOGY_DESCRIPTION_GET_SUCCESS_SENSOR_NAME);
+ streamsGroupTopologyDescriptionGetSuccessSensor.add(new Meter(
+
metrics.metricName("streams-group-topology-description-get-success-rate",
+ METRICS_GROUP,
+ "The rate of successful plugin.getTopology calls (a call
returning null counts as success)"),
+
metrics.metricName("streams-group-topology-description-get-success-count",
+ METRICS_GROUP,
+ "The total number of successful plugin.getTopology calls (a
call returning null counts as success)")));
+
+ Sensor streamsGroupTopologyDescriptionGetErrorSensor =
+
metrics.sensor(STREAMS_GROUP_TOPOLOGY_DESCRIPTION_GET_ERROR_SENSOR_NAME);
+ streamsGroupTopologyDescriptionGetErrorSensor.add(new Meter(
+
metrics.metricName("streams-group-topology-description-get-error-rate",
+ METRICS_GROUP,
+ "The rate of failed plugin.getTopology calls"),
+
metrics.metricName("streams-group-topology-description-get-error-count",
+ METRICS_GROUP,
+ "The total number of failed plugin.getTopology calls")));
Review Comment:
Same issue as the get-error rate description: this count description
currently claims to measure failed *plugin.getTopology* calls, but conversion
errors are also recorded against the get-error sensor. Update the description
to match the actual semantics.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetrics.java:
##########
@@ -442,6 +446,46 @@ public GroupCoordinatorMetrics(MetricsRegistry registry,
Metrics metrics) {
METRICS_GROUP,
"The total number of failed plugin.deleteTopology calls
(DeleteGroups and periodic cleanup combined)")));
+ Sensor streamsGroupTopologyDescriptionSetSuccessSensor =
+
metrics.sensor(STREAMS_GROUP_TOPOLOGY_DESCRIPTION_SET_SUCCESS_SENSOR_NAME);
+ streamsGroupTopologyDescriptionSetSuccessSensor.add(new Meter(
+
metrics.metricName("streams-group-topology-description-set-success-rate",
+ METRICS_GROUP,
+ "The rate of successful plugin.setTopology calls"),
+
metrics.metricName("streams-group-topology-description-set-success-count",
+ METRICS_GROUP,
+ "The total number of successful plugin.setTopology calls")));
+
+ Sensor streamsGroupTopologyDescriptionSetErrorSensor =
+
metrics.sensor(STREAMS_GROUP_TOPOLOGY_DESCRIPTION_SET_ERROR_SENSOR_NAME);
+ streamsGroupTopologyDescriptionSetErrorSensor.add(new Meter(
+
metrics.metricName("streams-group-topology-description-set-error-rate",
+ METRICS_GROUP,
+ "The rate of failed plugin.setTopology calls (any failure:
permanent, transient, or otherwise)"),
+
metrics.metricName("streams-group-topology-description-set-error-count",
+ METRICS_GROUP,
+ "The total number of failed plugin.setTopology calls (any
failure: permanent, transient, or otherwise)")));
+
+ Sensor streamsGroupTopologyDescriptionGetSuccessSensor =
+
metrics.sensor(STREAMS_GROUP_TOPOLOGY_DESCRIPTION_GET_SUCCESS_SENSOR_NAME);
+ streamsGroupTopologyDescriptionGetSuccessSensor.add(new Meter(
+
metrics.metricName("streams-group-topology-description-get-success-rate",
+ METRICS_GROUP,
+ "The rate of successful plugin.getTopology calls (a call
returning null counts as success)"),
Review Comment:
This description says the metric counts successful *plugin.getTopology*
calls, but the implementation records `get-error` (not `get-success`) when
conversion to the wire response fails even though the plugin call itself
succeeded. Consider updating the description to reflect that this metric tracks
successful getTopology *operations* (i.e., those that do not surface ERROR),
including null returns.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetrics.java:
##########
@@ -442,6 +446,46 @@ public GroupCoordinatorMetrics(MetricsRegistry registry,
Metrics metrics) {
METRICS_GROUP,
"The total number of failed plugin.deleteTopology calls
(DeleteGroups and periodic cleanup combined)")));
+ Sensor streamsGroupTopologyDescriptionSetSuccessSensor =
+
metrics.sensor(STREAMS_GROUP_TOPOLOGY_DESCRIPTION_SET_SUCCESS_SENSOR_NAME);
+ streamsGroupTopologyDescriptionSetSuccessSensor.add(new Meter(
+
metrics.metricName("streams-group-topology-description-set-success-rate",
+ METRICS_GROUP,
+ "The rate of successful plugin.setTopology calls"),
+
metrics.metricName("streams-group-topology-description-set-success-count",
+ METRICS_GROUP,
+ "The total number of successful plugin.setTopology calls")));
+
+ Sensor streamsGroupTopologyDescriptionSetErrorSensor =
+
metrics.sensor(STREAMS_GROUP_TOPOLOGY_DESCRIPTION_SET_ERROR_SENSOR_NAME);
+ streamsGroupTopologyDescriptionSetErrorSensor.add(new Meter(
+
metrics.metricName("streams-group-topology-description-set-error-rate",
+ METRICS_GROUP,
+ "The rate of failed plugin.setTopology calls (any failure:
permanent, transient, or otherwise)"),
+
metrics.metricName("streams-group-topology-description-set-error-count",
+ METRICS_GROUP,
+ "The total number of failed plugin.setTopology calls (any
failure: permanent, transient, or otherwise)")));
+
+ Sensor streamsGroupTopologyDescriptionGetSuccessSensor =
+
metrics.sensor(STREAMS_GROUP_TOPOLOGY_DESCRIPTION_GET_SUCCESS_SENSOR_NAME);
+ streamsGroupTopologyDescriptionGetSuccessSensor.add(new Meter(
+
metrics.metricName("streams-group-topology-description-get-success-rate",
+ METRICS_GROUP,
+ "The rate of successful plugin.getTopology calls (a call
returning null counts as success)"),
+
metrics.metricName("streams-group-topology-description-get-success-count",
+ METRICS_GROUP,
+ "The total number of successful plugin.getTopology calls (a
call returning null counts as success)")));
Review Comment:
Same issue as the get-success rate description: the count description
currently claims to measure successful *plugin.getTopology* calls, but
conversion failures are counted as get-error despite a successful plugin
return. Update this string to match the actual semantics (successful
getTopology operations).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
