Skip to content

Commit c23fc0c

Browse files
SylvainJugeSylvainJuge
committed
align custom jmx option with instrumentation
1 parent 84dd819 commit c23fc0c

File tree

2 files changed

+71
-21
lines changed

2 files changed

+71
-21
lines changed

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

Lines changed: 30 additions & 11 deletions
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,20 +144,31 @@ 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(JMX_CONFIG_LEGACY + " deprecated option is used, replacing with '" + JMX_CONFIG
154+
+ "' is recommended");
155+
List<String> list = new ArrayList<>(jmxConfig);
156+
list.add(jmxConfigDeprecated);
157+
jmxConfig = list;
158+
}
159+
160+
if (targetSystem.isEmpty() && jmxConfig.isEmpty()) {
143161
throw new ConfigurationException(
144-
"at least one of '" + JMX_TARGET_SYSTEM + "' or '" + JMX_CUSTOM_CONFIG + "' must be set");
162+
"at least one of '" + JMX_TARGET_SYSTEM + "' or '" + JMX_CONFIG + "' must be set");
145163
}
146164
targetSystem.forEach(
147165
s -> {
148166
if (!AVAILABLE_TARGET_SYSTEMS.contains(s)) {
149167
throw new ConfigurationException("unsupported target system: '" + s + "'");
150168
}
151169
});
152-
scraperConfig.customJmxScrapingConfigPath = customConfig;
170+
171+
scraperConfig.jmxConfig = jmxConfig;
153172
scraperConfig.targetSystems = new HashSet<>(targetSystem);
154173

155174
scraperConfig.username = config.getString("otel.jmx.username");

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

Lines changed: 41 additions & 10 deletions
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,34 @@ 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+
92+
7793
assertThat(config.getSamplingInterval())
7894
.describedAs("default sampling interval must align to default metric export interval")
7995
.isEqualTo(Duration.ofMinutes(1));
@@ -83,6 +99,21 @@ void shouldCreateMinimalValidConfiguration() {
8399
assertThat(config.getRealm()).isNull();
84100
}
85101

102+
@Test
103+
void legacyCustomScrapingConfig() {
104+
// Given
105+
Properties properties = new Properties();
106+
properties.setProperty(JMX_SERVICE_URL, "jservice:jmx:rmi:///jndi/rmi://localhost:9010/jmxrmi");
107+
properties.setProperty(JMX_CONFIG_LEGACY, "/file.yaml");
108+
properties.setProperty(JMX_CONFIG, "/another.yaml");
109+
110+
// When
111+
JmxScraperConfig config = fromConfig(TestUtil.configProperties(properties));
112+
113+
// Then
114+
assertThat(config.getJmxConfig()).containsOnly("/file.yaml", "/another.yaml");
115+
}
116+
86117
@Test
87118
void shouldFailValidation_missingServiceUrl() {
88119
// Given
@@ -99,14 +130,14 @@ void shouldFailValidation_missingServiceUrl() {
99130
void shouldFailValidation_missingConfigPathAndTargetSystem() {
100131
// Given
101132
Properties properties = (Properties) validProperties.clone();
102-
properties.remove(JMX_CUSTOM_CONFIG);
133+
properties.remove(JMX_CONFIG);
103134
properties.remove(JMX_TARGET_SYSTEM);
104135

105136
// When and Then
106137
assertThatThrownBy(() -> fromConfig(TestUtil.configProperties(properties)))
107138
.isInstanceOf(ConfigurationException.class)
108139
.hasMessage(
109-
"at least one of 'otel.jmx.target.system' or 'otel.jmx.custom.scraping.config' must be set");
140+
"at least one of 'otel.jmx.target.system' or 'otel.jmx.config' must be set");
110141
}
111142

112143
@Test

0 commit comments

Comments
 (0)