Migrate "runtime-metrics" test from groovy to java #8928
Conversation
.../io/opentelemetry/instrumentation/javaagent/runtimemetrics/java17/JmxRuntimeMetricsTest.java
Outdated
Show resolved
Hide resolved
|
Great! It could help to test JVM metrics with a GraalVM native executable. |
…aagent/src/test/java/io/opentelemetry/instrumentation/javaagent/runtimemetrics/java17/JmxRuntimeMetricsTest.java Co-authored-by: Jean Bisutti <[email protected]>
| @@ -76,6 +80,13 @@ public List<MetricData> metrics() { | |||
| return testRunner.getExportedMetrics(); | |||
| } | |||
|
|
|||
| /** Returns a list of metrics, filtered by instrumentation name */ | |||
| public List<MetricData> instrumentationMetrics(String instrumentationName) { | |||
There was a problem hiding this comment.
We don't have analogous methods for any other signal; and it's only used in this class. WDYT about making it private?
| public List<MetricData> instrumentationMetrics(String instrumentationName) { | |
| private List<MetricData> instrumentationMetrics(String instrumentationName) { |
There was a problem hiding this comment.
I intended it to be a public helper, analogous to metrics() but filtered to a specific instrumentation. It's only used internally now, but I could see it being useful elsewhere. If you think it's too much YAGNI then I will make it private.
There was a problem hiding this comment.
Let's make it private -- there's just one use case for that now; and I think all 3 signals should have very similar API, so if we're introducing some new thing here we should do similar thing everywhere.
There was a problem hiding this comment.
Ok I relent. :) I think you have a higher standard for test code than I do.
There was a problem hiding this comment.
Hah, I'm just thinking that YAGNI (probably) when it comes to making that method public
Related to #7195