Extension of JMX metrics testing framework with data point value matching#16274
Extension of JMX metrics testing framework with data point value matching#16274laurit merged 9 commits intoopen-telemetry:mainfrom
Conversation
| DEFAULT_EXCHANGE_DELAY_MS | ||
| + ThreadLocalRandom.current().nextLong(MAXIMUM_EXCHANGE_DELAY_DELTA)); |
There was a problem hiding this comment.
could also use
ThreadLocalRandom.current().nextLong(min, max)
| .hasDescription( | ||
| "Indicates the number of routes started successfully since context start-up or the last reset operation.") | ||
| .hasUnit("{route}") | ||
| .hasDataPointsWithIntValues(value -> value.isEqualTo(numberOfTestRoutes)) |
There was a problem hiding this comment.
It is a bit strange that jmx metrics have their own custom assertions. I guess when the jmx tests were added it might have been easier to do it this way but now you could use https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/smoke-tests/src/main/java/io/opentelemetry/smoketest/SmokeTestInstrumentationExtension.java to pull data from the fake backend and assert it the same way as other test do. I think ideally we should try to move towards using https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/MetricAssert.java here too. Keeping the api used in jmx assertions the same as in sdk assertions could make moving back to sdk assertions easier.
Anyway this was just something to consider for future work, not relevant for this pr.
There was a problem hiding this comment.
I agree, replacing those assertions with standard SDK test assertions or making them at least rely on the SDK assertions is something to consider as a future improvement, this was a shortcut taken when we migrated this code from the contrib repository.
Added optional mechanism for validating data point value in JMX metrics integration tests.