-
Notifications
You must be signed in to change notification settings - Fork 129
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
Reuse JMX Insights in Scraper + Tomcat support #1485
Changes from 5 commits
5948f57
7f7681b
93bf770
ac67d0f
97038df
67a50a0
edb3d6e
3a36466
70d0bb0
95c0b59
69f0fd1
3362166
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,16 +5,22 @@ | |
|
||
package io.opentelemetry.contrib.jmxscraper; | ||
|
||
import io.opentelemetry.api.GlobalOpenTelemetry; | ||
import io.opentelemetry.contrib.jmxscraper.config.ConfigurationException; | ||
import io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfig; | ||
import io.opentelemetry.instrumentation.jmx.engine.JmxMetricInsight; | ||
import io.opentelemetry.instrumentation.jmx.engine.MetricConfiguration; | ||
import io.opentelemetry.instrumentation.jmx.yaml.RuleParser; | ||
import java.io.DataInputStream; | ||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.nio.file.Files; | ||
import java.nio.file.Paths; | ||
import java.util.Arrays; | ||
import java.util.Collections; | ||
import java.util.List; | ||
import java.util.Properties; | ||
import java.util.concurrent.atomic.AtomicBoolean; | ||
import java.util.logging.Logger; | ||
import javax.management.MBeanServerConnection; | ||
import javax.management.remote.JMXConnector; | ||
|
@@ -23,10 +29,13 @@ public class JmxScraper { | |
private static final Logger logger = Logger.getLogger(JmxScraper.class.getName()); | ||
private static final String CONFIG_ARG = "-config"; | ||
|
||
private static final String OTEL_AUTOCONFIGURE = "otel.java.global-autoconfigure.enabled"; | ||
|
||
private final JmxConnectorBuilder client; | ||
private final JmxMetricInsight service; | ||
private final JmxScraperConfig config; | ||
|
||
// TODO depend on instrumentation 2.9.0 snapshot | ||
// private final JmxMetricInsight service; | ||
private final AtomicBoolean running = new AtomicBoolean(false); | ||
|
||
/** | ||
* Main method to create and run a {@link JmxScraper} instance. | ||
|
@@ -35,15 +44,25 @@ public class JmxScraper { | |
*/ | ||
@SuppressWarnings({"SystemOut", "SystemExitOutsideMain"}) | ||
public static void main(String[] args) { | ||
|
||
// enable SDK auto-configure if not explicitly set by user | ||
// TODO: refactor this to use AutoConfiguredOpenTelemetrySdk | ||
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. 👍 |
||
if (System.getProperty(OTEL_AUTOCONFIGURE) == null) { | ||
System.setProperty(OTEL_AUTOCONFIGURE, "true"); | ||
} | ||
|
||
try { | ||
JmxScraperConfig config = | ||
JmxScraperConfig.fromProperties(parseArgs(Arrays.asList(args)), System.getProperties()); | ||
// propagate effective user-provided configuration to JVM system properties | ||
// this also enables SDK auto-configuration to use those properties | ||
config.propagateSystemProperties(); | ||
// TODO: depend on instrumentation 2.9.0 snapshot | ||
// service = JmxMetricInsight.createService(GlobalOpenTelemetry.get(), | ||
// config.getIntervalMilliseconds()); | ||
JmxScraper jmxScraper = new JmxScraper(JmxConnectorBuilder.createNew(config.getServiceUrl())); | ||
|
||
JmxMetricInsight service = | ||
JmxMetricInsight.createService( | ||
GlobalOpenTelemetry.get(), config.getIntervalMilliseconds()); | ||
JmxScraper jmxScraper = | ||
new JmxScraper(JmxConnectorBuilder.createNew(config.getServiceUrl()), service, config); | ||
jmxScraper.start(); | ||
|
||
} catch (ArgumentsParsingException e) { | ||
|
@@ -109,29 +128,64 @@ private static Properties loadPropertiesFromPath(String path) throws Configurati | |
} | ||
} | ||
|
||
JmxScraper(JmxConnectorBuilder client) { | ||
JmxScraper(JmxConnectorBuilder client, JmxMetricInsight service, JmxScraperConfig config) { | ||
this.client = client; | ||
this.service = service; | ||
this.config = config; | ||
} | ||
|
||
private void start() throws IOException { | ||
Runtime.getRuntime() | ||
.addShutdownHook( | ||
new Thread( | ||
() -> { | ||
logger.info("JMX scraping stopped"); | ||
running.set(false); | ||
})); | ||
|
||
try (JMXConnector connector = client.build()) { | ||
MBeanServerConnection connection = connector.getMBeanServerConnection(); | ||
service.startRemote(getMetricConfig(config), () -> Collections.singletonList(connection)); | ||
|
||
running.set(true); | ||
logger.info("JMX scraping started"); | ||
|
||
while (running.get()) { | ||
try { | ||
Thread.sleep(100); | ||
} catch (InterruptedException e) { | ||
// silently ignored | ||
} | ||
} | ||
} | ||
} | ||
|
||
JMXConnector connector = client.build(); | ||
|
||
@SuppressWarnings("unused") | ||
MBeanServerConnection connection = connector.getMBeanServerConnection(); | ||
|
||
// TODO: depend on instrumentation 2.9.0 snapshot | ||
// MetricConfiguration metricConfig = new MetricConfiguration(); | ||
// TODO create JMX insight config from scraper config | ||
// service.startRemote(metricConfig, () -> Collections.singletonList(connection)); | ||
private static MetricConfiguration getMetricConfig(JmxScraperConfig scraperConfig) { | ||
MetricConfiguration config = new MetricConfiguration(); | ||
for (String system : scraperConfig.getTargetSystems()) { | ||
try { | ||
addRulesForSystem(system, config); | ||
} catch (RuntimeException e) { | ||
logger.warning("unable to load rules for system " + system + ": " + e.getMessage()); | ||
SylvainJuge marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
// TODO : add ability for user to provide custom yaml configurations | ||
|
||
logger.info("JMX scraping started"); | ||
return config; | ||
} | ||
|
||
// TODO: wait a bit to keep the JVM running, this won't be needed once calling jmx insight | ||
try { | ||
Thread.sleep(5000); | ||
} catch (InterruptedException e) { | ||
throw new IllegalStateException(e); | ||
private static void addRulesForSystem(String system, MetricConfiguration conf) { | ||
String yamlResource = system + ".yaml"; | ||
try (InputStream inputStream = | ||
JmxScraper.class.getClassLoader().getResourceAsStream(yamlResource)) { | ||
if (inputStream != null) { | ||
RuleParser parserInstance = RuleParser.get(); | ||
parserInstance.addMetricDefsTo(conf, inputStream, system); | ||
} else { | ||
throw new IllegalStateException("no support for " + system); | ||
SylvainJuge marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} catch (Exception e) { | ||
throw new IllegalStateException("error while loading rules for system " + system, e); | ||
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. Other exception would be better here as well 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. I'll leave this one as-is for now, as we don't have dedicated runtime exceptions, but this could be an improvement later. |
||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,9 +55,9 @@ public class JmxScraperConfig { | |
private String serviceUrl = ""; | ||
private String customJmxScrapingConfigPath = ""; | ||
private Set<String> targetSystems = Collections.emptySet(); | ||
private int intervalMilliseconds; | ||
private String metricsExporterType = ""; | ||
private String otlpExporterEndpoint = ""; | ||
private int intervalMilliseconds; // TODO only used to set 'otel.metric.export.interval' from SDK | ||
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. What this TODO means here? Is there anything aditional to be implemented? 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. Yes, basically this configuration option is only used to configure the metrics polling interval and there is already a dedicated configuration option in the SDK for this, so if someone is used to configure the SDK using this one we should reuse the existing config option (we'll deal with that when adding auto-configuration here). |
||
private String metricsExporterType = ""; // TODO only used to default to 'logging' if not set | ||
private String otlpExporterEndpoint = ""; // TODO not really needed here as handled by SDK | ||
private String username = ""; | ||
private String password = ""; | ||
private String realm = ""; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
--- | ||
|
||
# For Tomcat, the default JMX domain is "Catalina:", however with some deployments like embedded in spring-boot | ||
# we can have the "Tomcat:" domain used, thus we use both MBean names for the metrics. | ||
|
||
rules: | ||
|
||
- beans: | ||
- Catalina:type=Manager,host=localhost,context=* | ||
- Tomcat:type=Manager,host=localhost,context=* | ||
metricAttribute: | ||
# minor divergence from tomcat.groovy to capture metric for all deployed webapps | ||
context: param(context) | ||
mapping: | ||
activeSessions: | ||
metric: sessions | ||
type: gauge | ||
unit: sessions | ||
desc: The number of active sessions | ||
|
||
- beans: | ||
- Catalina:type=GlobalRequestProcessor,name=* | ||
- Tomcat:type=GlobalRequestProcessor,name=* | ||
prefix: tomcat. | ||
metricAttribute: | ||
proto_handler: param(name) | ||
mapping: | ||
errorCount: | ||
metric: errors | ||
type: counter | ||
unit: errors | ||
desc: The number of errors encountered | ||
requestCount: | ||
metric: request_count | ||
type: counter | ||
unit: requests | ||
desc: The total requests | ||
maxTime: | ||
metric: max_time | ||
type: gauge | ||
unit: ms | ||
desc: The total requests | ||
processingTime: | ||
metric: processing_time | ||
type: gauge | ||
unit: ms | ||
desc: The total processing time | ||
bytesSent: | ||
metric: traffic | ||
type: counter | ||
unit: by | ||
desc: The number of bytes transmitted | ||
metricAttribute: | ||
direction: const(sent) | ||
bytesReceived: | ||
metric: traffic | ||
type: counter | ||
unit: by | ||
desc: The number of bytes received | ||
metricAttribute: | ||
direction: const(received) | ||
|
||
- beans: | ||
- Catalina:type=ThreadPool,name=* | ||
- Tomcat:type=ThreadPool,name=* | ||
prefix: tomcat. | ||
metricAttribute: | ||
proto_handler: param(name) | ||
mapping: | ||
currentThreadCount: | ||
metric: threads | ||
type: updowncounter | ||
unit: threads | ||
metricAttribute: | ||
state: const(idle) | ||
currentThreadsBusy: | ||
metric: threads | ||
type: updowncounter | ||
unit: threads | ||
metricAttribute: | ||
state: const(busy) |
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.
I'm merging anyways since
jmx-scraper
artifact isn't published yet so no worries about relying on snapshot repo from maven central