From b8fea6ada72c3d9ff82e9ec6b3724d65499a923f Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Sun, 17 Sep 2023 16:22:36 -0400 Subject: [PATCH 1/3] Add ThreadFactory tags to differentiate platform and virtual threads Note that the total thread count metrics reported by ThreadMXBean only report on platform threads, so they needn't be updated. --- tritium-metrics/build.gradle | 1 + .../metrics/TaggedMetricsThreadFactory.java | 51 ++++++++++++++---- tritium-metrics/src/main/metrics/metrics.yml | 10 +++- .../TaggedMetricsThreadFactoryTest.java | 54 ++++++++++++++++++- versions.lock | 13 ++--- versions.props | 1 + 6 files changed, 109 insertions(+), 21 deletions(-) diff --git a/tritium-metrics/build.gradle b/tritium-metrics/build.gradle index 58e507666..602319c87 100644 --- a/tritium-metrics/build.gradle +++ b/tritium-metrics/build.gradle @@ -9,6 +9,7 @@ dependencies { implementation 'com.google.code.findbugs:jsr305' implementation 'com.google.guava:guava' + implementation 'com.palantir.nylon:nylon-threads' implementation 'com.palantir.safe-logging:logger' implementation 'com.palantir.safe-logging:preconditions' implementation 'com.palantir.safe-logging:safe-logging' diff --git a/tritium-metrics/src/main/java/com/palantir/tritium/metrics/TaggedMetricsThreadFactory.java b/tritium-metrics/src/main/java/com/palantir/tritium/metrics/TaggedMetricsThreadFactory.java index 43366f706..226e7e2dd 100644 --- a/tritium-metrics/src/main/java/com/palantir/tritium/metrics/TaggedMetricsThreadFactory.java +++ b/tritium-metrics/src/main/java/com/palantir/tritium/metrics/TaggedMetricsThreadFactory.java @@ -20,6 +20,9 @@ import com.codahale.metrics.Meter; import com.palantir.logsafe.Preconditions; import com.palantir.logsafe.Safe; +import com.palantir.nylon.threads.VirtualThreads; +import com.palantir.tritium.metrics.ExecutorMetrics.ThreadsCreated_ThreadType; +import com.palantir.tritium.metrics.ExecutorMetrics.ThreadsRunning_ThreadType; import java.util.concurrent.ThreadFactory; /** @@ -28,42 +31,64 @@ final class TaggedMetricsThreadFactory implements ThreadFactory { private final ThreadFactory delegate; - private final Meter created; - private final Counter running; + // Note that there's no guarantee a given ThreadFactory implementation + // will always produce the same kind of thread for every invocation, so + // we must track both variants. + private final Meter createdPlatform; + private final Meter createdVirtual; + private final Counter runningPlatform; + private final Counter runningVirtual; TaggedMetricsThreadFactory(ThreadFactory delegate, ExecutorMetrics metrics, @Safe String name) { this.delegate = Preconditions.checkNotNull(delegate, "ThreadFactory is required"); Preconditions.checkNotNull(name, "Name is required"); Preconditions.checkNotNull(metrics, "ExecutorMetrics is required"); - this.created = metrics.threadsCreated(name); - this.running = metrics.threadsRunning(name); + this.createdPlatform = metrics.threadsCreated() + .executor(name) + .threadType(ThreadsCreated_ThreadType.PLATFORM) + .build(); + this.createdVirtual = metrics.threadsCreated() + .executor(name) + .threadType(ThreadsCreated_ThreadType.VIRTUAL) + .build(); + this.runningPlatform = metrics.threadsRunning() + .executor(name) + .threadType(ThreadsRunning_ThreadType.PLATFORM) + .build(); + this.runningVirtual = metrics.threadsRunning() + .executor(name) + .threadType(ThreadsRunning_ThreadType.VIRTUAL) + .build(); } @Override public Thread newThread(Runnable runnable) { - Thread result = delegate.newThread( - new InstrumentedTask(Preconditions.checkNotNull(runnable, "Runnable is required"), running)); - created.mark(); + Thread result = + delegate.newThread(new InstrumentedTask(Preconditions.checkNotNull(runnable, "Runnable is required"))); + createdMeterFor(result).mark(); return result; } + private Meter createdMeterFor(Thread thread) { + return VirtualThreads.isVirtual(thread) ? createdVirtual : createdPlatform; + } + @Override public String toString() { return "TaggedMetricsThreadFactory{delegate=" + delegate + '}'; } - private static final class InstrumentedTask implements Runnable { + private final class InstrumentedTask implements Runnable { private final Runnable delegate; - private final Counter running; - InstrumentedTask(Runnable delegate, Counter running) { + InstrumentedTask(Runnable delegate) { this.delegate = delegate; - this.running = running; } @Override public void run() { + Counter running = runningCounterFor(Thread.currentThread()); running.inc(); try { delegate.run(); @@ -72,6 +97,10 @@ public void run() { } } + private Counter runningCounterFor(Thread thread) { + return VirtualThreads.isVirtual(thread) ? runningVirtual : runningPlatform; + } + @Override public String toString() { return "InstrumentedTask{delegate=" + delegate + '}'; diff --git a/tritium-metrics/src/main/metrics/metrics.yml b/tritium-metrics/src/main/metrics/metrics.yml index 2b34d68a1..f1b61bb38 100644 --- a/tritium-metrics/src/main/metrics/metrics.yml +++ b/tritium-metrics/src/main/metrics/metrics.yml @@ -79,9 +79,15 @@ namespaces: # ThreadFactory metrics threads.created: type: meter - tags: [executor] + tags: + - executor + - name: thread-type + values: [platform, virtual] docs: Rate that new threads are created for this executor. threads.running: type: counter - tags: [executor] + tags: + - executor + - name: thread-type + values: [platform, virtual] docs: Number of live threads created by this executor. diff --git a/tritium-metrics/src/test/java/com/palantir/tritium/metrics/TaggedMetricsThreadFactoryTest.java b/tritium-metrics/src/test/java/com/palantir/tritium/metrics/TaggedMetricsThreadFactoryTest.java index 20cde03f7..8b57fb00a 100644 --- a/tritium-metrics/src/test/java/com/palantir/tritium/metrics/TaggedMetricsThreadFactoryTest.java +++ b/tritium-metrics/src/test/java/com/palantir/tritium/metrics/TaggedMetricsThreadFactoryTest.java @@ -24,6 +24,9 @@ import com.google.common.util.concurrent.ThreadFactoryBuilder; import com.google.common.util.concurrent.Uninterruptibles; import com.palantir.logsafe.exceptions.SafeNullPointerException; +import com.palantir.nylon.threads.VirtualThreads; +import com.palantir.tritium.metrics.ExecutorMetrics.ThreadsCreated_ThreadType; +import com.palantir.tritium.metrics.ExecutorMetrics.ThreadsRunning_ThreadType; import com.palantir.tritium.metrics.registry.DefaultTaggedMetricRegistry; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.time.Duration; @@ -44,8 +47,55 @@ void testInstrumentation() { .build(); ThreadFactory instrumented = MetricRegistries.instrument(registry, delegate, name); ExecutorMetrics metrics = ExecutorMetrics.of(registry); - Counter running = metrics.threadsRunning(name); - Meter created = metrics.threadsCreated(name); + Counter running = metrics.threadsRunning() + .executor(name) + .threadType(ThreadsRunning_ThreadType.PLATFORM) + .build(); + Meter created = metrics.threadsCreated() + .executor(name) + .threadType(ThreadsCreated_ThreadType.PLATFORM) + .build(); + assertThat(running.getCount()).isZero(); + assertThat(created.getCount()).isZero(); + CountDownLatch latch = new CountDownLatch(1); + Thread thread = instrumented.newThread(() -> Uninterruptibles.awaitUninterruptibly(latch)); + assertThat(created.getCount()).isOne(); + // thread has not started yet + assertThat(running.getCount()).isZero(); + thread.start(); + // Allow the thread to start in the background + Awaitility.waitAtMost(Duration.ofSeconds(3)).untilAsserted(() -> { + assertThat(created.getCount()).isOne(); + assertThat(running.getCount()).isOne(); + }); + latch.countDown(); + Awaitility.waitAtMost(Duration.ofSeconds(3)).untilAsserted(() -> { + assertThat(created.getCount()).isOne(); + assertThat(running.getCount()).isZero(); + }); + Awaitility.waitAtMost(Duration.ofSeconds(1)) + .untilAsserted(() -> assertThat(thread.isAlive()).isFalse()); + } + + @Test + void testVirtualThreadInstrumentation() { + String name = "name"; + TaggedMetricRegistry registry = new DefaultTaggedMetricRegistry(); + ThreadFactory delegate = VirtualThreads.get() + .orElseThrow(() -> new AssertionError("Expected jdk21+")) + .ofVirtual() + .name("virtual-test-", 0) + .factory(); + ThreadFactory instrumented = MetricRegistries.instrument(registry, delegate, name); + ExecutorMetrics metrics = ExecutorMetrics.of(registry); + Counter running = metrics.threadsRunning() + .executor(name) + .threadType(ThreadsRunning_ThreadType.VIRTUAL) + .build(); + Meter created = metrics.threadsCreated() + .executor(name) + .threadType(ThreadsCreated_ThreadType.VIRTUAL) + .build(); assertThat(running.getCount()).isZero(); assertThat(created.getCount()).isZero(); CountDownLatch latch = new CountDownLatch(1); diff --git a/versions.lock b/versions.lock index baf7a958b..34f44cb79 100644 --- a/versions.lock +++ b/versions.lock @@ -11,11 +11,12 @@ com.google.j2objc:j2objc-annotations:2.8 (1 constraints: be09f5a0) com.palantir.delegate.processors:delegate-processors:1.1.0 (1 constraints: 0405f335) com.palantir.goethe:goethe:0.11.0 (1 constraints: 711660f3) com.palantir.jvm.diagnostics:jvm-diagnostics:0.3.0 (1 constraints: 0505f435) -com.palantir.safe-logging:logger:3.6.0 (2 constraints: aa11c1f9) +com.palantir.nylon:nylon-threads:0.4.0 (1 constraints: 0605f735) +com.palantir.safe-logging:logger:3.6.0 (3 constraints: f21f1b22) com.palantir.safe-logging:logger-slf4j:3.6.0 (1 constraints: 040e6542) com.palantir.safe-logging:logger-spi:3.6.0 (2 constraints: 171e6d7b) -com.palantir.safe-logging:preconditions:3.6.0 (2 constraints: aa11c1f9) -com.palantir.safe-logging:safe-logging:3.6.0 (5 constraints: 2d4085dc) +com.palantir.safe-logging:preconditions:3.6.0 (3 constraints: f21f1b22) +com.palantir.safe-logging:safe-logging:3.6.0 (6 constraints: 754ef87e) com.palantir.tracing:tracing:6.18.0 (1 constraints: 41055f3b) com.palantir.tracing:tracing-api:6.18.0 (2 constraints: 17121d19) com.squareup:javapoet:1.13.0 (3 constraints: 9e27444d) @@ -24,9 +25,12 @@ io.dropwizard.metrics:metrics-jvm:4.2.19 (1 constraints: 4205483b) net.bytebuddy:byte-buddy:1.14.7 (3 constraints: 001ca9f4) org.checkerframework:checker-qual:3.37.0 (3 constraints: de24cc63) org.hdrhistogram:HdrHistogram:2.1.12 (1 constraints: 3805313b) +org.jboss.logging:jboss-logging:3.4.3.Final (3 constraints: f2300ed8) +org.jboss.threads:jboss-threads:3.5.0.Final (3 constraints: b92aebe5) org.jetbrains:annotations:24.0.1 (2 constraints: 11204d00) org.mpierce.metrics.reservoir:hdrhistogram-metrics-reservoir:1.1.3 (1 constraints: 0705f635) org.slf4j:slf4j-api:1.7.36 (7 constraints: d75d16df) +org.wildfly.common:wildfly-common:1.5.4.Final (2 constraints: 741cfbf1) [Test dependencies] com.google.auto.value:auto-value:1.7.4 (1 constraints: 1f1221fb) @@ -48,8 +52,6 @@ org.assertj:assertj-core:3.24.2 (1 constraints: 3d05473b) org.awaitility:awaitility:4.2.0 (1 constraints: 08050536) org.hamcrest:hamcrest:2.1 (1 constraints: 6f0b2cce) org.hamcrest:hamcrest-core:1.3 (1 constraints: cc05fe3f) -org.jboss.logging:jboss-logging:3.4.3.Final (3 constraints: f2300ed8) -org.jboss.threads:jboss-threads:3.5.0.Final (2 constraints: 5a1a5743) org.jboss.xnio:xnio-api:3.8.8.Final (2 constraints: 791a6546) org.jboss.xnio:xnio-nio:3.8.8.Final (1 constraints: c90dd230) org.jetbrains.kotlin:kotlin-stdlib:1.3.40 (2 constraints: 01173b5e) @@ -67,4 +69,3 @@ org.opentest4j:opentest4j:1.3.0 (6 constraints: 7846fee1) org.ow2.asm:asm:9.0 (1 constraints: 030aa6a4) org.slf4j:slf4j-simple:1.7.36 (1 constraints: 43054b3b) org.wildfly.client:wildfly-client-config:1.0.1.Final (1 constraints: 940c6308) -org.wildfly.common:wildfly-common:1.5.4.Final (2 constraints: 741cfbf1) diff --git a/versions.props b/versions.props index 5a6284226..e459bb96c 100644 --- a/versions.props +++ b/versions.props @@ -8,6 +8,7 @@ com.google.guava:guava = 32.1.2-jre com.google.testing.compile:* = 0.19 com.palantir.delegate.processors:* = 1.1.0 com.palantir.jvm.diagnostics:* = 0.3.0 +com.palantir.nylon:* = 0.4.0 com.palantir.safe-logging:* = 3.6.0 com.palantir.tracing:* = 6.18.0 com.squareup:javapoet = 1.13.0 From f9c895a05c5305f5f82786fe65c59517415459fc Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Sun, 17 Sep 2023 20:29:45 +0000 Subject: [PATCH 2/3] Add generated changelog entries --- changelog/@unreleased/pr-1794.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-1794.v2.yml diff --git a/changelog/@unreleased/pr-1794.v2.yml b/changelog/@unreleased/pr-1794.v2.yml new file mode 100644 index 000000000..2efdc46f6 --- /dev/null +++ b/changelog/@unreleased/pr-1794.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Add ThreadFactory tags to differentiate platform and virtual threads + links: + - https://github.com/palantir/tritium/pull/1794 From 888f7e72940c2fe527c923edd330ec987830f990 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Sun, 17 Sep 2023 18:24:39 -0400 Subject: [PATCH 3/3] testVirtualThreadInstrumentation uses an assumeThat for older jdks --- .../metrics/TaggedMetricsThreadFactoryTest.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tritium-metrics/src/test/java/com/palantir/tritium/metrics/TaggedMetricsThreadFactoryTest.java b/tritium-metrics/src/test/java/com/palantir/tritium/metrics/TaggedMetricsThreadFactoryTest.java index 8b57fb00a..e9c614cf8 100644 --- a/tritium-metrics/src/test/java/com/palantir/tritium/metrics/TaggedMetricsThreadFactoryTest.java +++ b/tritium-metrics/src/test/java/com/palantir/tritium/metrics/TaggedMetricsThreadFactoryTest.java @@ -18,6 +18,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assumptions.assumeThat; import com.codahale.metrics.Counter; import com.codahale.metrics.Meter; @@ -25,11 +26,13 @@ import com.google.common.util.concurrent.Uninterruptibles; import com.palantir.logsafe.exceptions.SafeNullPointerException; import com.palantir.nylon.threads.VirtualThreads; +import com.palantir.nylon.threads.VirtualThreads.VirtualThreadSupport; import com.palantir.tritium.metrics.ExecutorMetrics.ThreadsCreated_ThreadType; import com.palantir.tritium.metrics.ExecutorMetrics.ThreadsRunning_ThreadType; import com.palantir.tritium.metrics.registry.DefaultTaggedMetricRegistry; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.time.Duration; +import java.util.Optional; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ThreadFactory; import org.awaitility.Awaitility; @@ -81,8 +84,12 @@ void testInstrumentation() { void testVirtualThreadInstrumentation() { String name = "name"; TaggedMetricRegistry registry = new DefaultTaggedMetricRegistry(); - ThreadFactory delegate = VirtualThreads.get() - .orElseThrow(() -> new AssertionError("Expected jdk21+")) + Optional maybeVirtualThreadSupport = VirtualThreads.get(); + assumeThat(maybeVirtualThreadSupport) + .as("Virtual thread tests require a runtime environment with virtual thread support") + .isPresent(); + ThreadFactory delegate = maybeVirtualThreadSupport + .orElseThrow() .ofVirtual() .name("virtual-test-", 0) .factory();