-
Notifications
You must be signed in to change notification settings - Fork 146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
jmx scraper connection test #1684
Merged
Merged
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
0e0d106
add test mode with CLI args
SylvainJuge 6388684
allow test mode with jmx scraper container
SylvainJuge f213ecd
test connection with container-to-container
SylvainJuge 2075314
remove connection test + fixed port mapping
SylvainJuge f9841f2
add a few comments for clarity
SylvainJuge 5ade3bd
rename a bit
SylvainJuge 28b04c0
provide error msg to end-user in CLI
SylvainJuge 4ba98a4
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-c…
SylvainJuge adfcd48
document test mode
SylvainJuge 935e235
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-c…
SylvainJuge File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
133 changes: 133 additions & 0 deletions
133
...raper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,133 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.contrib.jmxscraper; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
import java.util.function.Function; | ||
import org.junit.jupiter.api.AfterAll; | ||
import org.junit.jupiter.api.BeforeAll; | ||
import org.junit.jupiter.api.Test; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.testcontainers.containers.GenericContainer; | ||
import org.testcontainers.containers.Network; | ||
import org.testcontainers.containers.output.Slf4jLogConsumer; | ||
|
||
/** | ||
* Tests all supported ways to connect to remote JMX interface. This indirectly tests | ||
* JmxConnectionBuilder and relies on containers to minimize the JMX/RMI network complications which | ||
* are not NAT-friendly. | ||
*/ | ||
public class JmxConnectionTest { | ||
|
||
// OTLP endpoint is not used in test mode, but still has to be provided | ||
private static final String DUMMY_OTLP_ENDPOINT = "http://dummy-otlp-endpoint:8080/"; | ||
private static final String SCRAPER_BASE_IMAGE = "openjdk:8u342-jre-slim"; | ||
|
||
private static final int JMX_PORT = 9999; | ||
private static final String APP_HOST = "app"; | ||
|
||
private static final Logger jmxScraperLogger = LoggerFactory.getLogger("JmxScraperContainer"); | ||
private static final Logger appLogger = LoggerFactory.getLogger("TestAppContainer"); | ||
|
||
private static Network network; | ||
|
||
@BeforeAll | ||
static void beforeAll() { | ||
network = Network.newNetwork(); | ||
} | ||
|
||
@AfterAll | ||
static void afterAll() { | ||
network.close(); | ||
} | ||
|
||
@Test | ||
void connectionError() { | ||
try (JmxScraperContainer scraper = scraperContainer().withRmiServiceUrl("unknown_host", 1234)) { | ||
scraper.start(); | ||
waitTerminated(scraper); | ||
checkConnectionLogs(scraper, /* expectedOk= */ false); | ||
} | ||
} | ||
|
||
@Test | ||
void connectNoAuth() { | ||
connectionTest( | ||
app -> app.withJmxPort(JMX_PORT), scraper -> scraper.withRmiServiceUrl(APP_HOST, JMX_PORT)); | ||
} | ||
|
||
@Test | ||
void userPassword() { | ||
String login = "user"; | ||
String pwd = "t0p!Secret"; | ||
connectionTest( | ||
app -> app.withJmxPort(JMX_PORT).withUserAuth(login, pwd), | ||
scraper -> scraper.withRmiServiceUrl(APP_HOST, JMX_PORT).withUser(login).withPassword(pwd)); | ||
} | ||
|
||
private static void connectionTest( | ||
Function<TestAppContainer, TestAppContainer> customizeApp, | ||
Function<JmxScraperContainer, JmxScraperContainer> customizeScraper) { | ||
try (TestAppContainer app = customizeApp.apply(appContainer())) { | ||
app.start(); | ||
try (JmxScraperContainer scraper = customizeScraper.apply(scraperContainer())) { | ||
scraper.start(); | ||
waitTerminated(scraper); | ||
checkConnectionLogs(scraper, /* expectedOk= */ true); | ||
} | ||
} | ||
} | ||
|
||
private static void checkConnectionLogs(JmxScraperContainer scraper, boolean expectedOk) { | ||
|
||
String[] logLines = scraper.getLogs().split("\n"); | ||
String lastLine = logLines[logLines.length - 1]; | ||
|
||
if (expectedOk) { | ||
assertThat(lastLine) | ||
.describedAs("should log connection success") | ||
.endsWith("JMX connection test OK"); | ||
} else { | ||
assertThat(lastLine) | ||
.describedAs("should log connection failure") | ||
.endsWith("JMX connection test ERROR"); | ||
} | ||
} | ||
|
||
private static void waitTerminated(GenericContainer<?> container) { | ||
int retries = 10; | ||
while (retries > 0 && container.isRunning()) { | ||
retries--; | ||
try { | ||
Thread.sleep(100); | ||
} catch (InterruptedException e) { | ||
throw new RuntimeException(e); | ||
} | ||
} | ||
assertThat(retries) | ||
.describedAs("container should stop when testing connection") | ||
.isNotEqualTo(0); | ||
} | ||
|
||
private static JmxScraperContainer scraperContainer() { | ||
return new JmxScraperContainer(DUMMY_OTLP_ENDPOINT, SCRAPER_BASE_IMAGE) | ||
.withLogConsumer(new Slf4jLogConsumer(jmxScraperLogger)) | ||
.withNetwork(network) | ||
// mandatory to have a target system even if we don't collect metrics | ||
.withTargetSystem("jvm") | ||
// we are only testing JMX connection here | ||
.withTestJmx(); | ||
} | ||
|
||
private static TestAppContainer appContainer() { | ||
return new TestAppContainer() | ||
.withLogConsumer(new Slf4jLogConsumer(appLogger)) | ||
.withNetwork(network) | ||
.withNetworkAliases(APP_HOST); | ||
} | ||
} |
95 changes: 0 additions & 95 deletions
95
...src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java
This file was deleted.
Oops, something went wrong.
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,28 +61,6 @@ public TestAppContainer withUserAuth(String login, String pwd) { | |
return this; | ||
} | ||
|
||
/** | ||
* Configures app container for host-to-container access, port will be used as-is from host to | ||
* work-around JMX in docker. This is optional on Linux as there is a network route and the | ||
* container is accessible, but not on Mac where the container runs in an isolated VM. | ||
* | ||
* @param port port to use, must be available on host. | ||
* @return this | ||
*/ | ||
@CanIgnoreReturnValue | ||
public TestAppContainer withHostAccessFixedJmxPort(int port) { | ||
// To get host->container JMX connection working docker must expose JMX/RMI port under the same | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [for reviewer] this is no longer necessary as we only rely on container-to-container communication and we don't have to use docker NAT to access the JMX endpoint. |
||
// port number. Because of this testcontainers' standard exposed port randomization approach | ||
// can't be used. | ||
// Explanation: | ||
// https://forums.docker.com/t/exposing-mapped-jmx-ports-from-multiple-containers/5287/6 | ||
properties.put("com.sun.management.jmxremote.port", Integer.toString(port)); | ||
properties.put("com.sun.management.jmxremote.rmi.port", Integer.toString(port)); | ||
properties.put("java.rmi.server.hostname", getHost()); | ||
addFixedExposedPort(port, port); | ||
return this; | ||
} | ||
|
||
@Override | ||
public void start() { | ||
// TODO: add support for ssl | ||
|
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[for reviewer] we have to parse and check scraper logs in order to know if the connection is OK or not, this is because we can't access the process exit value when the program completes.