Skip to content

Commit 11a2e1a

Browse files
jefchienrobsunday
jefchien
authored and
robsunday
committed
Fix Tomcat metric definitions to aggregate multiple MBeans. (open-telemetry#1366)
1 parent 9fa44be commit 11a2e1a

File tree

12 files changed

+138
-22
lines changed

12 files changed

+138
-22
lines changed

‎jmx-metrics/README.md

+4
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ Kafka metric-gathering scripts determined by the comma-separated list in `otel.j
3535
it will then run the scripts on the desired interval length of `otel.jmx.interval.milliseconds` and
3636
export the resulting metrics.
3737

38+
Some metrics (e.g. `tomcat.sessions`) are configured to query multiple MBeans. By default, only the value in the first MBean
39+
is recorded for the metric and all other values are dropped. To aggregate the MBean values together, set the
40+
`otel.jmx.aggregate.across.mbeans` property to `true`.
41+
3842
For custom metrics and unsupported targets, you can provide your own MBean querying scripts to produce
3943
OpenTelemetry instruments:
4044

‎jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/GroovyRunner.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ public class GroovyRunner {
7070
Binding binding = new Binding();
7171
binding.setVariable("log", logger);
7272

73-
OtelHelper otelHelper = new OtelHelper(jmxClient, this.groovyMetricEnvironment);
73+
OtelHelper otelHelper =
74+
new OtelHelper(jmxClient, this.groovyMetricEnvironment, config.aggregateAcrossMBeans);
7475
binding.setVariable("otel", otelHelper);
7576

7677
for (final Script script : scripts) {

‎jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/InstrumentHelper.groovy

+48-3
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ class InstrumentHelper {
4545
private final Map<String, Closure> labelFuncs
4646
private final Closure instrument
4747
private final GroovyMetricEnvironment metricEnvironment
48+
private final boolean aggregateAcrossMBeans
4849

4950
/**
5051
* An InstrumentHelper provides the ability to easily create and update {@link io.opentelemetry.api.metrics.Instrument}
@@ -63,8 +64,9 @@ class InstrumentHelper {
6364
* (e.g. new OtelHelper().&doubleValueRecorder)
6465
* @param metricenvironment - The {@link GroovyMetricEnvironment} used to register callbacks onto the SDK meter for
6566
* batch callbacks used to handle {@link CompositeData}
67+
* @param aggregateAcrossMBeans - Whether to aggregate multiple MBeans together before recording.
6668
*/
67-
InstrumentHelper(MBeanHelper mBeanHelper, String instrumentName, String description, String unit, Map<String, Closure<?>> labelFuncs, Map<String, Map<String, Closure<?>>> MBeanAttributes, Closure<?> instrument, GroovyMetricEnvironment metricEnvironment) {
69+
InstrumentHelper(MBeanHelper mBeanHelper, String instrumentName, String description, String unit, Map<String, Closure<?>> labelFuncs, Map<String, Map<String, Closure<?>>> MBeanAttributes, Closure<?> instrument, GroovyMetricEnvironment metricEnvironment, boolean aggregateAcrossMBeans) {
6870
this.mBeanHelper = mBeanHelper
6971
this.instrumentName = instrumentName
7072
this.description = description
@@ -73,6 +75,7 @@ class InstrumentHelper {
7375
this.mBeanAttributes = MBeanAttributes
7476
this.instrument = instrument
7577
this.metricEnvironment = metricEnvironment
78+
this.aggregateAcrossMBeans = aggregateAcrossMBeans
7679
}
7780

7881
void update() {
@@ -181,19 +184,39 @@ class InstrumentHelper {
181184
return labels
182185
}
183186

187+
private static String getAggregationKey(String instrumentName, Map<String, String> labels) {
188+
def labelsKey = labels.sort().collect { key, value -> "${key}:${value}" }.join(";")
189+
return "${instrumentName}/${labelsKey}"
190+
}
191+
184192
// Create a closure for simple attributes that will retrieve mbean information on
185193
// callback to ensure that metrics are collected on request
186194
private Closure prepareUpdateClosure(List<GroovyMBean> mbeans, attributes) {
187195
return { result ->
196+
def aggregations = [:] as Map<String, Aggregation>
197+
boolean requireAggregation = aggregateAcrossMBeans && mbeans.size() > 1 && instrumentIsValue(instrument)
188198
[mbeans, attributes].combinations().each { pair ->
189199
def (mbean, attribute) = pair
190200
def value = MBeanHelper.getBeanAttribute(mbean, attribute)
191201
if (value != null) {
192202
def labels = getLabels(mbean, labelFuncs, mBeanAttributes[attribute])
193-
logger.fine("Recording ${instrumentName} - ${instrument.method} w/ ${value} - ${labels}")
194-
recordDataPoint(instrument, result, value, GroovyMetricEnvironment.mapToAttributes(labels))
203+
if (requireAggregation) {
204+
def key = getAggregationKey(instrumentName, labels)
205+
if (aggregations[key] == null) {
206+
aggregations[key] = new Aggregation(labels)
207+
}
208+
logger.fine("Aggregating ${mbean.name()} ${instrumentName} - ${instrument.method} w/ ${value} - ${labels}")
209+
aggregations[key].add(value)
210+
} else {
211+
logger.fine("Recording ${mbean.name()} ${instrumentName} - ${instrument.method} w/ ${value} - ${labels}")
212+
recordDataPoint(instrument, result, value, GroovyMetricEnvironment.mapToAttributes(labels))
213+
}
195214
}
196215
}
216+
aggregations.each { entry ->
217+
logger.fine("Recording ${instrumentName} - ${instrument.method} - w/ ${entry.value.value} - ${entry.value.labels}")
218+
recordDataPoint(instrument, result, entry.value.value, GroovyMetricEnvironment.mapToAttributes(entry.value.labels))
219+
}
197220
}
198221
}
199222

@@ -252,6 +275,14 @@ class InstrumentHelper {
252275
].contains(inst.method)
253276
}
254277

278+
@PackageScope
279+
static boolean instrumentIsValue(inst) {
280+
return [
281+
"doubleValueCallback",
282+
"longValueCallback"
283+
].contains(inst.method)
284+
}
285+
255286
@PackageScope
256287
static boolean instrumentIsCounter(inst) {
257288
return [
@@ -261,4 +292,18 @@ class InstrumentHelper {
261292
"longUpDownCounter"
262293
].contains(inst.method)
263294
}
295+
296+
static class Aggregation {
297+
private final Map<String, String> labels
298+
private def value
299+
300+
Aggregation(Map<String, String> labels) {
301+
this.labels = labels
302+
this.value = 0.0
303+
}
304+
305+
void add(value) {
306+
this.value += value
307+
}
308+
}
264309
}

‎jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/JmxConfig.java

+5
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ class JmxConfig {
3131
static final String JMX_PASSWORD = PREFIX + "jmx.password";
3232
static final String JMX_REMOTE_PROFILE = PREFIX + "jmx.remote.profile";
3333
static final String JMX_REALM = PREFIX + "jmx.realm";
34+
static final String JMX_AGGREGATE_ACROSS_MBEANS = PREFIX + "jmx.aggregate.across.mbeans";
3435

3536
// These properties need to be copied into System Properties if provided via the property
3637
// file so that they are available to the JMX Connection builder
@@ -77,6 +78,8 @@ class JmxConfig {
7778
final boolean registrySsl;
7879
final Properties properties;
7980

81+
final boolean aggregateAcrossMBeans;
82+
8083
JmxConfig(final Properties props) {
8184
properties = new Properties();
8285
// putAll() instead of using constructor defaults
@@ -112,6 +115,8 @@ class JmxConfig {
112115
realm = properties.getProperty(JMX_REALM);
113116

114117
registrySsl = Boolean.valueOf(properties.getProperty(REGISTRY_SSL));
118+
aggregateAcrossMBeans =
119+
Boolean.parseBoolean(properties.getProperty(JMX_AGGREGATE_ACROSS_MBEANS));
115120

116121
// For the list of System Properties, if they have been set in the properties file
117122
// they need to be set in Java System Properties.

‎jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/OtelHelper.groovy

+4-2
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,12 @@ class OtelHelper {
2222

2323
private final JmxClient jmxClient
2424
private final GroovyMetricEnvironment groovyMetricEnvironment
25+
private final boolean aggregateAcrossMBeans
2526

26-
OtelHelper(JmxClient jmxClient, GroovyMetricEnvironment groovyMetricEnvironment) {
27+
OtelHelper(JmxClient jmxClient, GroovyMetricEnvironment groovyMetricEnvironment, boolean aggregateAcrossMBeans) {
2728
this.jmxClient = jmxClient
2829
this.groovyMetricEnvironment = groovyMetricEnvironment
30+
this.aggregateAcrossMBeans = aggregateAcrossMBeans
2931
}
3032

3133
/**
@@ -99,7 +101,7 @@ class OtelHelper {
99101
* attribute value(s). The parameters map to the InstrumentHelper constructor.
100102
*/
101103
InstrumentHelper instrument(MBeanHelper mBeanHelper, String instrumentName, String description, String unit, Map<String, Closure> labelFuncs, Map<String, Map<String, Closure>> attributes, Closure otelInstrument) {
102-
def instrumentHelper = new InstrumentHelper(mBeanHelper, instrumentName, description, unit, labelFuncs, attributes, otelInstrument, groovyMetricEnvironment)
104+
def instrumentHelper = new InstrumentHelper(mBeanHelper, instrumentName, description, unit, labelFuncs, attributes, otelInstrument, groovyMetricEnvironment, aggregateAcrossMBeans)
103105
instrumentHelper.update()
104106
return instrumentHelper
105107
}

‎jmx-metrics/src/main/resources/target-systems/tomcat.groovy

+8-8
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@
1515
*/
1616

1717

18-
def beantomcatmanager = otel.mbean("Catalina:type=Manager,host=localhost,context=*")
19-
otel.instrument(beantomcatmanager, "tomcat.sessions", "The number of active sessions.", "sessions", "activeSessions", otel.&doubleValueCallback)
18+
def beantomcatmanager = otel.mbeans("Catalina:type=Manager,host=localhost,context=*")
19+
otel.instrument(beantomcatmanager, "tomcat.sessions", "The number of active sessions.", "sessions", "activeSessions", otel.&longValueCallback)
2020

21-
def beantomcatrequestProcessor = otel.mbean("Catalina:type=GlobalRequestProcessor,name=*")
21+
def beantomcatrequestProcessor = otel.mbeans("Catalina:type=GlobalRequestProcessor,name=*")
2222
otel.instrument(beantomcatrequestProcessor, "tomcat.errors", "The number of errors encountered.", "errors",
2323
["proto_handler" : { mbean -> mbean.name().getKeyProperty("name") }],
2424
"errorCount", otel.&longCounterCallback)
@@ -37,15 +37,15 @@ otel.instrument(beantomcatrequestProcessor, "tomcat.traffic",
3737
["bytesReceived":["direction" : {"received"}], "bytesSent": ["direction" : {"sent"}]],
3838
otel.&longCounterCallback)
3939

40-
def beantomcatconnectors = otel.mbean("Catalina:type=ThreadPool,name=*")
40+
def beantomcatconnectors = otel.mbeans("Catalina:type=ThreadPool,name=*")
4141
otel.instrument(beantomcatconnectors, "tomcat.threads", "The number of threads", "threads",
4242
["proto_handler" : { mbean -> mbean.name().getKeyProperty("name") }],
4343
["currentThreadCount":["state":{"idle"}],"currentThreadsBusy":["state":{"busy"}]], otel.&longValueCallback)
4444

45-
def beantomcatnewmanager = otel.mbean("Tomcat:type=Manager,host=localhost,context=*")
46-
otel.instrument(beantomcatnewmanager, "tomcat.sessions", "The number of active sessions.", "sessions", "activeSessions", otel.&doubleValueCallback)
45+
def beantomcatnewmanager = otel.mbeans("Tomcat:type=Manager,host=localhost,context=*")
46+
otel.instrument(beantomcatnewmanager, "tomcat.sessions", "The number of active sessions.", "sessions", "activeSessions", otel.&longValueCallback)
4747

48-
def beantomcatnewrequestProcessor = otel.mbean("Tomcat:type=GlobalRequestProcessor,name=*")
48+
def beantomcatnewrequestProcessor = otel.mbeans("Tomcat:type=GlobalRequestProcessor,name=*")
4949
otel.instrument(beantomcatnewrequestProcessor, "tomcat.errors", "The number of errors encountered.", "errors",
5050
["proto_handler" : { mbean -> mbean.name().getKeyProperty("name") }],
5151
"errorCount", otel.&longCounterCallback)
@@ -64,7 +64,7 @@ otel.instrument(beantomcatnewrequestProcessor, "tomcat.traffic",
6464
["bytesReceived":["direction" : {"received"}], "bytesSent": ["direction" : {"sent"}]],
6565
otel.&longCounterCallback)
6666

67-
def beantomcatnewconnectors = otel.mbean("Tomcat:type=ThreadPool,name=*")
67+
def beantomcatnewconnectors = otel.mbeans("Tomcat:type=ThreadPool,name=*")
6868
otel.instrument(beantomcatnewconnectors, "tomcat.threads", "The number of threads", "threads",
6969
["proto_handler" : { mbean -> mbean.name().getKeyProperty("name") }],
7070
["currentThreadCount":["state":{"idle"}],"currentThreadsBusy":["state":{"busy"}]], otel.&longValueCallback)

‎jmx-metrics/src/test/java/io/opentelemetry/contrib/jmxmetrics/InstrumenterHelperTest.java

+60-5
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ void setupOtel() {
9696
metricReader = InMemoryMetricReader.create();
9797
meterProvider = SdkMeterProvider.builder().registerMetricReader(metricReader).build();
9898
metricEnvironment = new GroovyMetricEnvironment(meterProvider, "otel.test");
99-
otel = new OtelHelper(jmxClient, metricEnvironment);
99+
otel = new OtelHelper(jmxClient, metricEnvironment, false);
100100
}
101101

102102
@AfterEach
@@ -429,7 +429,36 @@ void doubleValueCallback() throws Exception {
429429
}
430430

431431
@Test
432-
void doubleValueCallbackMultipleMBeans() throws Exception {
432+
void doubleValueCallbackMBeans() throws Exception {
433+
String instrumentMethod = "doubleValueCallback";
434+
String thingName = "multiple:type=" + instrumentMethod + ".Thing";
435+
MBeanHelper mBeanHelper = registerThings(thingName);
436+
437+
String instrumentName = "multiple." + instrumentMethod + ".gauge";
438+
String description = "multiple double gauge description";
439+
440+
updateWithHelper(
441+
mBeanHelper,
442+
instrumentMethod,
443+
instrumentName,
444+
description,
445+
"Double",
446+
new HashMap<>(),
447+
/* aggregateAcrossMBeans= */ true);
448+
449+
assertThat(metricReader.collectAllMetrics())
450+
.satisfiesExactly(
451+
metric ->
452+
assertThat(metric)
453+
.hasName(instrumentName)
454+
.hasDescription(description)
455+
.hasUnit("1")
456+
.hasDoubleGaugeSatisfying(
457+
gauge -> gauge.hasPointsSatisfying(assertDoublePoint())));
458+
}
459+
460+
@Test
461+
void doubleValueCallbackListMBeans() throws Exception {
433462
String instrumentMethod = "doubleValueCallback";
434463
ArrayList<String> thingNames = new ArrayList<>();
435464
for (int i = 0; i < 4; i++) {
@@ -515,6 +544,12 @@ void longValueCallback() throws Exception {
515544
gauge -> gauge.hasPointsSatisfying(assertLongPoints())));
516545
}
517546

547+
@SuppressWarnings("unchecked")
548+
private Consumer<DoublePointAssert>[] assertDoublePoint() {
549+
return Stream.<Consumer<DoublePointAssert>>of(point -> point.hasValue(123.456 * 4))
550+
.toArray(Consumer[]::new);
551+
}
552+
518553
@SuppressWarnings("unchecked")
519554
private Consumer<DoublePointAssert>[] assertDoublePoints() {
520555
return Stream.<Consumer<DoublePointAssert>>of(
@@ -679,11 +714,29 @@ void updateWithHelper(
679714
String instrumentName,
680715
String description,
681716
String attribute) {
682-
Closure<?> instrument = (Closure<?>) Eval.me("otel", otel, "otel.&" + instrumentMethod);
683717
Map<String, Closure<?>> labelFuncs = new HashMap<>();
684718
labelFuncs.put("labelOne", (Closure<?>) Eval.me("{ unused -> 'labelOneValue' }"));
685719
labelFuncs.put(
686720
"labelTwo", (Closure<?>) Eval.me("{ mbean -> mbean.name().getKeyProperty('thing') }"));
721+
updateWithHelper(
722+
mBeanHelper,
723+
instrumentMethod,
724+
instrumentName,
725+
description,
726+
attribute,
727+
labelFuncs,
728+
/* aggregateAcrossMBeans= */ false);
729+
}
730+
731+
void updateWithHelper(
732+
MBeanHelper mBeanHelper,
733+
String instrumentMethod,
734+
String instrumentName,
735+
String description,
736+
String attribute,
737+
Map<String, Closure<?>> labelFuncs,
738+
boolean aggregateAcrossMBeans) {
739+
Closure<?> instrument = (Closure<?>) Eval.me("otel", otel, "otel.&" + instrumentMethod);
687740
InstrumentHelper instrumentHelper =
688741
new InstrumentHelper(
689742
mBeanHelper,
@@ -693,7 +746,8 @@ void updateWithHelper(
693746
labelFuncs,
694747
Collections.singletonMap(attribute, null),
695748
instrument,
696-
metricEnvironment);
749+
metricEnvironment,
750+
aggregateAcrossMBeans);
697751
instrumentHelper.update();
698752
}
699753

@@ -714,7 +768,8 @@ void updateWithHelperMultiAttribute(
714768
labelFuncs,
715769
attributes,
716770
instrument,
717-
metricEnvironment);
771+
metricEnvironment,
772+
false);
718773
instrumentHelper.update();
719774
}
720775

‎jmx-metrics/src/test/java/io/opentelemetry/contrib/jmxmetrics/JmxConfigTest.java

+3
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ void defaultValues() {
5252
assertThat(config.remoteProfile).isNull();
5353
assertThat(config.realm).isNull();
5454
assertThat(config.properties.getProperty("otel.metric.export.interval")).isEqualTo("10000");
55+
assertThat(config.aggregateAcrossMBeans).isFalse();
5556
}
5657

5758
@Test
@@ -87,6 +88,7 @@ void specifiedValues() {
8788
assertThat(config.password).isEqualTo("myPassword");
8889
assertThat(config.remoteProfile).isEqualTo("myRemoteProfile");
8990
assertThat(config.realm).isEqualTo("myRealm");
91+
assertThat(config.aggregateAcrossMBeans).isFalse();
9092
}
9193

9294
@Test
@@ -109,6 +111,7 @@ void propertiesFile() {
109111
assertThat(config.password).isEqualTo("myPassw\\ord");
110112
assertThat(config.remoteProfile).isEqualTo("SASL/DIGEST-MD5");
111113
assertThat(config.realm).isEqualTo("myRealm");
114+
assertThat(config.aggregateAcrossMBeans).isTrue();
112115

113116
// These properties are set from the config file loading into JmxConfig
114117
assertThat(System.getProperty("javax.net.ssl.keyStore")).isEqualTo("/my/key/store");

‎jmx-metrics/src/test/java/io/opentelemetry/contrib/jmxmetrics/OtelHelperAsynchronousMetricTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class OtelHelperAsynchronousMetricTest {
2929
void setUp() {
3030
metricReader = InMemoryMetricReader.create();
3131
meterProvider = SdkMeterProvider.builder().registerMetricReader(metricReader).build();
32-
otel = new OtelHelper(null, new GroovyMetricEnvironment(meterProvider, "otel.test"));
32+
otel = new OtelHelper(null, new GroovyMetricEnvironment(meterProvider, "otel.test"), false);
3333
}
3434

3535
@Test

‎jmx-metrics/src/test/java/io/opentelemetry/contrib/jmxmetrics/OtelHelperJmxTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ private static JMXServiceURL setupServer(Map<String, String> env) throws Excepti
132132
}
133133

134134
private static OtelHelper setupHelper(JmxConfig config) throws Exception {
135-
return new OtelHelper(new JmxClient(config), new GroovyMetricEnvironment(config));
135+
return new OtelHelper(new JmxClient(config), new GroovyMetricEnvironment(config), false);
136136
}
137137

138138
private static void verifyClient(Properties props) throws Exception {

‎jmx-metrics/src/test/java/io/opentelemetry/contrib/jmxmetrics/OtelHelperSynchronousMetricTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class OtelHelperSynchronousMetricTest {
3333
void setUp() {
3434
metricReader = InMemoryMetricReader.create();
3535
meterProvider = SdkMeterProvider.builder().registerMetricReader(metricReader).build();
36-
otel = new OtelHelper(null, new GroovyMetricEnvironment(meterProvider, "otel.test"));
36+
otel = new OtelHelper(null, new GroovyMetricEnvironment(meterProvider, "otel.test"), false);
3737
}
3838

3939
@Test

‎jmx-metrics/src/test/resources/all.properties

+1
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,4 @@ javax.net.ssl.keyStoreType=JKS
1919
javax.net.ssl.trustStore=/my/trust/store
2020
javax.net.ssl.trustStorePassword=def456
2121
javax.net.ssl.trustStoreType=JKS
22+
otel.jmx.aggregate.across.mbeans=true

0 commit comments

Comments
 (0)