jmx-metrics: register all GroovyMetricEnvironment instruments

[rmfitzpatrick]

Description: Bug fix - The otel.instrument() helpers do not retrieve the previously built instrument instances on subsequent groovy script runs and this leads to logged duplicate instrument recording warnings. Because the repeated script running pattern is not aligned with the metric api, instrument creation should be memoized in the metric environment by descriptor to avoid the unsupported usage.

Existing Issue(s): https://github.com/open-telemetry/opentelemetry-java-contrib/issues/222

Testing: Added instance checks for synchronous instrument tests. I briefly investigated testing w/ captured loggers but a straightforward way to access the internal sdk instrument's loggers wasn't apparent so I'm not sure of a clean way to detect warnings in async storage. Since these* unavailable statements are prone to drift it's not a solid practice to begin with.

[carlosalberto]

Thanks for working on this, I was recently wondering about such messages ;)

[jamesmoessis]

I noticed this problem too, is this going to get merged?

[trask]

I noticed this problem too, is this going to get merged?

ping @rmfitzpatrick

[jack-berg]

I noticed this problem too, is this going to get merged?

Do you have a repo or some logs you can share? As mentioned here, if I understand correctly, this issue shouldn't happen anymore as the SDK behavior changed a while back so that no warning log is produced when multiple instruments are initialized with the same name, description, unit, type, and value type.

[rmfitzpatrick]

@jack-berg when I run the updated unit tests without the registry they fail for identity equivalence. Would this be expected with the current sdk?

OtelHelperSynchronousMetricTest > longUpDownCounter() FAILED
    java.lang.AssertionError:
    Expecting actual:
      io.opentelemetry.sdk.metrics.SdkLongUpDownCounter@4d968142
    and actual:
      io.opentelemetry.sdk.metrics.SdkLongUpDownCounter@4d968142
    to refer to the same object
        at io.opentelemetry.contrib.jmxmetrics.OtelHelperSynchronousMetricTest.longUpDownCounter(OtelHelperSynchronousMetricTest.java:289)

[jack-berg]

They aren't guaranteed to be identical objects (although those logs appear to indicate they are identical 🤔), but they are guaranteed to aggregate and export to the same metric.

[trask]

if I understand correctly, this issue shouldn't happen anymore as the SDK behavior changed a while back so that no warning log is produced when multiple instruments are initialized with the same name, description, unit, type, and value type.

@rmfitzpatrick @jamesmoessis is this still an issue?

[github-actions[bot]]

This has been automatically marked as stale because it has been marked as needing author feedback and has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.