Skip to content

Conversation

@jhaals
Copy link
Member

@jhaals jhaals commented Nov 8, 2017

Java9 fails with InaccessibleObjectException which causes all metric
reporting to fail. This is a new exception in java9 so we can't catch it
in Java8, simply return 0 if exception is thrown. This is tested in
java8/java9

java.lang.reflect.InaccessibleObjectException: Unable to make public long com.sun.management.internal.OperatingSystemImpl.getOpenFileDescriptorCount() accessible: module jdk.management does not "opens com.sun.management.internal" to unnamed module @385c9627

Java9 fails with `InaccessibleObjectException` which causes all metric
reporting to fail. This is a new exception in java9 so we can't catch it
in Java8, simply return 0 if exception is thrown. This is tested in
java8/java9

```
java.lang.reflect.InaccessibleObjectException: Unable to make public long com.sun.management.internal.OperatingSystemImpl.getOpenFileDescriptorCount() accessible: module jdk.management does not "opens com.sun.management.internal" to unnamed module @385c9627
```
@udoprog
Copy link
Contributor

udoprog commented Nov 8, 2017

Hey,

Any way this can be detected before we are polling the Gauge, so that we don't have to register the gauge at all if it's not supported?

Maybe try to poll it once during registration?

@jhaals
Copy link
Member Author

jhaals commented Nov 8, 2017

Hi,
Yes that's a good points, we created a quickfix in apollo for this spotify/apollo#231 but we can patch it to not register at all in java9

@udoprog udoprog added the type:enhancement New feature or request label Jan 2, 2018
@udoprog
Copy link
Contributor

udoprog commented Jan 2, 2018

@jhaals proper fix would be appreciated if you have the time :)

@jhaals
Copy link
Member Author

jhaals commented Jan 3, 2018

@udoprog I can wrap the whole gauges.put() in a try catch which would return an empty HashMap on every call to getMetrics, would that be sufficient or do you have anything else in mind?
An alternative could be to throw an exception in the constructor and fail early so that the service using the library can handle it themselves.

@jhaals
Copy link
Member Author

jhaals commented Jan 3, 2018

This was merged into dropwizard/metrics 12 days ago dropwizard/metrics#1236
It won't be out until 4.0 but I guess we can copy their code if you prefer that.

@udoprog
Copy link
Contributor

udoprog commented Jan 3, 2018

Hm, that's an interesting approach. I'd think it would be preferably not to register anything if the operation is not supported.

Apparently we can do an instanceof check instead, that seem to be cleaner than a try-catch?

@mattnworb
Copy link
Member

I upgraded the metrics library for exactly this issue in #30, sorry I missed this PR when making that

@jhaals
Copy link
Member Author

jhaals commented Oct 4, 2018

No worries @mattnworb, your PR is preferred 👍

@jhaals jhaals closed this Oct 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants