KAFKA-18489 - Fix Connector tasks metrics task-startup-attempts, task…

[ashwinpankaj]

…-startup-success

A minor bug was introduced as part of EOS in Apache Kafka 3 years ago The line

final TaskStatus.Listener taskStatusListener = workerMetricsGroup.wrapStatusListener(statusListener);

wraps herder's listener in workerMetricsGroup's listener. WorkerMetricsGroup is the entity which records task success and failure in the metric .
In the PR, the wrapped listener was missed and instead the herder listener is passed to the task builder (as part of constructor).

workerMetricsGroup's UT already validates that the metrics get updated when tasks fail or succeed.
I did not see a way to test connector's metrics using JMX

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[mimaison]

Thanks for the PR.

I've not looked into this issue but if we're fixing some logic then I was expecting some updates in the tests too. Why are the existing tests passing if you say that some metrics are not updated correctly? Are they not tested?

[ashwinpankaj]

Thanks @mimaison - yes. What we have are unit tests for workermetricsgroup - which ensure that if wrappedListener is used metrics will get updated. But there are no tests to ensure that workermetricsgroup's task status listener is used while building tasks

[mimaison]

But there are no tests to ensure that workermetricsgroup's task status listener is used while building tasks

Ok, then we need to create these tests in this PR. We can't just change code without adding matching tests.

[github-actions]

A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the triage label
should be removed to prevent this automation from happening again.

[ashwinpankaj]

@mimaison thanks ! Can you please point to a test where we check the value of metrics using JMX so that I can create a test based on it ?

[mimaison]

Have a look in WorkerTest, you can find methods like assertStatusMetrics() that check metrics without using JMX.

[github-actions]

A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the triage label
should be removed to prevent this automation from happening again.