Skip to content

Commit 1615862

Browse files
authored
jmx scraper align custom yaml config (#1678)
1 parent 4cd6a0a commit 1615862

File tree

4 files changed

+83
-32
lines changed

4 files changed

+83
-32
lines changed

jmx-scraper/README.md

+8-8
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ Minimal configuration required
1616

1717
- `otel.jmx.service.url` for example `service:jmx:rmi:///jndi/rmi://server:9999/jmxrmi` for `server`
1818
host on port `9999` with RMI JMX connector.
19-
- `otel.jmx.target.system` or `otel.jmx.custom.scraping.config`
19+
- `otel.jmx.target.system` or `otel.jmx.config`
2020

2121
Configuration can be provided through:
2222

@@ -36,13 +36,13 @@ For example the `otel.jmx.service.url` option can be set with the `OTEL_JMX_SERV
3636

3737
## Configuration reference
3838

39-
| config option | description |
40-
|-----------------------------------|----------------------------------------------------------------------------------------------|
41-
| `otel.jmx.service.url` | mandatory JMX URL to connect to the remote JVM |
42-
| `otel.jmx.target.system` | comma-separated list of systems to monitor, mandatory unless a custom configuration is used |
43-
| `otel.jmx.custom.scraping.config` | path to a custom YAML metrics definition, mandatory when `otel.jmx.target.system` is not set |
44-
| `otel.jmx.username` | user name for JMX connection, mandatory when JMX authentication is enabled on target JVM |
45-
| `otel.jmx.password` | password for JMX connection, mandatory when JMX authentication is enabled on target JVM |
39+
| config option | description |
40+
|--------------------------|---------------------------------------------------------------------------------------------------------------------|
41+
| `otel.jmx.service.url` | mandatory JMX URL to connect to the remote JVM |
42+
| `otel.jmx.target.system` | comma-separated list of systems to monitor, mandatory unless a custom configuration is used |
43+
| `otel.jmx.config` | comma-separated list of paths to custom YAML metrics definition, mandatory when `otel.jmx.target.system` is not set |
44+
| `otel.jmx.username` | user name for JMX connection, mandatory when JMX authentication is enabled on target JVM |
45+
| `otel.jmx.password` | password for JMX connection, mandatory when JMX authentication is enabled on target JVM |
4646

4747
Supported values for `otel.jmx.target.system`:
4848

jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/JmxScraperConfig.java

+34-12
Original file line numberDiff line numberDiff line change
@@ -8,25 +8,34 @@
88
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
99
import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException;
1010
import java.time.Duration;
11+
import java.util.ArrayList;
1112
import java.util.Arrays;
1213
import java.util.Collections;
1314
import java.util.HashSet;
1415
import java.util.List;
1516
import java.util.Set;
17+
import java.util.logging.Logger;
1618
import javax.annotation.Nullable;
1719

1820
/** This class keeps application settings */
1921
public class JmxScraperConfig {
2022

23+
private static final Logger logger = Logger.getLogger(JmxScraperConfig.class.getName());
24+
2125
// metric sdk configuration
2226
static final String METRIC_EXPORT_INTERVAL = "otel.metric.export.interval";
2327

2428
// not documented on purpose as using the SDK 'otel.metric.export.interval' is preferred
2529
static final String JMX_INTERVAL_LEGACY = "otel.jmx.interval.milliseconds";
2630

2731
static final String JMX_SERVICE_URL = "otel.jmx.service.url";
28-
// TODO: align with instrumentation 'otel.jmx.config' + support list of values
29-
static final String JMX_CUSTOM_CONFIG = "otel.jmx.custom.scraping.config";
32+
33+
// the same as config option defined in instrumentation
34+
static final String JMX_CONFIG = "otel.jmx.config";
35+
36+
// not documented on purpose as it's deprecated
37+
static final String JMX_CONFIG_LEGACY = "otel.jmx.custom.scraping.config";
38+
3039
static final String JMX_TARGET_SYSTEM = "otel.jmx.target.system";
3140

3241
static final String JMX_USERNAME = "otel.jmx.username";
@@ -55,7 +64,7 @@ public class JmxScraperConfig {
5564

5665
private String serviceUrl = "";
5766

58-
@Nullable private String customJmxScrapingConfigPath;
67+
private List<String> jmxConfig = Collections.emptyList();
5968

6069
private Set<String> targetSystems = Collections.emptySet();
6170

@@ -76,9 +85,8 @@ public String getServiceUrl() {
7685
return serviceUrl;
7786
}
7887

79-
@Nullable
80-
public String getCustomJmxScrapingConfigPath() {
81-
return customJmxScrapingConfigPath;
88+
public List<String> getJmxConfig() {
89+
return jmxConfig;
8290
}
8391

8492
public Set<String> getTargetSystems() {
@@ -136,21 +144,35 @@ public static JmxScraperConfig fromConfig(ConfigProperties config) {
136144
}
137145
scraperConfig.serviceUrl = serviceUrl;
138146

139-
// TODO: we could support multiple values
140-
String customConfig = config.getString(JMX_CUSTOM_CONFIG, "");
147+
List<String> jmxConfig = config.getList(JMX_CONFIG);
141148
List<String> targetSystem = config.getList(JMX_TARGET_SYSTEM);
142-
if (targetSystem.isEmpty() && customConfig.isEmpty()) {
149+
150+
// providing compatibility with the deprecated 'otel.jmx.custom.scraping.config' config option
151+
String jmxConfigDeprecated = config.getString(JMX_CONFIG_LEGACY);
152+
if (jmxConfigDeprecated != null) {
153+
logger.warning(
154+
JMX_CONFIG_LEGACY
155+
+ " deprecated option is used, replacing with '"
156+
+ JMX_CONFIG
157+
+ "' is recommended");
158+
List<String> list = new ArrayList<>(jmxConfig);
159+
list.add(jmxConfigDeprecated);
160+
jmxConfig = list;
161+
}
162+
163+
if (targetSystem.isEmpty() && jmxConfig.isEmpty()) {
143164
throw new ConfigurationException(
144-
"at least one of '" + JMX_TARGET_SYSTEM + "' or '" + JMX_CUSTOM_CONFIG + "' must be set");
165+
"at least one of '" + JMX_TARGET_SYSTEM + "' or '" + JMX_CONFIG + "' must be set");
145166
}
146167
targetSystem.forEach(
147168
s -> {
148169
if (!AVAILABLE_TARGET_SYSTEMS.contains(s)) {
149170
throw new ConfigurationException("unsupported target system: '" + s + "'");
150171
}
151172
});
152-
scraperConfig.customJmxScrapingConfigPath = customConfig;
153-
scraperConfig.targetSystems = new HashSet<>(targetSystem);
173+
174+
scraperConfig.jmxConfig = Collections.unmodifiableList(jmxConfig);
175+
scraperConfig.targetSystems = Collections.unmodifiableSet(new HashSet<>(targetSystem));
154176

155177
scraperConfig.username = config.getString("otel.jmx.username");
156178
scraperConfig.password = config.getString("otel.jmx.password");

jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/config/JmxScraperConfigTest.java

+40-11
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55

66
package io.opentelemetry.contrib.jmxscraper.config;
77

8-
import static io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfig.JMX_CUSTOM_CONFIG;
8+
import static io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfig.JMX_CONFIG;
9+
import static io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfig.JMX_CONFIG_LEGACY;
910
import static io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfig.JMX_PASSWORD;
1011
import static io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfig.JMX_REALM;
1112
import static io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfig.JMX_REGISTRY_SSL;
@@ -22,6 +23,8 @@
2223
import java.util.Properties;
2324
import org.junit.jupiter.api.BeforeAll;
2425
import org.junit.jupiter.api.Test;
26+
import org.junit.jupiter.params.ParameterizedTest;
27+
import org.junit.jupiter.params.provider.ValueSource;
2528

2629
class JmxScraperConfigTest {
2730
private static Properties validProperties;
@@ -31,7 +34,7 @@ static void setUp() {
3134
validProperties = new Properties();
3235
validProperties.setProperty(
3336
JMX_SERVICE_URL, "jservice:jmx:rmi:///jndi/rmi://localhost:9010/jmxrmi");
34-
validProperties.setProperty(JMX_CUSTOM_CONFIG, "/path/to/config.yaml");
37+
validProperties.setProperty(JMX_CONFIG, "/path/to/config.yaml");
3538
validProperties.setProperty(JMX_TARGET_SYSTEM, "tomcat, activemq");
3639
validProperties.setProperty(JMX_REGISTRY_SSL, "true");
3740
validProperties.setProperty(JMX_USERNAME, "some-user");
@@ -50,7 +53,7 @@ void shouldPassValidation() {
5053
// Then
5154
assertThat(config.getServiceUrl())
5255
.isEqualTo("jservice:jmx:rmi:///jndi/rmi://localhost:9010/jmxrmi");
53-
assertThat(config.getCustomJmxScrapingConfigPath()).isEqualTo("/path/to/config.yaml");
56+
assertThat(config.getJmxConfig()).containsExactly("/path/to/config.yaml");
5457
assertThat(config.getTargetSystems()).containsExactlyInAnyOrder("tomcat", "activemq");
5558
assertThat(config.getSamplingInterval()).isEqualTo(Duration.ofSeconds(10));
5659
assertThat(config.getUsername()).isEqualTo("some-user");
@@ -59,21 +62,33 @@ void shouldPassValidation() {
5962
assertThat(config.getRealm()).isEqualTo("some-realm");
6063
}
6164

62-
@Test
63-
void shouldCreateMinimalValidConfiguration() {
65+
@ParameterizedTest(name = "custom yaml = {arguments}")
66+
@ValueSource(booleans = {true, false})
67+
public void shouldCreateMinimalValidConfiguration(boolean customYaml) {
6468
// Given
6569
Properties properties = new Properties();
6670
properties.setProperty(JMX_SERVICE_URL, "jservice:jmx:rmi:///jndi/rmi://localhost:9010/jmxrmi");
67-
properties.setProperty(JMX_CUSTOM_CONFIG, "/file.properties");
71+
if (customYaml) {
72+
properties.setProperty(JMX_CONFIG, "/file.yaml");
73+
} else {
74+
properties.setProperty(JMX_TARGET_SYSTEM, "tomcat");
75+
}
6876

6977
// When
7078
JmxScraperConfig config = fromConfig(TestUtil.configProperties(properties));
7179

7280
// Then
7381
assertThat(config.getServiceUrl())
7482
.isEqualTo("jservice:jmx:rmi:///jndi/rmi://localhost:9010/jmxrmi");
75-
assertThat(config.getCustomJmxScrapingConfigPath()).isEqualTo("/file.properties");
76-
assertThat(config.getTargetSystems()).isEmpty();
83+
84+
if (customYaml) {
85+
assertThat(config.getJmxConfig()).containsExactly("/file.yaml");
86+
assertThat(config.getTargetSystems()).isEmpty();
87+
} else {
88+
assertThat(config.getJmxConfig()).isEmpty();
89+
assertThat(config.getTargetSystems()).containsExactly("tomcat");
90+
}
91+
7792
assertThat(config.getSamplingInterval())
7893
.describedAs("default sampling interval must align to default metric export interval")
7994
.isEqualTo(Duration.ofMinutes(1));
@@ -83,6 +98,21 @@ void shouldCreateMinimalValidConfiguration() {
8398
assertThat(config.getRealm()).isNull();
8499
}
85100

101+
@Test
102+
void legacyCustomScrapingConfig() {
103+
// Given
104+
Properties properties = new Properties();
105+
properties.setProperty(JMX_SERVICE_URL, "jservice:jmx:rmi:///jndi/rmi://localhost:9010/jmxrmi");
106+
properties.setProperty(JMX_CONFIG_LEGACY, "/file.yaml");
107+
properties.setProperty(JMX_CONFIG, "/another.yaml");
108+
109+
// When
110+
JmxScraperConfig config = fromConfig(TestUtil.configProperties(properties));
111+
112+
// Then
113+
assertThat(config.getJmxConfig()).containsOnly("/file.yaml", "/another.yaml");
114+
}
115+
86116
@Test
87117
void shouldFailValidation_missingServiceUrl() {
88118
// Given
@@ -99,14 +129,13 @@ void shouldFailValidation_missingServiceUrl() {
99129
void shouldFailValidation_missingConfigPathAndTargetSystem() {
100130
// Given
101131
Properties properties = (Properties) validProperties.clone();
102-
properties.remove(JMX_CUSTOM_CONFIG);
132+
properties.remove(JMX_CONFIG);
103133
properties.remove(JMX_TARGET_SYSTEM);
104134

105135
// When and Then
106136
assertThatThrownBy(() -> fromConfig(TestUtil.configProperties(properties)))
107137
.isInstanceOf(ConfigurationException.class)
108-
.hasMessage(
109-
"at least one of 'otel.jmx.target.system' or 'otel.jmx.custom.scraping.config' must be set");
138+
.hasMessage("at least one of 'otel.jmx.target.system' or 'otel.jmx.config' must be set");
110139
}
111140

112141
@Test

jmx-scraper/src/test/resources/validConfig.properties

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
otel.jmx.service.url=service:jmx:rmi:///jndi/rmi://myhost:12345/jmxrmi
2-
otel.jmx.custom.scraping.config=/my/scraping-config.yaml
2+
otel.jmx.config=/my/scraping-config.yaml
33
otel.jmx.target.system=jvm,cassandra
44
otel.metrics.exporter=otlp
55
otel.metric.export.interval=1000

0 commit comments

Comments
 (0)