This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit c26fb7293e872e240c34ec9edae66c9920c86ede Author: Rene Cordier <[email protected]> AuthorDate: Fri Mar 15 13:57:55 2019 +0700 JAMES-2686 Add pre-deletion hooks metrics --- .../cassandra/CassandraMailboxManagerTest.java | 3 +- .../CassandraMessageIdManagerTestSystem.java | 3 +- .../manager/InMemoryIntegrationResources.java | 5 +- .../resources/META-INF/spring/spring-mailbox.xml | 3 +- .../james/mailbox/store/PreDeletionHooks.java | 13 +++- .../james/mailbox/store/PreDeletionHooksTest.java | 85 +++++++++++++++++++++- .../apache/james/metrics/api/MetricFactory.java | 2 +- 7 files changed, 103 insertions(+), 11 deletions(-) diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMailboxManagerTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMailboxManagerTest.java index bb9e6c9..eb0394c 100644 --- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMailboxManagerTest.java +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMailboxManagerTest.java @@ -23,6 +23,7 @@ import org.apache.james.mailbox.MailboxManagerTest; import org.apache.james.mailbox.cassandra.mail.MailboxAggregateModule; import org.apache.james.mailbox.events.EventBus; import org.apache.james.mailbox.store.PreDeletionHooks; +import org.apache.james.metrics.api.NoopMetricFactory; import org.junit.jupiter.api.extension.RegisterExtension; public class CassandraMailboxManagerTest extends MailboxManagerTest<CassandraMailboxManager> { @@ -34,7 +35,7 @@ public class CassandraMailboxManagerTest extends MailboxManagerTest<CassandraMai return CassandraMailboxManagerProvider.provideMailboxManager( cassandra.getCassandraCluster().getConf(), cassandra.getCassandraCluster().getTypesProvider(), - new PreDeletionHooks(preDeletionHooks())); + new PreDeletionHooks(preDeletionHooks(), new NoopMetricFactory())); } @Override diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMessageIdManagerTestSystem.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMessageIdManagerTestSystem.java index 9a34f8c..cbd640d 100644 --- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMessageIdManagerTestSystem.java +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMessageIdManagerTestSystem.java @@ -31,6 +31,7 @@ import org.apache.james.mailbox.store.MessageIdManagerTestSystem; import org.apache.james.mailbox.store.PreDeletionHooks; import org.apache.james.mailbox.store.quota.ListeningCurrentQuotaUpdater; import org.apache.james.mailbox.store.quota.StoreCurrentQuotaManager; +import org.apache.james.metrics.api.NoopMetricFactory; class CassandraMessageIdManagerTestSystem { @@ -38,7 +39,7 @@ class CassandraMessageIdManagerTestSystem { Set<PreDeletionHook> preDeletionHooks) { CassandraMailboxSessionMapperFactory mapperFactory = CassandraTestSystemFixture.createMapperFactory(cassandra); - return new MessageIdManagerTestSystem(CassandraTestSystemFixture.createMessageIdManager(mapperFactory, quotaManager, eventBus, new PreDeletionHooks(preDeletionHooks)), + return new MessageIdManagerTestSystem(CassandraTestSystemFixture.createMessageIdManager(mapperFactory, quotaManager, eventBus, new PreDeletionHooks(preDeletionHooks, new NoopMetricFactory())), new CassandraMessageId.Factory(), mapperFactory, CassandraTestSystemFixture.createMailboxManager(mapperFactory)) { diff --git a/mailbox/memory/src/test/java/org/apache/james/mailbox/inmemory/manager/InMemoryIntegrationResources.java b/mailbox/memory/src/test/java/org/apache/james/mailbox/inmemory/manager/InMemoryIntegrationResources.java index 655d988..d9b36e7 100644 --- a/mailbox/memory/src/test/java/org/apache/james/mailbox/inmemory/manager/InMemoryIntegrationResources.java +++ b/mailbox/memory/src/test/java/org/apache/james/mailbox/inmemory/manager/InMemoryIntegrationResources.java @@ -329,10 +329,11 @@ public class InMemoryIntegrationResources implements IntegrationResources<StoreM } private PreDeletionHooks createHooks(MailboxManagerPreInstanciationStage preInstanciationStage) { - return new PreDeletionHooks(preDeletionHooksFactories.build() + ImmutableSet<PreDeletionHook> preDeletionHooksSet = preDeletionHooksFactories.build() .stream() .map(biFunction -> biFunction.apply(preInstanciationStage)) - .collect(Guavate.toImmutableSet())); + .collect(Guavate.toImmutableSet()); + return new PreDeletionHooks(preDeletionHooksSet, new NoopMetricFactory()); } } diff --git a/mailbox/spring/src/main/resources/META-INF/spring/spring-mailbox.xml b/mailbox/spring/src/main/resources/META-INF/spring/spring-mailbox.xml index 227edee..294ea5d 100644 --- a/mailbox/spring/src/main/resources/META-INF/spring/spring-mailbox.xml +++ b/mailbox/spring/src/main/resources/META-INF/spring/spring-mailbox.xml @@ -81,7 +81,8 @@ </bean> - <bean id ="preDeletionHooks" class="org.apache.james.mailbox.store.PreDeletionHooks"> + <bean id="preDeletionHooks" class="org.apache.james.mailbox.store.PreDeletionHooks"> <constructor-arg index="0"><set/></constructor-arg> + <constructor-arg index="1" ref="metricFactory" /> </bean> </beans> diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/PreDeletionHooks.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/PreDeletionHooks.java index 849079d..aac7b5c 100644 --- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/PreDeletionHooks.java +++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/PreDeletionHooks.java @@ -24,6 +24,8 @@ import java.util.Set; import javax.inject.Inject; import org.apache.james.mailbox.extension.PreDeletionHook; +import org.apache.james.metrics.api.MetricFactory; +import org.apache.james.metrics.api.NoopMetricFactory; import com.google.common.collect.ImmutableSet; @@ -32,20 +34,25 @@ import reactor.core.publisher.Mono; import reactor.core.scheduler.Schedulers; public class PreDeletionHooks { - public static final PreDeletionHooks NO_PRE_DELETION_HOOK = new PreDeletionHooks(ImmutableSet.of()); + public static final PreDeletionHooks NO_PRE_DELETION_HOOK = new PreDeletionHooks(ImmutableSet.of(), new NoopMetricFactory()); + + static final String PRE_DELETION_HOOK_METRIC_NAME = "preDeletionHook"; private final Set<PreDeletionHook> hooks; + private final MetricFactory metricFactory; @Inject - public PreDeletionHooks(Set<PreDeletionHook> hooks) { + public PreDeletionHooks(Set<PreDeletionHook> hooks, MetricFactory metricFactory) { this.hooks = hooks; + this.metricFactory = metricFactory; } public Mono<Void> runHooks(PreDeletionHook.DeleteOperation deleteOperation) { return Flux.fromIterable(hooks) .publishOn(Schedulers.elastic()) .limitRate(1) - .flatMap(hook -> hook.notifyDelete(deleteOperation)) + .flatMap(hook -> metricFactory.runPublishingTimerMetric(PRE_DELETION_HOOK_METRIC_NAME, + Mono.from(hook.notifyDelete(deleteOperation)))) .then(); } } diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/PreDeletionHooksTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/PreDeletionHooksTest.java index 79ab6ca..81ec41a 100644 --- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/PreDeletionHooksTest.java +++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/PreDeletionHooksTest.java @@ -19,6 +19,8 @@ package org.apache.james.mailbox.store; +import static org.apache.james.mailbox.store.PreDeletionHooks.PRE_DELETION_HOOK_METRIC_NAME; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; @@ -29,25 +31,35 @@ import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import java.time.Duration; +import java.util.Collection; import java.util.Date; +import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; +import java.util.function.Consumer; import javax.mail.Flags; +import org.apache.commons.lang3.NotImplementedException; import org.apache.james.mailbox.MessageUid; import org.apache.james.mailbox.MetadataWithMailboxId; import org.apache.james.mailbox.extension.PreDeletionHook; import org.apache.james.mailbox.model.MessageMetaData; import org.apache.james.mailbox.model.TestId; import org.apache.james.mailbox.model.TestMessageId; +import org.apache.james.metrics.api.Metric; +import org.apache.james.metrics.api.MetricFactory; +import org.apache.james.metrics.api.TimeMetric; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import org.reactivestreams.Publisher; +import com.google.common.base.Stopwatch; +import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Multimap; +import com.google.common.collect.Multimaps; import reactor.core.publisher.Mono; @@ -63,13 +75,63 @@ class PreDeletionHooksTest { private PreDeletionHook hook1; private PreDeletionHook hook2; private PreDeletionHooks testee; + private RecordingMetricFactory metricFactory; + + public static class RecordingTimeMetric implements TimeMetric { + private final String name; + private final Stopwatch stopwatch = Stopwatch.createStarted(); + private final Consumer<Long> publishCallback; + + RecordingTimeMetric(String name, Consumer<Long> publishCallback) { + this.name = name; + this.publishCallback = publishCallback; + } + + @Override + public String name() { + return name; + } + + @Override + public long stopAndPublish() { + long elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS); + publishCallback.accept(elapsed); + return elapsed; + } + } + + public static class RecordingMetricFactory implements MetricFactory { + private final Multimap<String, Long> executionTimesInMs = Multimaps.synchronizedSetMultimap(HashMultimap.create()); + + @Override + public Metric generate(String name) { + throw new NotImplementedException("Not implemented"); + } + + @Override + public TimeMetric timer(String name) { + return new RecordingTimeMetric(name, executionTime -> { + synchronized (executionTimesInMs) { + executionTimesInMs.put(name, executionTime); + } + }); + } + + Collection<Long> executionTimesFor(String name) { + synchronized (executionTimesInMs) { + return executionTimesInMs.get(name); + } + } + } @BeforeEach void setUp() { hook1 = mock(PreDeletionHook.class); hook2 = mock(PreDeletionHook.class); - testee = new PreDeletionHooks(ImmutableSet.of(hook1, hook2)); + metricFactory = new RecordingMetricFactory(); + + testee = new PreDeletionHooks(ImmutableSet.of(hook1, hook2), metricFactory); } @Test @@ -153,4 +215,23 @@ class PreDeletionHooksTest { .describedAs("RunHook does not throw if hooks are executed in a sequential manner") .doesNotThrowAnyException(); } + + @Test + void runHooksShouldPublishTimerMetrics() { + long sleepDurationInMs = Duration.ofSeconds(1).toMillis(); + + Mono<Void> notifyDeleteAnswer = Mono.fromCallable(() -> { + Thread.sleep(sleepDurationInMs); + return Mono.empty(); + }).then(); + + when(hook1.notifyDelete(any())).thenReturn(notifyDeleteAnswer); + when(hook2.notifyDelete(any())).thenReturn(notifyDeleteAnswer); + + testee.runHooks(DELETE_OPERATION).block(); + + assertThat(metricFactory.executionTimesFor(PRE_DELETION_HOOK_METRIC_NAME)) + .hasSize(2) + .allSatisfy(executionInMs -> assertThat(executionInMs).isGreaterThanOrEqualTo(sleepDurationInMs)); + } } \ No newline at end of file diff --git a/metrics/metrics-api/src/main/java/org/apache/james/metrics/api/MetricFactory.java b/metrics/metrics-api/src/main/java/org/apache/james/metrics/api/MetricFactory.java index d476cf4..18c34f7 100644 --- a/metrics/metrics-api/src/main/java/org/apache/james/metrics/api/MetricFactory.java +++ b/metrics/metrics-api/src/main/java/org/apache/james/metrics/api/MetricFactory.java @@ -40,7 +40,7 @@ public interface MetricFactory { default <T> Mono<T> runPublishingTimerMetric(String name, Mono<T> mono) { TimeMetric timer = timer(name); - return mono.doOnNext(ignored -> timer.stopAndPublish()); + return mono.doOnSuccess(success -> timer.stopAndPublish()); } default void runPublishingTimerMetric(String name, Runnable runnable) { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
