Skip to content

ZOOKEEPER-4324: dropwizard-metrics should be optional - #2402

Merged
anmolnar merged 3 commits into
apache:masterfrom
dsmiley:ZK-4324-dropWizardDepOpt2
Sep 17, 2026
Merged

ZOOKEEPER-4324: dropwizard-metrics should be optional#2402
anmolnar merged 3 commits into
apache:masterfrom
dsmiley:ZK-4324-dropWizardDepOpt2

Conversation

@dsmiley

@dsmiley dsmiley commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

See https://issues.apache.org/jira/browse/ZOOKEEPER-4324

I verified this change in Solr -- I was able to start Solr in SolrCloud mode with the following setting in its zoo.cfg: metricsProvider.className=org.apache.zookeeper.metrics.impl.NullMetricsProvider

Ideally I'd add an "ArchUnit" test as well, as I did in #2374

maoling
maoling approved these changes Jun 26, 2026
View reviewed changes

@maoling maoling left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR fixes an issue where external projects (e.g. Solr) could still encounter ClassNotFoundException or build failures even after excluding Dropwizard and configuring metricsProvider.className=org.apache.zookeeper.metrics.impl.NullMetricsProvider.

I tested this change with PrometheusMetricsProvider, DefaultMetricsProvider, and NullMetricsProvider, and all three configurations work as expected.

@dsmiley

dsmiley commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

@PDavid could you please consider merging this?

@dsmiley
dsmiley force-pushed the ZK-4324-dropWizardDepOpt2 branch from 1970197 to dc177b2 Compare September 15, 2026 18:59
@dsmiley

dsmiley commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

@anmolnar maybe this PR is of interest to you as well. I brought it up to date. Pom.xml already has "archunit". This PR adds an archunit test to ensure the dropwizard dependency is isolated.

PDavid
PDavid approved these changes Sep 16, 2026
View reviewed changes

@PDavid PDavid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this looks good to me. Also thanks for adding the ArchUnit test. 👍

anmolnar
anmolnar approved these changes Sep 17, 2026
View reviewed changes

@anmolnar anmolnar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚢
Thanks @dsmiley !

@anmolnar
anmolnar merged commit 508e473 into apache:master Sep 17, 2026
17 checks passed
asf-gitbox-commits pushed a commit that referenced this pull request Sep 17, 2026
@dsmiley @anmolnar
ZOOKEEPER-4324: dropwizard-metrics should be optional
12e6150 
Reviewers: maoling, PDavid, anmolnar
Author: dsmiley
Closes #2402 from dsmiley/ZK-4324-dropWizardDepOpt2

(cherry picked from commit 508e473)
Signed-off-by: Andor Molnar <andor@cloudera.com>
@anmolnar

anmolnar commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Merged to master and I backported it to branch-3.9 as well.

@anmolnar

anmolnar commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Acted to fast. branch-3.9 doesn't have archunit dependency, because we didn't backport the optional Netty patch. Reverted.

@dsmiley

dsmiley commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

It would have been simple, I think, to simply add the dependency to pom.xml accordingly?

@anmolnar

anmolnar commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

It would have been simple, I think, to simply add the dependency to pom.xml accordingly?

Or we just keep the two patches (Netty + dropwizard) in the master branch only and release with 3.10.0 soon.
Any objection?

@dsmiley

dsmiley commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

Nothing I said should be taken as impacting the 3.10 release. It's a question of a future 9.x release benefiting. Your call.

@anmolnar

anmolnar commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Nothing I said should be taken as impacting the 3.10 release. It's a question of a future 9.x release benefiting. Your call.

I leave it up for the community. Anybody wants to see them in 3.9.x, open PR and I'll be happy to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PDavid PDavid PDavid approved these changes
@anmolnar anmolnar anmolnar approved these changes
@maoling maoling maoling approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@dsmiley @anmolnar @PDavid @maoling