Skip to content

Conversation

@SylvainJuge
Copy link
Contributor

@SylvainJuge SylvainJuge commented Jun 23, 2025

Fixes #14070

After further investigation, aggregation of values over multiple MBean instances already works as expected, this PR adds proper test and documentation.

This however confirms that using this aggregation with gauge metrics can produce unpredictable results when more than one MBean instance as the "last value" aggregation is being used. Tests and documentation have been updated to reflect this.

@SylvainJuge SylvainJuge self-assigned this Jun 23, 2025
@SylvainJuge SylvainJuge changed the title test jmx metric implicit aggregation document & test jmx metric implicit aggregation Jun 24, 2025
@SylvainJuge SylvainJuge marked this pull request as ready for review June 24, 2025 08:09
@SylvainJuge SylvainJuge requested a review from a team as a code owner June 24, 2025 08:09
@trask
Copy link
Member

trask commented Jun 25, 2025

cc @PeterF778 @robsunday

@PeterF778
Copy link
Contributor

Just thinking about aggregating gauge metrics. Would it make sense to average the values? This would solve the issue with non-deterministic aggregation of multiple values, and it would work as expected when only one value is subject to aggregation.

@SylvainJuge
Copy link
Contributor Author

Just thinking about aggregating gauge metrics. Would it make sense to average the values? This would solve the issue with non-deterministic aggregation of multiple values, and it would work as expected when only one value is subject to aggregation.

While this would bring some determinism, I think it would still provide unexpected results depending on which metric is being captured:

  • for a metric that measures an average: the result is an average of an average
  • for a metric that measures a maximum: the result would be an average of two maximums

If we apply this to the context of web/application containers, one hypothetical web server could be to have two connectors, each monitored with its own MBean instance: for example one configured for http traffic and one for https. In practice most of the traffic would happen only on one of those configured connectors, which means the metrics from the other one would be zero. In such case the metrics reported would be half of the actual and expected value.

However, doing so could help to provide a "consistently less wrong value", as in example we could have either the correct expected value or a zero.

@laurit laurit added this to the v2.18.0 milestone Jul 11, 2025
SylvainJuge and others added 2 commits July 11, 2025 13:21
…etry/instrumentation/jmx/engine/MetricAggregationTest.java

Co-authored-by: Lauri Tulmin <[email protected]>
@otelbot-java-instrumentation
Copy link
Contributor

🔧 The result from spotlessApply was committed to the PR branch.

@trask trask merged commit 298319f into open-telemetry:main Jul 15, 2025
89 checks passed
@SylvainJuge SylvainJuge deleted the jmx-metrics-advice branch July 15, 2025 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ability to drop JMX MBean attributes and aggregate properly

5 participants