Skip to content

SOLR-18544: Source CPU utilization metric from JFR instead JMX - #5185

Open
mlbiscoc wants to merge 2 commits into
apache:mainfrom
mlbiscoc:SOLR-18544
Open

mlbiscoc wants to merge 2 commits into
apache:mainfrom
mlbiscoc:SOLR-18544

Conversation

@mlbiscoc

Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/SOLR-18544

Partially a regression from SOLR-18407 but also partially a JDK bug that got exposed from the OTEL upgrade.

What I found was running the PlacementPluginIntegrationTest.testAttributeFetcherImpl with 21.0.6-tem fails but running with 21.0.9-tem passes.

This was because previously the metric was sourced from JFR. Then after the upgrade it started to source it from JMX which exposed from JDK bug. So this change just sources it from JFR again.

@epugh epugh 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.

I love me a java class class called "Experimental" ;-). I wonder if we need the long comment... "Get CPU utlizatoin from JFR instead of JDG to work aroudn bugs"?

@dsmiley dsmiley 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.

Just approving based on what you're saying. I no longer have the older JDK 21 to try it

@nick-boss-tech

Copy link
Copy Markdown
Contributor

Quick Review:
The JFR selector behaves as intended in the pinned OpenTelemetry version: it preserves the existing experimental metrics and suppresses the duplicate JMX CPU source.
P3 readiness issue: the changelog entry is missing, and the current-head validator fails for that reason. A short SOLR-18544 bug-fix entry should cover the operator-visible metric recovery.
Precommit and Crave tests are green. The older-JDK fix remains author-reported; the existing placement assertion checks metric presence, which alone does not prove that JFR samples update its value.
AI-generated static review.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants