Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4.x fix pinned meter type #9657

Merged
merged 3 commits into from
Jan 17, 2025
Merged

Conversation

tjquinno
Copy link
Member

Description

The recent addition of meters for virtual threads contains code that, when a pinned thread is reported, looks up the previously-registered Timer for recent pinned threads so it can update that timer. That look-up logic is flawed because it used the wrong name.

There is a Gauge for total pinned threads as well as a Timer for recent pinned threads(recent because as with all distribution summaries the timer's decays over time so "old" pinned thread events might eventually not influence the data).

The code that attempted to look up the timer used the gauge's name. Helidon metrics rejected the attempt to look up the gauge as a timer.

This PR corrects the name used for the look-up and also adds the correct scope tag and value to the look-up.

There is also a new test to make sure the look-up works correctly.

Here is the stack trace before the fix:

java.lang.IllegalArgumentException: Matching meter is of type io.micrometer.core.instrument.composite.CompositeGauge but io.helidon.metrics.api.Timer was requested
        at io.helidon.metrics.providers.micrometer.MMeterRegistry.meter(MMeterRegistry.java:333)
        at io.helidon.metrics.api.MeterRegistry.timer(MeterRegistry.java:135)
        at io.helidon.metrics.systemmeters.VThreadSystemMetersProvider.findPinned(VThreadSystemMetersProvider.java:119)
        at io.helidon.common.LazyValueImpl.get(LazyValueImpl.java:118)
        at io.helidon.metrics.systemmeters.VThreadSystemMetersProvider.recordThreadPin(VThreadSystemMetersProvider.java:146)
        at jdk.jfr/jdk.jfr.internal.consumer.Dispatcher$EventDispatcher.offer(Dispatcher.java:53)
        at jdk.jfr/jdk.jfr.internal.consumer.Dispatcher.dispatch(Dispatcher.java:185)
        at jdk.jfr/jdk.jfr.internal.consumer.EventDirectoryStream.processOrdered(EventDirectoryStream.java:253)
        at jdk.jfr/jdk.jfr.internal.consumer.EventDirectoryStream.processRecursionSafe(EventDirectoryStream.java:164)
        at jdk.jfr/jdk.jfr.internal.consumer.EventDirectoryStream.process(EventDirectoryStream.java:124)
        at jdk.jfr/jdk.jfr.internal.consumer.AbstractEventStream.execute(AbstractEventStream.java:261)
        at jdk.jfr/jdk.jfr.internal.consumer.AbstractEventStream$1.run(AbstractEventStream.java:284)
        at jdk.jfr/jdk.jfr.internal.consumer.AbstractEventStream$1.run(AbstractEventStream.java:281)
        at java.base/java.security.AccessController.doPrivileged(AccessController.java:400)
        at jdk.jfr/jdk.jfr.internal.consumer.AbstractEventStream.run(AbstractEventStream.java:281)
        at jdk.jfr/jdk.jfr.internal.consumer.AbstractEventStream.lambda$startAsync$1(AbstractEventStream.java:224)
        at java.base/java.lang.Thread.run(Thread.java:1583)

Documentation

Bug fix. No doc impact.

@tjquinno tjquinno requested a review from spericas January 16, 2025 15:34
@tjquinno tjquinno self-assigned this Jan 16, 2025
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jan 16, 2025
@romain-grecourt romain-grecourt force-pushed the 4.x-fix-pinned-meter-type branch from 6923ff6 to 46b3843 Compare January 16, 2025 22:43
@tjquinno tjquinno merged commit f419a62 into helidon-io:main Jan 17, 2025
58 checks passed
@tjquinno tjquinno deleted the 4.x-fix-pinned-meter-type branch January 17, 2025 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants