jmx-scraper: reconnect when connection is lost#2910
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the jmx-scraper runtime to better tolerate target restarts by re-establishing the JMX connection after it is lost (addressing issue #2909), and adds an integration test to validate reconnect behavior.
Changes:
- Add a connection wrapper in
JmxScraperthat can drop the currentMBeanServerConnectionon disconnect and lazily reconnect on the next scrape cycle. - Refactor integration-test container orchestration into a shared base class to reduce duplication.
- Add a new integration test that stops and restarts the target container and verifies metrics resume after restart.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java |
Introduces a ConnectionHandler used by the JMX telemetry supplier to reconnect when the connector disconnects. |
jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java |
Simplifies target-system integration tests by inheriting shared container lifecycle from the new base. |
jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/BaseTargetSystemIntegrationTest.java |
New shared integration-test base class containing OTLP test server + container startup/verification helpers. |
jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/ReconnectTest.java |
New integration test that validates scraper reconnect behavior across a target container restart. |
Comment on lines
+26
to
+30
| target.stop(); | ||
| otlpServer.reset(); | ||
| Thread.sleep(2_000); | ||
| List<ExportMetricsServiceRequest> receivedMetrics = otlpServer.getMetrics(); | ||
| assertThat(receivedMetrics).isEmpty(); |
SylvainJuge
approved these changes
Jun 15, 2026
|
|
||
| jmxTelemetry.start(() -> singletonList(connection)); | ||
| try (ConnectionHandler connectionHandler = new ConnectionHandler(client)) { | ||
| ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); |
Contributor
There was a problem hiding this comment.
What are the known cases where the context CL matters here ? It would be worth to document that a bit with a comment.
Contributor
Author
There was a problem hiding this comment.
wildfly tests fail without it, comment added
SylvainJuge
approved these changes
Jun 15, 2026
jaydeluca
approved these changes
Jun 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #2909