KAFKA-9441: Cleanup Streams metrics for removed task commit latency metrics

[mjsax]

Follow up to #8218 (KIP-447) that remove usage of task commit latency sensor. However, the underlying metrics were still registered, even if unused. This PR does the follow up cleanup to remove the metrics entirely.

The thread level commit metrics are no rollups but tracked individually and thus are not affected.

Call for review @cadonna @abbccdda @guozhangwang

[guozhangwang]

I thought we are still going to retain the thread-level commit-latency, by making its own recording calls than relying on its children metrics (which are removed now), is that the case? Or are you planning to add it in another PR?

I’d suggest we use one PR to do this refactoring rather than two PRs to remove it and then add it back.

[abbccdda]

LGTM

[mjsax]

I thought we are still going to retain the thread-level commit-latency, by making its own recording calls than relying on its children metrics (which are removed now), is that the case? Or are you planning to add it in another PR?

We do. As explained in the PR description:

The thread level commit metrics are no rollups but tracked individually and thus are not affected.

[guozhangwang]

Maybe I'm missing sth here. Where do we record the AvgAndMaxLatency at thread level now?

[mjsax]

This code never changed with KIP-447 work:

The sensor is define here: https://github.com/apache/kafka/blob/trunk/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java#L801

And used here:

• https://github.com/apache/kafka/blob/trunk/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java#L661
• https://github.com/apache/kafka/blob/trunk/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java#L801
• https://github.com/apache/kafka/blob/trunk/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java#L821

[guozhangwang]

LGTM! My previous comment was actually void so it should be good to go.

[mjsax]

Rebased to resolve merge conflict.

Retest this please.

[mjsax]

Java. 8 passed.
Java 11 kafka.api.ConsumerBounceTest.testRollingBrokerRestartsWithSmallerMaxGroupSizeConfigDisruptsBigGroup

Retest this please.

[mjsax]

QueryableStateIntegrationTest failed for both runs.

Retest this please

[mjsax]

Java 8 passed:
Java 11:

QueryableStateIntegrationTest.concurrentAccesses
RecordCollectorTest.shouldNotThrowStreamsExceptionOnSubsequentCallIfASendFailsWithContinueExceptionHandler

Retest this please.

[mjsax]

Java 8: QueryableStateIntegrationTest.concurrentAccesses
Java 11 passed.

[cadonna]

@mjsax Thank you for the PR

Overall LGTM

I have just one request regarding the test you removed in ThreadMetricsTest.

[cadonna]

req: Please keep this test and do only remove the following part (and of course the parts that become obsolete):

StreamsMetricsImpl.addAvgAndMaxToSensor(
            expectedSensor,
            TASK_LEVEL_GROUP,
            tagMap,
            operationLatency,
            avgLatencyDescription,
            maxLatencyDescription
        );

[mjsax]

Ack. Good call.