Merge pull request #6 from xenit-eu/fix-meterfilter

Move the applying of MeterFilters to the MeterRegistryRegistrar
xenit-eu · Aug 12, 2019 · 57942cf · 57942cf
2 parents 024143f + 96af078
commit 57942cf
Show file tree

Hide file tree

Showing 8 changed files with 93 additions and 52 deletions.
diff --git a/...lemetry-platform/src/main/java/eu/xenit/alfred/telemetry/binder/MeterBinderRegistrar.java b/...lemetry-platform/src/main/java/eu/xenit/alfred/telemetry/binder/MeterBinderRegistrar.java
@@ -47,14 +47,9 @@ public void afterPropertiesSet() {
             return;
         }
 
-        this.addFilters(meterRegistry);
         this.addBinders(meterRegistry);
     }
 
-    protected void addFilters(final MeterRegistry registry) {
-        ctx.getBeansOfType(MeterFilter.class).values().forEach(registry.config()::meterFilter);
-    }
-
     private void addBinders(final MeterRegistry registry) {
         ctx.getBeansOfType(MeterBinder.class).values().stream()
                 .filter(this::isBinderEnabled)

diff --git a/...src/main/java/eu/xenit/alfred/telemetry/binder/care4alf/Care4AlfMeterBinderRegistrar.java b/...src/main/java/eu/xenit/alfred/telemetry/binder/care4alf/Care4AlfMeterBinderRegistrar.java
@@ -2,6 +2,7 @@
 
 import eu.xenit.alfred.telemetry.binder.MeterBinderRegistrar;
 import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.Tags;
 import io.micrometer.core.instrument.composite.CompositeMeterRegistry;
 import io.micrometer.core.instrument.config.MeterFilter;
 import org.slf4j.Logger;
@@ -12,16 +13,18 @@ public class Care4AlfMeterBinderRegistrar extends MeterBinderRegistrar {
     private static final Logger LOGGER = LoggerFactory.getLogger(Care4AlfMeterBinderRegistrar.class);
 
     public Care4AlfMeterBinderRegistrar(MeterRegistry meterRegistry) {
-        super(new CompositeMeterRegistry().add(meterRegistry));
+        super(wrapRegistry(meterRegistry));
         this.filtersEnabledByDefault = false;
     }
 
-    @Override
-    protected void addFilters(MeterRegistry registry) {
-        super.addFilters(registry);
-        registry.config()
-                .meterFilter(MeterFilter.replaceTagValues("application", s -> "c4a"))
+    private static MeterRegistry wrapRegistry(final MeterRegistry originalRegistry) {
+        final MeterRegistry wrapped = new CompositeMeterRegistry().add(originalRegistry);
+
+        wrapped.config()
+                .meterFilter(MeterFilter.commonTags(Tags.of("application", "c4a")))
                 .meterFilter(new TicketMetricsMeterFilter());
+
+        return wrapped;
     }
 
     @Override

diff --git a/...metry-platform/src/main/java/eu/xenit/alfred/telemetry/config/CommonTagFilterFactory.java b/...metry-platform/src/main/java/eu/xenit/alfred/telemetry/config/CommonTagFilterFactory.java
@@ -1,12 +1,16 @@
 package eu.xenit.alfred.telemetry.config;
 
+import io.micrometer.core.instrument.Meter;
 import io.micrometer.core.instrument.Tag;
 import io.micrometer.core.instrument.config.MeterFilter;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
+import java.util.ArrayList;
 import java.util.Hashtable;
+import java.util.List;
 import java.util.Properties;
 import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
 import javax.annotation.Nonnull;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -32,7 +36,23 @@ public Class<?> getObjectType() {
     @Override
     @Nonnull
     protected MeterFilter createInstance() {
-        return MeterFilter.commonTags(getCommonTags());
+        return commonTagsIfNotExists(getCommonTags());
+    }
+
+    public static MeterFilter commonTagsIfNotExists(Iterable<Tag> tags) {
+        return new MeterFilter() {
+            @Override
+            @Nonnull
+            public Meter.Id map(@Nonnull Meter.Id id) {
+                List<Tag> allTags = new ArrayList<>(id.getTags());
+
+                StreamSupport.stream(tags.spliterator(), false)
+                        .filter(t -> id.getTag(t.getKey()) == null)
+                        .forEach(allTags::add);
+
+                return new Meter.Id(id.getName(), allTags, id.getBaseUnit(), id.getDescription(), id.getType());
+            }
+        };
     }
 
     private Iterable<Tag> getCommonTags() {

diff --git a/...elemetry-platform/src/main/java/eu/xenit/alfred/telemetry/registry/RegistryRegistrar.java b/...elemetry-platform/src/main/java/eu/xenit/alfred/telemetry/registry/RegistryRegistrar.java
@@ -7,34 +7,35 @@
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.Metrics;
 import io.micrometer.core.instrument.composite.CompositeMeterRegistry;
-import java.util.Arrays;
-import java.util.List;
+import io.micrometer.core.instrument.config.MeterFilter;
+import javax.annotation.Nonnull;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.InitializingBean;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.ApplicationContextAware;
 
 /**
  * {@link InitializingBean} that processes {@link RegistryFactoryWrapper}'s and registers the corresponding {@link
  * MeterRegistry}, retrieved via the {@link RegistryFactory}, into the {@link Metrics#globalRegistry global meter
  * registry} if the registry {@link RegistryFactoryWrapper#isRegistryEnabled() is enabled} and the registry {@link
  * RegistryFactoryWrapper#isRegistryAvailableOnClassPath() is available on the classpath}
  */
-public class RegistryRegistrar implements InitializingBean {
+public class RegistryRegistrar implements InitializingBean, ApplicationContextAware {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(RegistryRegistrar.class);
 
     private CompositeMeterRegistry globalMeterRegistry;
+    private ApplicationContext ctx;
 
-    private List<RegistryFactoryWrapper> registryFactoryWrappers;
-
-    public RegistryRegistrar(CompositeMeterRegistry globalMeterRegistry, RegistryFactoryWrapper... wrappers) {
+    public RegistryRegistrar(CompositeMeterRegistry globalMeterRegistry) {
         this.globalMeterRegistry = globalMeterRegistry;
-        this.registryFactoryWrappers = Arrays.asList(wrappers);
     }
 
     @Override
     public void afterPropertiesSet() {
-        registryFactoryWrappers.forEach(this::processRegistryFactoryWrapper);
+        ctx.getBeansOfType(RegistryFactoryWrapper.class).values().forEach(this::processRegistryFactoryWrapper);
     }
 
     private void processRegistryFactoryWrapper(RegistryFactoryWrapper factoryWrapper) {
@@ -61,12 +62,19 @@ private void processRegistryFactoryWrapper(RegistryFactoryWrapper factoryWrapper
             return;
         }
 
-        globalMeterRegistry.add(factoryWrapper.getRegistryFactory().createRegistry());
+        final MeterRegistry registry = factoryWrapper.getRegistryFactory().createRegistry();
+        this.addFilters(registry);
+
+        globalMeterRegistry.add(registry);
         LOGGER.info("Registered Micrometer registry '{}'", factoryWrapper.getRegistryClass());
 
         this.incrementRegistryCounter();
     }
 
+    private void addFilters(final MeterRegistry registry) {
+        ctx.getBeansOfType(MeterFilter.class).values().forEach(registry.config()::meterFilter);
+    }
+
     private void incrementRegistryCounter() {
         Counter counter = globalMeterRegistry.counter("alfred.telemetry.registries");
         //noinspection ConstantConditions -> can be null in unit test scenarios
@@ -77,4 +85,9 @@ private void incrementRegistryCounter() {
         }
 
     }
+
+    @Override
+    public void setApplicationContext(@Nonnull ApplicationContext applicationContext) throws BeansException {
+        this.ctx = applicationContext;
+    }
 }
diff --git a/...src/main/resources/alfresco/module/alfred-telemetry-platform/context/registry-context.xml b/...src/main/resources/alfresco/module/alfred-telemetry-platform/context/registry-context.xml
@@ -12,14 +12,6 @@
     <bean id="alfred-telemetry.RegistryRegistrar"
             class="eu.xenit.alfred.telemetry.registry.RegistryRegistrar">
         <constructor-arg ref="meterRegistry"/>
-        <constructor-arg>
-            <list>
-                <ref bean="alfred-telemetry.SimpleRegistryFactoryWrapper"/>
-                <ref bean="alfred-telemetry.JmxRegistryFactoryWrapper"/>
-                <ref bean="alfred-telemetry.GraphiteRegistryFactoryWrapper"/>
-                <ref bean="alfred-telemetry.PrometheusRegistryFactoryWrapper"/>
-            </list>
-        </constructor-arg>
     </bean>
 
     <!-- Simple Meter registry -->

diff --git a/...try-platform/src/test/java/eu/xenit/alfred/telemetry/binder/MeterBinderRegistrarTest.java b/...try-platform/src/test/java/eu/xenit/alfred/telemetry/binder/MeterBinderRegistrarTest.java
@@ -109,19 +109,6 @@ void enabled() {
                     is(1));
         }
 
-        @Test
-        void addMeterFilters() {
-            MeterFilter mockedFilter = MeterFilter.commonTags(Tags.of("stop", "hammertime"));
-            when(applicationContext.getBeansOfType(MeterFilter.class))
-                    .thenReturn(Collections.singletonMap("filter", mockedFilter));
-
-            registrar.afterPropertiesSet();
-
-            Counter testCounter = registrar.getMeterRegistry().find(BasicTestMetrics.COUNTER_NAME).counter();
-            assertThat(testCounter, is(not(nullValue())));
-            assertThat(testCounter.getId().getTag("stop"), is("hammertime"));
-        }
-
     }
 
     @Nested

diff --git a/...test/java/eu/xenit/alfred/telemetry/binder/care4alf/Care4AlfMeterBinderRegistrarTest.java b/...test/java/eu/xenit/alfred/telemetry/binder/care4alf/Care4AlfMeterBinderRegistrarTest.java
@@ -5,6 +5,7 @@
 import static org.mockito.Mockito.when;
 
 import eu.xenit.alfred.telemetry.binder.MeterBinderRegistrar;
+import eu.xenit.alfred.telemetry.config.CommonTagFilterFactory;
 import io.micrometer.core.instrument.Counter;
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.Tags;
@@ -23,16 +24,17 @@ class Care4AlfMeterBinderRegistrarTest {
 
     private MeterBinderRegistrar registrar;
 
+    private MeterRegistry globalRegistry;
+
     @Mock
     private ApplicationContext applicationContext;
 
     @BeforeEach
     void setup() {
-        when(applicationContext.getBeansOfType(MeterFilter.class))
-                .thenReturn(
-                        Collections.singletonMap("filter", MeterFilter.commonTags(Tags.of("application", "alfresco"))));
-        MeterRegistry registry = new SimpleMeterRegistry();
-        registrar = new Care4AlfMeterBinderRegistrar(registry);
+        globalRegistry = new SimpleMeterRegistry();
+        globalRegistry.config().meterFilter(CommonTagFilterFactory.commonTagsIfNotExists(Tags.of("application", "alfresco")));
+
+        registrar = new Care4AlfMeterBinderRegistrar(globalRegistry);
         registrar.setApplicationContext(applicationContext);
         registrar.setEnabled(true);
     }
@@ -45,6 +47,9 @@ void commonTags() {
         registrar.afterPropertiesSet();
         final Counter counter = registrar.getMeterRegistry().counter("counter");
         assertThat(counter.getId().getTag("application"), is("c4a"));
+
+        final Counter counterThroughGlobalRegistry = globalRegistry.counter("counter");
+        assertThat(counterThroughGlobalRegistry.getId().getTag("application"), is("alfresco"));
     }
 
 }
diff --git a/...etry-platform/src/test/java/eu/xenit/alfred/telemetry/registry/RegistryRegistrarTest.java b/...etry-platform/src/test/java/eu/xenit/alfred/telemetry/registry/RegistryRegistrarTest.java
@@ -11,17 +11,33 @@
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.composite.CompositeMeterRegistry;
 import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
+import java.util.Collections;
+import org.junit.Before;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.support.GenericXmlApplicationContext;
 
 @ExtendWith(MockitoExtension.class)
 class RegistryRegistrarTest {
 
     @Mock
     private CompositeMeterRegistry meterRegistry;
 
+    @Mock
+    private ApplicationContext applicationContext;
+
+    private RegistryRegistrar registrar;
+
+    @BeforeEach
+    void setup() {
+        registrar = new RegistryRegistrar(meterRegistry);
+        registrar.setApplicationContext(applicationContext);
+    }
+
     @Test
     void dontRegisterIfVersionIfIncompatibleRegistryVersion() {
         RegistryFactoryWrapper factoryWrapper = mock(RegistryFactoryWrapper.class);
@@ -31,7 +47,9 @@ void dontRegisterIfVersionIfIncompatibleRegistryVersion() {
         when(factoryWrapper.getRegistryVersion())
                 .thenReturn(Version.fromString("101.12.07"));
 
-        RegistryRegistrar registrar = new RegistryRegistrar(meterRegistry, factoryWrapper);
+        when(applicationContext.getBeansOfType(RegistryFactoryWrapper.class))
+                .thenReturn(Collections.singletonMap("factoryWrapper", factoryWrapper));
+
         registrar.afterPropertiesSet();
 
         verify(meterRegistry, never()).add(any(MeterRegistry.class));
@@ -42,7 +60,9 @@ void dontRegisterIfRegistryEnabled() {
         RegistryFactoryWrapper factoryWrapper = mock(RegistryFactoryWrapper.class);
         when(factoryWrapper.isRegistryEnabled()).thenReturn(false);
 
-        RegistryRegistrar registrar = new RegistryRegistrar(meterRegistry, factoryWrapper);
+        when(applicationContext.getBeansOfType(RegistryFactoryWrapper.class))
+                .thenReturn(Collections.singletonMap("factoryWrapper", factoryWrapper));
+
         registrar.afterPropertiesSet();
 
         verify(meterRegistry, never()).add(any(MeterRegistry.class));
@@ -54,7 +74,9 @@ void dontRegisterIfRegistryNotOnClasspath() {
         when(factoryWrapper.isRegistryEnabled()).thenReturn(true);
         when(factoryWrapper.isRegistryAvailableOnClassPath()).thenReturn(false);
 
-        RegistryRegistrar registrar = new RegistryRegistrar(meterRegistry, factoryWrapper);
+        when(applicationContext.getBeansOfType(RegistryFactoryWrapper.class))
+                .thenReturn(Collections.singletonMap("factoryWrapper", factoryWrapper));
+
         registrar.afterPropertiesSet();
 
         verify(meterRegistry, never()).add(any(MeterRegistry.class));
@@ -67,7 +89,9 @@ void register() {
         when(factoryWrapper.isRegistryAvailableOnClassPath()).thenReturn(true);
         when(factoryWrapper.getRegistryFactory()).thenReturn(SimpleMeterRegistry::new);
 
-        RegistryRegistrar registrar = new RegistryRegistrar(meterRegistry, factoryWrapper);
+        when(applicationContext.getBeansOfType(RegistryFactoryWrapper.class))
+                .thenReturn(Collections.singletonMap("factoryWrapper", factoryWrapper));
+
         registrar.afterPropertiesSet();
 
         verify(meterRegistry).add(any(SimpleMeterRegistry.class));
@@ -81,7 +105,9 @@ void register_verifyVersions() {
         when(factoryWrapper.getRegistryFactory()).thenReturn(SimpleMeterRegistry::new);
         when(factoryWrapper.getRegistryVersion()).thenReturn(VersionUtilTest.getMicrometerVersionFromGradle());
 
-        RegistryRegistrar registrar = new RegistryRegistrar(meterRegistry, factoryWrapper);
+        when(applicationContext.getBeansOfType(RegistryFactoryWrapper.class))
+                .thenReturn(Collections.singletonMap("factoryWrapper", factoryWrapper));
+
         registrar.afterPropertiesSet();
 
         verify(meterRegistry).add(any(SimpleMeterRegistry.class));