From 8f2272885b0dd4a70276d37dbabc8e3a1bde1ba0 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 24 Mar 2021 15:54:29 +0100 Subject: [PATCH] Metrics for events (#1601) Updates legman to version 2, which allows the usage of the MicrometerPlugin. The plugin will collect metrics for subscriber invocations and the underlying executor. Furthermore this change will fix the usage of wrong subject context in the asynchronous events. --- .../com/cloudogu/scm/JavaModulePlugin.groovy | 8 +- build.gradle | 1 + gradle/changelog/event_metrics.yaml | 4 + gradle/dependencies.gradle | 6 +- .../java/sonia/scm/event/ScmTestEventBus.java | 5 +- scm-webapp/build.gradle | 4 + .../sonia/scm/event/LegmanScmEventBus.java | 84 +++++++-------- .../scm/metrics/MeterRegistryProvider.java | 29 ++++- .../scm/event/LegmanScmEventBusTest.java | 101 ++++++++++++++++++ 9 files changed, 184 insertions(+), 58 deletions(-) create mode 100644 gradle/changelog/event_metrics.yaml create mode 100644 scm-webapp/src/test/java/sonia/scm/event/LegmanScmEventBusTest.java diff --git a/build-plugins/src/main/groovy/com/cloudogu/scm/JavaModulePlugin.groovy b/build-plugins/src/main/groovy/com/cloudogu/scm/JavaModulePlugin.groovy index 7e31b0ae76..62dd20c001 100644 --- a/build-plugins/src/main/groovy/com/cloudogu/scm/JavaModulePlugin.groovy +++ b/build-plugins/src/main/groovy/com/cloudogu/scm/JavaModulePlugin.groovy @@ -27,6 +27,7 @@ import com.hierynomus.gradle.license.tasks.LicenseCheck import org.gradle.api.Plugin import org.gradle.api.Project import org.gradle.api.publish.maven.MavenPublication +import org.gradle.api.tasks.compile.JavaCompile import org.gradle.api.tasks.javadoc.Javadoc import org.gradle.jvm.toolchain.JavaLanguageVersion @@ -45,12 +46,9 @@ class JavaModulePlugin implements Plugin { withSourcesJar() } - project.compileJava { - options.release = 8 - } - - project.compileTestJava { + project.tasks.withType(JavaCompile) { options.release = 8 + options.encoding = 'UTF-8' } project.tasks.withType(Javadoc) { diff --git a/build.gradle b/build.gradle index 97a16d52e5..888372232d 100644 --- a/build.gradle +++ b/build.gradle @@ -35,6 +35,7 @@ changelog { subprojects { s -> repositories { + mavenLocal() maven { url 'https://packages.scm-manager.org/repository/public/' } diff --git a/gradle/changelog/event_metrics.yaml b/gradle/changelog/event_metrics.yaml new file mode 100644 index 0000000000..d70603e168 --- /dev/null +++ b/gradle/changelog/event_metrics.yaml @@ -0,0 +1,4 @@ +- type: added + description: Metrics for events ([#1601](https://github.com/scm-manager/scm-manager/pull/1601)) +- type: fixed + description: Wrong subject context for asynchronous subscriber ([#1601](https://github.com/scm-manager/scm-manager/pull/1601)) diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index 2ef6786bab..f94d112bd0 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -6,6 +6,8 @@ ext { // TODO upgrade to 2.12.0, but this breaks openapi spec generation jacksonVersion = '2.11.3' + legmanVersion = '2.0.0' + mapstructVersion = '1.3.1.Final' jaxbVersion = '2.3.3' shiroVersion = '1.7.1' @@ -73,7 +75,9 @@ ext { mapstructProcessor: "org.mapstruct:mapstruct-processor:${mapstructVersion}", // events - legman: 'com.github.legman:core:1.6.2', + legman: "com.cloudogu.legman:core:${legmanVersion}", + legmanShiro: "com.cloudogu.legman.support:shiro:${legmanVersion}", + legmanMicrometer: "com.cloudogu.legman.support:micrometer:${legmanVersion}", // xml binding jaxbApi: "jakarta.xml.bind:jakarta.xml.bind-api:${jaxbVersion}", diff --git a/scm-test/src/main/java/sonia/scm/event/ScmTestEventBus.java b/scm-test/src/main/java/sonia/scm/event/ScmTestEventBus.java index a32b28e80d..7147a8ff23 100644 --- a/scm-test/src/main/java/sonia/scm/event/ScmTestEventBus.java +++ b/scm-test/src/main/java/sonia/scm/event/ScmTestEventBus.java @@ -21,13 +21,12 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.event; //~--- non-JDK imports -------------------------------------------------------- import com.github.legman.EventBus; -import com.github.legman.EventBusRegistry; /** * @@ -76,5 +75,5 @@ public class ScmTestEventBus extends ScmEventBus //~--- fields --------------------------------------------------------------- /** Field description */ - private final EventBus eventBus = EventBusRegistry.getEventBus(); + private final EventBus eventBus = new EventBus("testing"); } diff --git a/scm-webapp/build.gradle b/scm-webapp/build.gradle index ac60be27f1..39504b90a2 100644 --- a/scm-webapp/build.gradle +++ b/scm-webapp/build.gradle @@ -113,6 +113,10 @@ dependencies { // util implementation libraries.commonsCompress + // events + implementation libraries.legmanShiro + implementation libraries.legmanMicrometer + // lombok compileOnly libraries.lombok testCompileOnly libraries.lombok diff --git a/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java b/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java index ad55bf6343..efc5b7b4a2 100644 --- a/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java +++ b/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java @@ -21,66 +21,72 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.event; //~--- non-JDK imports -------------------------------------------------------- import com.github.legman.EventBus; import com.github.legman.Subscribe; +import com.github.legman.micrometer.MicrometerPlugin; +import com.github.legman.shiro.ShiroPlugin; +import com.google.common.annotations.VisibleForTesting; +import io.micrometer.core.instrument.Tag; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import sonia.scm.metrics.MeterRegistryProvider; +import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicLong; /** * * @author Sebastian Sdorra */ -public class LegmanScmEventBus extends ScmEventBus -{ +public class LegmanScmEventBus extends ScmEventBus { private static final AtomicLong INSTANCE_COUNTER = new AtomicLong(); - - - /** Field description */ - private static final String NAME = "ScmEventBus-%s"; + private static final String FORMAT_NAME = "ScmEventBus-%s"; /** * the logger for LegmanScmEventBus */ - private static final Logger logger = - LoggerFactory.getLogger(LegmanScmEventBus.class); - - //~--- constructors --------------------------------------------------------- + private static final Logger logger = LoggerFactory.getLogger(LegmanScmEventBus.class); private String name; + private EventBus eventBus; - /** - * Constructs ... - * - */ public LegmanScmEventBus() { - eventBus = create(); + eventBus = create(null); } - private EventBus create() { - name = String.format(NAME, INSTANCE_COUNTER.incrementAndGet()); + @VisibleForTesting + LegmanScmEventBus(Executor executor) { + eventBus = create(executor); + } + + private EventBus create(Executor executor) { + name = String.format(FORMAT_NAME, INSTANCE_COUNTER.incrementAndGet()); logger.info("create new event bus {}", name); - return new EventBus(name); + + MicrometerPlugin micrometerPlugin = new MicrometerPlugin(MeterRegistryProvider.getStaticRegistry()) + .withExecutorTags(Tag.of("type", "fixed")); + + ShiroPlugin shiroPlugin = new ShiroPlugin(); + + EventBus.Builder builder = EventBus.builder() + .withIdentifier(name) + .withPlugins(shiroPlugin, micrometerPlugin); + + if (executor != null) { + builder.withExecutor(executor); + } + + return builder.build(); } - //~--- methods -------------------------------------------------------------- - - /** - * {@inheritDoc} - * - * - * @param event - */ @Override - public void post(Object event) - { + public void post(Object event) { if (eventBus != null) { logger.debug("post {} to event bus {}", event, name); eventBus.post(event); @@ -93,12 +99,9 @@ public class LegmanScmEventBus extends ScmEventBus * Registers a object to the eventbus. * * @param object object to register - * - * @see {@link #register(java.lang.Object)} */ @Override - public void register(Object object) - { + public void register(Object object) { if (eventBus != null) { logger.trace("register {} to event bus {}", object, name); eventBus.register(object); @@ -107,15 +110,8 @@ public class LegmanScmEventBus extends ScmEventBus } } - /** - * {@inheritDoc} - * - * - * @param object - */ @Override - public void unregister(Object object) - { + public void unregister(Object object) { if (eventBus != null) { logger.trace("unregister {} from event bus {}", object, name); @@ -147,11 +143,7 @@ public class LegmanScmEventBus extends ScmEventBus eventBus.shutdown(); } logger.info("recreate event bus because of received RecreateEventBusEvent"); - eventBus = create(); + eventBus = create(null); } - //~--- fields --------------------------------------------------------------- - - /** event bus */ - private EventBus eventBus; } diff --git a/scm-webapp/src/main/java/sonia/scm/metrics/MeterRegistryProvider.java b/scm-webapp/src/main/java/sonia/scm/metrics/MeterRegistryProvider.java index cc5c1c348d..497ff2b289 100644 --- a/scm-webapp/src/main/java/sonia/scm/metrics/MeterRegistryProvider.java +++ b/scm-webapp/src/main/java/sonia/scm/metrics/MeterRegistryProvider.java @@ -24,6 +24,9 @@ package sonia.scm.metrics; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import io.micrometer.core.instrument.Meter; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.binder.jvm.ClassLoaderMetrics; import io.micrometer.core.instrument.binder.jvm.JvmGcMetrics; @@ -37,11 +40,18 @@ import org.slf4j.LoggerFactory; import javax.inject.Inject; import javax.inject.Provider; import javax.inject.Singleton; +import java.util.List; import java.util.Set; @Singleton public class MeterRegistryProvider implements Provider { + private static final CompositeMeterRegistry staticRegistry = new CompositeMeterRegistry(); + + public static MeterRegistry getStaticRegistry() { + return staticRegistry; + } + private static final Logger LOG = LoggerFactory.getLogger(MeterRegistryProvider.class); private final Set providerSet; @@ -49,6 +59,19 @@ public class MeterRegistryProvider implements Provider { @Inject public MeterRegistryProvider(Set providerSet) { this.providerSet = providerSet; + resetStaticRegistry(); + } + + private void resetStaticRegistry() { + Set registries = ImmutableSet.copyOf(staticRegistry.getRegistries()); + for (MeterRegistry registry : registries) { + staticRegistry.remove(registry); + } + + List meters = ImmutableList.copyOf(staticRegistry.getMeters()); + for (Meter meter : meters) { + staticRegistry.remove(meter); + } } @Override @@ -64,6 +87,7 @@ public class MeterRegistryProvider implements Provider { if (providerSet.size() == 1) { MeterRegistry registry = providerSet.iterator().next().getRegistry(); LOG.debug("create meter registry from single registration: {}", registry.getClass()); + staticRegistry.add(registry); return registry; } return createCompositeRegistry(); @@ -71,13 +95,12 @@ public class MeterRegistryProvider implements Provider { private CompositeMeterRegistry createCompositeRegistry() { LOG.debug("create composite meter registry"); - CompositeMeterRegistry registry = new CompositeMeterRegistry(); for (MonitoringSystem provider : providerSet) { MeterRegistry subRegistry = provider.getRegistry(); LOG.debug("register {} as part of composite meter registry", subRegistry.getClass()); - registry.add(subRegistry); + staticRegistry.add(subRegistry); } - return registry; + return staticRegistry; } @SuppressWarnings("java:S2095") // we can't close JvmGcMetrics, but it should be ok diff --git a/scm-webapp/src/test/java/sonia/scm/event/LegmanScmEventBusTest.java b/scm-webapp/src/test/java/sonia/scm/event/LegmanScmEventBusTest.java new file mode 100644 index 0000000000..c827e89fa5 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/event/LegmanScmEventBusTest.java @@ -0,0 +1,101 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.event; + +import com.github.legman.Subscribe; +import org.apache.shiro.SecurityUtils; +import org.apache.shiro.mgt.DefaultSecurityManager; +import org.apache.shiro.subject.SimplePrincipalCollection; +import org.apache.shiro.subject.Subject; +import org.apache.shiro.util.ThreadContext; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import static org.awaitility.Awaitility.await; + +@ExtendWith(MockitoExtension.class) +class LegmanScmEventBusTest { + + private LegmanScmEventBus eventBus; + + @BeforeEach + void setUp() { + DefaultSecurityManager manager = new DefaultSecurityManager(); + ThreadContext.bind(manager); + // we have to use a single thread executor to ensure that the + // same thread is used for the test. This allows us to test, + // that even if the same thread is used the correct subject is in place. + eventBus = new LegmanScmEventBus(Executors.newSingleThreadExecutor()); + } + + @AfterEach + void tearDown() { + eventBus.shutdownEventBus(new ShutdownEventBusEvent()); + ThreadContext.unbindSubject(); + ThreadContext.unbindSecurityManager(); + } + + @Test + void shouldPassShiroContext() { + bindSubject("dent"); + + PrincipalCapturingSubscriber subscriber = new PrincipalCapturingSubscriber(); + eventBus.register(subscriber); + eventBus.post("first"); + + bindSubject("trillian"); + eventBus.post("second"); + + await() + .atMost(1, TimeUnit.SECONDS) + .until(() -> "trillian".equals(subscriber.principal)); + } + + private void bindSubject(String principal) { + Subject dent = new Subject.Builder() + .authenticated(true) + .principals(new SimplePrincipalCollection(principal, "test")) + .buildSubject(); + ThreadContext.bind(dent); + } + + static class PrincipalCapturingSubscriber { + + private Object principal; + + @Subscribe + public void handle(String event) { + principal = SecurityUtils.getSubject().getPrincipal(); + } + + } + +}