Skip to content

[Bug] MM2 should have its own default Strimzi Metrics configuration (#12180)#12277

Merged
ppatierno merged 1 commit intostrimzi:mainfrom
syedazeez337:fix/mm2-metrics-configuration
Jan 7, 2026
Merged

[Bug] MM2 should have its own default Strimzi Metrics configuration (#12180)#12277
ppatierno merged 1 commit intostrimzi:mainfrom
syedazeez337:fix/mm2-metrics-configuration

Conversation

@syedazeez337
Copy link
Copy Markdown
Contributor

@syedazeez337 syedazeez337 commented Dec 29, 2025

This PR fixes GitHub issue #12180 by giving MirrorMaker 2 its own default Strimzi Metrics configuration, separating it from Kafka Connect's configuration.
Problem
Previously, Kafka Connect and MirrorMaker 2 shared the same default metrics configuration, which incorrectly exposed MM2 connector metrics in Kafka Connect's default metrics configuration:

  • kafka_connect_mirror_mirrorcheckpointconnector.*
  • kafka_connect_mirror_mirrorsourceconnector.*
    These MM2-specific metrics were appearing in Kafka Connect deployments even when MirrorMaker 2 wasn't being used.
    Solution
    Code Changes
  1. KafkaConnectCluster.java:
    • Removed MM2-specific metrics from DEFAULT_METRICS_ALLOW_LIST
    • Created DEFAULT_MIRROR_MAKER_2_METRICS_ALLOW_LIST with Connect + MM2 metrics
    • Added getDefaultMetricsAllowList() method for extensibility
    • Updated metrics configuration to use the new method
  2. KafkaMirrorMaker2Cluster.java:
    • Overrode getDefaultMetricsAllowList() to return MM2-specific metrics
  3. Added Tests:
    • testDefaultMetricsConfigurationDoesNotIncludeMM2Metrics()
    • testDefaultMetricsConfigurationIncludesBothConnectAndMM2Metrics()

Testing
All tests pass:
✅ Kafka Connect doesn't include MM2 metrics
✅ MirrorMaker 2 includes both Connect and MM2 metrics
✅ All existing tests still pass

@scholzj scholzj added this to the 0.50.0 milestone Dec 29, 2025
@scholzj scholzj linked an issue Dec 29, 2025 that may be closed by this pull request
[Bug] MM2 should have its own default Strimzi Metrics configuration #12180
Closed
scholzj
scholzj reviewed Dec 29, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I left few comments - but it looks good otherwise.

Comment thread cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaConnectCluster.java Outdated
Comment thread ...r-operator/src/test/java/io/strimzi/operator/cluster/model/KafkaMirrorMaker2ClusterTest.java Outdated
@scholzj scholzj requested a review from ppatierno December 29, 2025 20:16
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 29, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.80%. Comparing base (805612e) to head (10c6c2a).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff            @@
##               main   #12277   +/-   ##
=========================================
  Coverage     74.79%   74.80%           
- Complexity     6626     6631    +5     
=========================================
  Files           376      376           
  Lines         25345    25351    +6     
  Branches       3402     3402           
=========================================
+ Hits          18957    18964    +7     
  Misses         4998     4998           
+ Partials       1390     1389    -1
Files with missing lines Coverage Δ
...zi/operator/cluster/model/KafkaConnectCluster.java 92.32% <100.00%> (+0.02%) ⬆️
...erator/cluster/model/KafkaMirrorMaker2Cluster.java 95.65% <100.00%> (+0.12%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@syedazeez337 syedazeez337 force-pushed the fix/mm2-metrics-configuration branch from e2bad14 to 2504004 Compare December 30, 2025 04:34
@syedazeez337
Copy link
Copy Markdown
Contributor Author

syedazeez337 commented Dec 30, 2025

Hi @scholzj
I have updated my code with your suggestion. Thank you for reviewing my code.
I want to contribute more to this project, I would like to know if there is help needed on certain issues. Let me know, happy to collaborate and do the work.

@scholzj
Copy link
Copy Markdown
Member

scholzj commented Dec 30, 2025

/gha run pipeline=regression

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Dec 30, 2025 •
edited
Loading

⏳ System test verification started: link

The following 6 job(s) will be executed:

  • regression-brokers-and-security-amd64 (oracle-vm-8cpu-32gb-x86-64)
  • regression-operators-amd64 (oracle-vm-8cpu-32gb-x86-64)
  • regression-operands-amd64 (oracle-vm-8cpu-32gb-x86-64)
  • regression-brokers-and-security-arm64 (oracle-vm-8cpu-32gb-arm64)
  • regression-operators-arm64 (oracle-vm-8cpu-32gb-arm64)
  • regression-operands-arm64 (oracle-vm-8cpu-32gb-arm64)

Tests will start after successful build completion.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Dec 30, 2025

🎉 System test verification passed: link

scholzj
scholzj approved these changes Dec 30, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit about the formatting. But looks good to me otherwise. Thanks.

Comment thread cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaConnectCluster.java Outdated
Comment on lines +94 to +97
/**
* Default Strimzi Metrics Reporter allowlist for Kafka Connect.
* Check example dashboard compatibility in case of changes to existing regexes.
*/
Copy link
Copy Markdown
Member

@scholzj scholzj Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this indentation change should not be here?

Copy link
Copy Markdown
Contributor Author

@syedazeez337 syedazeez337 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for pointing it out. It is addressed in the new commit

@syedazeez337
[Bug] MM2 should have its own default Strimzi Metrics configuration s…
10c6c2a 
…trimzi#12180

This commit fixes the issue where Kafka Connect and MirrorMaker 2 shared
the same default Strimzi Metrics configuration, incorrectly exposing MM2
connector metrics in Kafka Connect's default metrics configuration.

Changes:
- Removed MM2-specific metrics (mirrorcheckpointconnector, mirrorsourceconnector)
  from Kafka Connect's DEFAULT_METRICS_ALLOW_LIST
- Created DEFAULT_MIRROR_MAKER_2_METRICS_ALLOW_LIST for MirrorMaker 2 with
  both Connect and MM2-specific metrics
- Added getDefaultMetricsAllowList() method for extensibility
- Added comprehensive unit tests for both components

Signed-off-by: Azeez Syed <syedazeez337@gmail.com>
@syedazeez337 syedazeez337 force-pushed the fix/mm2-metrics-configuration branch from 2504004 to 10c6c2a Compare December 31, 2025 05:16
ppatierno
ppatierno approved these changes Jan 7, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@ppatierno ppatierno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the contribution!

@ppatierno ppatierno merged commit a043ab6 into strimzi:main Jan 7, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@scholzj scholzj scholzj approved these changes

@ppatierno ppatierno ppatierno approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.50.0

Development

Successfully merging this pull request may close these issues.

[Bug] MM2 should have its own default Strimzi Metrics configuration

3 participants

@syedazeez337 @scholzj @ppatierno