Merge pull request #282 from ExpediaGroup/feature/waggledance_metrics

Added waggledance metrics with tags for enabling filtering in datadog

CHANGELOG.md

@@ -1,4 +1,14 @@
-## [3.11.3] - TBD
+## [3.11.4] - 2023-08-23
+### Changed
+- Metrics have been incorporated into Waggle Dance with the inclusion of tags, which will facilitate filtering within Datadog.
+- Exclude jetty-all from core module, because it makes spring start fail and makes `WaggleDanceIntegrationTest` fail.
+- Upgrade maven.surefire.plugin.version to 3.1.2 (was 3.0.0-m5).
+- Exclude jdk.tools clashes with > java8 JDK.
+### Fixed
+- Exclude Junit5 dependencies as they clashed with Junit4 and caused maven to stop running the tests.
+- Fixed metric(graphite) integration test (was broken since 3.10.12 (spring-boot upgrade).
+
+## [3.11.3] - YANKED
 ### Fixed
 - Exclude Junit5 dependencies as they clashed with Junit4 and caused maven to stop running the tests.
 - Fixed metric(graphite) integration test (was broken since 3.10.12 (spring-boot upgrade).

pom.xml

@@ -1,5 +1,4 @@
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
   <modelVersion>4.0.0</modelVersion>
 
   <parent>
@@ -10,7 +9,7 @@
 
   <groupId>com.hotels</groupId>
   <artifactId>waggle-dance-parent</artifactId>
-  <version>3.11.3-SNAPSHOT</version>
+  <version>3.11.4-SNAPSHOT</version>
   <packaging>pom</packaging>
   <name>Waggle Dance Parent</name>
   <description>Hive Metastore federation service.</description>

waggle-dance-api/pom.xml

@@ -4,7 +4,7 @@
   <parent>
     <groupId>com.hotels</groupId>
     <artifactId>waggle-dance-parent</artifactId>
-    <version>3.11.3-SNAPSHOT</version>
+    <version>3.11.4-SNAPSHOT</version>
   </parent>
 
   <artifactId>waggle-dance-api</artifactId>

waggle-dance-boot/pom.xml

@@ -4,7 +4,7 @@
   <parent>
     <groupId>com.hotels</groupId>
     <artifactId>waggle-dance-parent</artifactId>
-    <version>3.11.3-SNAPSHOT</version>
+    <version>3.11.4-SNAPSHOT</version>
   </parent>
 
   <artifactId>waggle-dance-boot</artifactId>

waggle-dance-core/pom.xml

@@ -4,7 +4,7 @@
   <parent>
     <groupId>com.hotels</groupId>
     <artifactId>waggle-dance-parent</artifactId>
-    <version>3.11.3-SNAPSHOT</version>
+    <version>3.11.4-SNAPSHOT</version>
   </parent>
 
   <artifactId>waggle-dance-core</artifactId>

waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/metrics/MonitoredAspect.java

@@ -27,6 +27,8 @@
 import org.springframework.beans.factory.annotation.Configurable;
 
 import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.Tag;
+import io.micrometer.core.instrument.Tags;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
@@ -51,6 +53,8 @@ public Object monitor(ProceedingJoinPoint pjp) throws Throwable {
   public Object monitor(ProceedingJoinPoint pjp, Monitored monitored) throws Throwable {
 
     String metricBasePath = buildMetricBasePath(pjp);
+    String className = getClassName(pjp);
+    String methodName = getMethodName(pjp);
 
     String result = null;
     Stopwatch stopwatch = Stopwatch.createStarted();
@@ -63,10 +67,12 @@ public Object monitor(ProceedingJoinPoint pjp, Monitored monitored) throws Throw
       throw t;
     } finally {
       stopwatch.stop();
-      increment(buildMetricPath(COUNTER, metricBasePath, getMonitorMetastore(), "calls"));
-      increment(buildMetricPath(COUNTER, metricBasePath, getMonitorMetastore(), result));
-      submit(buildMetricPath(TIMER, metricBasePath, getMonitorMetastore(), "duration"),
-          stopwatch.elapsed(TimeUnit.MILLISECONDS));
+      increment(buildMetricName(COUNTER, metricBasePath, getMonitorMetastore(), "calls"),methodName,
+          buildMetricName(COUNTER, className, "calls"));
+      increment(buildMetricName(COUNTER, metricBasePath, getMonitorMetastore(), result),methodName,
+          buildMetricName(COUNTER, className, result));
+      submit(buildMetricName(TIMER, metricBasePath, getMonitorMetastore(), "duration"),
+          stopwatch.elapsed(TimeUnit.MILLISECONDS),methodName, buildMetricName(TIMER, className, "duration"));
     }
   }
 
@@ -75,21 +81,39 @@ void setMeterRegistry(MeterRegistry meterRegistry) {
     this.meterRegistry = meterRegistry;
   }
 
-  private void increment(String metricName) {
+  private void increment(String metricName, String methodName, String metricWithTag) {
     if (meterRegistry != null) {
+      Iterable<Tag> tags = getMetricsTags(methodName);
       meterRegistry.counter(metricName).increment();
+      meterRegistry.counter(metricWithTag, tags).increment();
     }
   }
 
-  private void submit(String metricName, long value) {
+  private void submit(String metricName, long value, String methodName, String metricWithTag) {
     if (meterRegistry != null) {
+      Iterable<Tag> tags = getMetricsTags(methodName);
       meterRegistry.timer(metricName).record(Duration.ofMillis(value));
+      meterRegistry.timer(metricWithTag, tags).record(Duration.ofMillis(value));
     }
   }
 
+  private Tags getMetricsTags(String methodName) {
+    Tag federationTag = Tag.of("federation_namespace", getMonitorMetastore());
+    Tag methodTag = Tag.of("method_name", methodName);
+    return Tags.of(federationTag).and(methodTag);
+  }
+
+  private String getMethodName(ProceedingJoinPoint pjp) {
+    return clean(pjp.getSignature().getName());
+  }
+
+  private String getClassName(ProceedingJoinPoint pjp) {
+    return clean(pjp.getSignature().getDeclaringTypeName());
+  }
+
   private String buildMetricBasePath(ProceedingJoinPoint pjp) {
-    String className = clean(pjp.getSignature().getDeclaringTypeName());
-    String methodName = clean(pjp.getSignature().getName());
+    String className = getClassName(pjp);
+    String methodName = getMethodName(pjp);
     return new StringBuilder(className).append(".").append(methodName).toString();
   }
 
@@ -98,8 +122,7 @@ private String clean(String string) {
     return result;
   }
 
-  private String buildMetricPath(String... parts) {
+  private String buildMetricName(String... parts) {
     return DOT_JOINER.join(parts);
   }
-
 }

waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/metrics/MonitoredAspectTest.java

@@ -1,5 +1,5 @@
 /**
- * Copyright (C) 2016-2020 Expedia, Inc.
+ * Copyright (C) 2016-2023 Expedia, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,6 +20,8 @@
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.when;
 
+import java.util.Collection;
+
 import org.aspectj.lang.ProceedingJoinPoint;
 import org.aspectj.lang.Signature;
 import org.junit.Before;
@@ -28,6 +30,7 @@
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 
+import io.micrometer.core.instrument.Meter;
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.search.RequiredSearch;
 import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
@@ -109,6 +112,39 @@ public void monitorSuccesses() throws Throwable {
     assertThat(rs.timer().count(), is(1L));
   }
 
+  @Test
+  public void monitorSuccessWithTags() throws Throwable {
+    aspect.monitor(pjp, monitored);
+
+    RequiredSearch rs = meterRegistry.get("counter.Type_Anonymous.success");
+    assertThat(rs.counter().count(), is(1.0));
+
+    rs = meterRegistry.get("counter.Type_Anonymous.calls");
+    assertThat(rs.counter().count(), is(1.0));
+
+    rs = meterRegistry.get("timer.Type_Anonymous.duration");
+    assertThat(rs.timer().count(), is(1L));
+
+    // Verify the tags for successMeters
+    Collection<Meter> successMeters = meterRegistry.get("counter.Type_Anonymous.success").meters();
+    Meter successMeter = successMeters.iterator().next();
+    assertThat(successMeter.getId().getTag("federation_namespace"), is("all"));
+    assertThat(successMeter.getId().getTag("method_name"), is("myMethod"));
+
+    // Verify the tags for callsMeters
+    Collection<Meter> callsMeters = meterRegistry.get("counter.Type_Anonymous.calls").meters();
+    Meter callsMeter = callsMeters.iterator().next();
+    assertThat(callsMeter.getId().getTag("federation_namespace"), is("all"));
+    assertThat(successMeter.getId().getTag("method_name"), is("myMethod"));
+
+    // Verify the tags for durationMeters
+    Collection<Meter> durationMeters = meterRegistry.get("timer.Type_Anonymous.duration").meters();
+    Meter durationMeter = durationMeters.iterator().next();
+    assertThat(durationMeter.getId().getTag("federation_namespace"), is("all"));
+    assertThat(successMeter.getId().getTag("method_name"), is("myMethod"));
+  }
+
+
   @Test
   public void monitorFailuresForSpecificMetastore() throws Throwable {
     CurrentMonitoredMetaStoreHolder.monitorMetastore("metastoreName");

waggle-dance-integration-tests/pom.xml

@@ -4,7 +4,7 @@
   <parent>
     <groupId>com.hotels</groupId>
     <artifactId>waggle-dance-parent</artifactId>
-    <version>3.11.3-SNAPSHOT</version>
+    <version>3.11.4-SNAPSHOT</version>
   </parent>
 
   <artifactId>waggle-dance-integration-tests</artifactId>

waggle-dance-integration-tests/src/test/java/com/hotels/bdp/waggledance/WaggleDanceIntegrationTest.java

@@ -401,6 +401,10 @@ public void typicalWithGraphite() throws Exception {
         "graphitePrefix.counter.com.hotels.bdp.waggledance.server.FederatedHMSHandler.get_table_req.remote.calls;metricattribute=count 1");
     assertMetric(metrics,
         "graphitePrefix.counter.com.hotels.bdp.waggledance.server.FederatedHMSHandler.get_table_req.remote.success;metricattribute=count 1");
+    assertMetric(metrics,
+        "graphitePrefix.counter.com.hotels.bdp.waggledance.server.FederatedHMSHandler.success");
+    assertMetric(metrics,
+        "graphitePrefix.counter.com.hotels.bdp.waggledance.server.FederatedHMSHandler.calls");
   }
 
   private void assertMetric(Set<String> metrics, String partialMetric) {

waggle-dance-rest/pom.xml

@@ -4,7 +4,7 @@
   <parent>
     <groupId>com.hotels</groupId>
     <artifactId>waggle-dance-parent</artifactId>
-    <version>3.11.3-SNAPSHOT</version>
+    <version>3.11.4-SNAPSHOT</version>
   </parent>
 
   <artifactId>waggle-dance-rest</artifactId>

waggle-dance-rpm/pom.xml

@@ -4,7 +4,7 @@
   <parent>
     <groupId>com.hotels</groupId>
     <artifactId>waggle-dance-parent</artifactId>
-    <version>3.11.3-SNAPSHOT</version>
+    <version>3.11.4-SNAPSHOT</version>
   </parent>
 
   <artifactId>waggle-dance-rpm</artifactId>

waggle-dance/pom.xml

@@ -4,7 +4,7 @@
   <parent>
     <groupId>com.hotels</groupId>
     <artifactId>waggle-dance-parent</artifactId>
-    <version>3.11.3-SNAPSHOT</version>
+    <version>3.11.4-SNAPSHOT</version>
   </parent>
 
   <artifactId>waggle-dance</artifactId>