Merge 12b8778 into cb0f7eb

ExpediaDotCom · Dec 28, 2018 · 9c47f85 · 9c47f85
2 parents cb0f7eb + 12b8778
commit 9c47f85
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 30 deletions.
diff --git a/pom.xml b/pom.xml
@@ -12,7 +12,7 @@
 
     <groupId>com.expedia.www</groupId>
     <artifactId>haystack-logback-metrics-appender</artifactId>
-    <version>1.0.5</version>
+    <version>1.0.6</version>
     <packaging>jar</packaging>
 
     <scm>

diff --git a/...ava/com/expedia/www/haystack/metrics/appenders/logback/EmitToGraphiteLogbackAppender.java b/...ava/com/expedia/www/haystack/metrics/appenders/logback/EmitToGraphiteLogbackAppender.java
@@ -1,5 +1,5 @@
 /*
- *  Copyright 2017 Expedia, Inc.
+ *  Copyright 2018 Expedia, Inc.
  *
  *        Licensed under the Apache License, Version 2.0 (the "License");
  *        you may not use this file except in compliance with the License.
@@ -145,14 +145,23 @@ boolean isLevelSevereEnoughToCount(Level level) {
         return level == ERROR;
     }
 
+    // From https://github.com/ExpediaDotCom/haystack-logback-metrics-appender/issues/28
+    // Scaling issues with InfluxDb have led us to change the way that Graphite messages are changed into tagged metrics
+    // in InfluxDb; in particular, the InfluxDb template for the error metrics was changed from
+    // "haystack.errors.* system.metricGroup.subsystem.fqClass.host.lineNumber.measurement*" to
+    // "haystack.errors.* measurement.measurement.measurement.fqClass.host.field*". As a result, the presence of line
+    // number in the metric needs to be removed. In the interest of simplicity, I will comment out the code that inserts
+    // line number into the Graphite metric, to facilitate a potential setting-based change in the future to allow this
+    // package to create both types of Graphite metrics.
     @VisibleForTesting
     Counter getCounter(Level level, StackTraceElement stackTraceElement) {
         final String fullyQualifiedClassName = changePeriodsToDashes(stackTraceElement.getClassName());
-        final String lineNumber = Integer.toString(stackTraceElement.getLineNumber());
-        final String key = fullyQualifiedClassName + ':' + lineNumber;
+        //final String lineNumber = Integer.toString(stackTraceElement.getLineNumber());
+        @SuppressWarnings("UnnecessaryLocalVariable")
+        final String key = fullyQualifiedClassName/* + ':' + lineNumber*/;
         if (!ERRORS_COUNTERS.containsKey(key)) {
             final Counter counter = factory.createCounter(
-                    metricObjects, subsystem, fullyQualifiedClassName, lineNumber, level.toString());
+                    metricObjects, subsystem, fullyQualifiedClassName, /*lineNumber, */level.toString());
 
             // It is possible but highly unlikely that two threads are in this if() block at the same time; if that
             // occurs, only one of the calls to ERRORS_COUNTERS.putIfAbsent(hashCode, counter) in the next line of code
@@ -170,9 +179,9 @@ static String changePeriodsToDashes(String fullyQualifiedClassName) {
     @VisibleForTesting
     static class Factory {
         Counter createCounter(MetricObjects metricObjects, String subsystem, String fullyQualifiedClassName,
-                              String lineNumber, String counterName) {
+                              /*String lineNumber, */String counterName) {
             return metricObjects.createAndRegisterResettingCounter(
-                    ERRORS_METRIC_GROUP, subsystem, fullyQualifiedClassName, lineNumber, counterName);
+                    ERRORS_METRIC_GROUP, subsystem, fullyQualifiedClassName, /*lineNumber, */counterName);
         }
 
         StartUpMetric createStartUpMetric(MetricObjects metricObjects, String subsystem, Timer timer) {

diff --git a/src/main/java/com/expedia/www/haystack/metrics/appenders/logback/StartUpMetric.java b/src/main/java/com/expedia/www/haystack/metrics/appenders/logback/StartUpMetric.java
@@ -39,8 +39,8 @@ class StartUpMetric {
 
     StartUpMetric(Timer timer, Factory factory, MetricObjects metricObjects, String subsystem) {
         this.timer = timer;
-        this.counter = factory.createCounter(metricObjects, subsystem,
-                LINE_NUMBER_OF_EMIT_METHOD_IN_START_UP_METRIC_CLASS);
+        this.counter = factory.createCounter(metricObjects, subsystem/*,
+                LINE_NUMBER_OF_EMIT_METHOD_IN_START_UP_METRIC_CLASS*/);
     }
 
     void start() {
@@ -58,17 +58,17 @@ void stop() {
         timer.cancel();
     }
 
-    private static final String LINE_NUMBER_OF_EMIT_METHOD_IN_START_UP_METRIC_CLASS = Integer.toString(
-            new Throwable().getStackTrace()[0].getLineNumber() + 2);
+//    private static final String LINE_NUMBER_OF_EMIT_METHOD_IN_START_UP_METRIC_CLASS = Integer.toString(
+//            new Throwable().getStackTrace()[0].getLineNumber() + 2);
     private void emit() {
         counter.increment(METRIC_VALUE);
     }
 
     @VisibleForTesting
     static class Factory {
-        Counter createCounter(MetricObjects metricObjects, String subsystem, String lineNumber) {
+        Counter createCounter(MetricObjects metricObjects, String subsystem/*, String lineNumber*/) {
             return metricObjects.createAndRegisterResettingCounter(
-                    ERRORS_METRIC_GROUP, subsystem, FULLY_QUALIFIED_CLASS_NAME, lineNumber, ERROR.toString());
+                    ERRORS_METRIC_GROUP, subsystem, FULLY_QUALIFIED_CLASS_NAME, /*lineNumber, */ERROR.toString());
         }
     }
 }
diff --git a/...com/expedia/www/haystack/metrics/appenders/logback/EmitToGraphiteLogbackAppenderTest.java b/...com/expedia/www/haystack/metrics/appenders/logback/EmitToGraphiteLogbackAppenderTest.java
@@ -43,7 +43,7 @@
 import static ch.qos.logback.classic.Level.WARN;
 import static com.expedia.www.haystack.metrics.appenders.logback.EmitToGraphiteLogbackAppender.ERRORS_COUNTERS;
 import static com.expedia.www.haystack.metrics.appenders.logback.EmitToGraphiteLogbackAppender.ERRORS_METRIC_GROUP;
-import static com.expedia.www.haystack.metrics.appenders.logback.StartUpMetricTest.LINE_NUMBER_OF_EMIT_METHOD_IN_START_UP_METRIC_CLASS;
+//import static com.expedia.www.haystack.metrics.appenders.logback.StartUpMetricTest.LINE_NUMBER_OF_EMIT_METHOD_IN_START_UP_METRIC_CLASS;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -69,7 +69,7 @@ public class EmitToGraphiteLogbackAppenderTest {
     private static final int POLL_INTERVAL_SECONDS = RANDOM.nextInt(Byte.MAX_VALUE);
     private static final int QUEUE_SIZE = RANDOM.nextInt(Byte.MAX_VALUE);
     private static final int LINE_NUMBER = RANDOM.nextInt(Integer.MAX_VALUE);
-    private static final String S_LINE_NUMBER = Integer.toString(LINE_NUMBER);
+//    private static final String S_LINE_NUMBER = Integer.toString(LINE_NUMBER);
     private static final boolean SEND_AS_RATE = RANDOM.nextBoolean();
     private static final Class<StartUpMetric> START_UP_METRIC_CLASS = StartUpMetric.class;
     private static final String START_UP_METRIC_FULLY_QUALIFIED_CLASS_NAME = START_UP_METRIC_CLASS.getName().replace('.', '-');
@@ -132,14 +132,14 @@ public void testDefaultConstructor() {
     @Test
     public void testFactoryCreateCounter() {
         when(mockMetricObjects.createAndRegisterResettingCounter(
-                anyString(), anyString(), anyString(), anyString(), anyString())).thenReturn(mockCounter);
+                anyString(), anyString(), anyString(), /*anyString(), */anyString())).thenReturn(mockCounter);
 
         final Counter counter = factory.createCounter(
-                mockMetricObjects, SUBSYSTEM, START_UP_METRIC_FULLY_QUALIFIED_CLASS_NAME, S_LINE_NUMBER, COUNTER_NAME);
+                mockMetricObjects, SUBSYSTEM, START_UP_METRIC_FULLY_QUALIFIED_CLASS_NAME, /*S_LINE_NUMBER, */COUNTER_NAME);
 
         assertSame(mockCounter, counter);
         verify(mockMetricObjects).createAndRegisterResettingCounter(ERRORS_METRIC_GROUP,
-                SUBSYSTEM, START_UP_METRIC_FULLY_QUALIFIED_CLASS_NAME, S_LINE_NUMBER, COUNTER_NAME);
+                SUBSYSTEM, START_UP_METRIC_FULLY_QUALIFIED_CLASS_NAME, /*S_LINE_NUMBER, */COUNTER_NAME);
     }
 
     @Test
@@ -152,7 +152,7 @@ public void testFactoryCreateStartUpMetric() {
         assertNotNull(startUpMetric);
         verify(mockMetricObjects).createAndRegisterResettingCounter(ERRORS_METRIC_GROUP,
                 SUBSYSTEM, START_UP_METRIC_FULLY_QUALIFIED_CLASS_NAME,
-                LINE_NUMBER_OF_EMIT_METHOD_IN_START_UP_METRIC_CLASS, COUNTER_NAME);
+                /*LINE_NUMBER_OF_EMIT_METHOD_IN_START_UP_METRIC_CLASS, */COUNTER_NAME);
     }
 
     @Test
@@ -179,16 +179,16 @@ public void testAppendLevelIsSevereEnoughToCount() {
         when(mockLoggingEvent.getLevel()).thenReturn(ERROR);
         final StackTraceElement[] stackTraceElements = new Exception().getStackTrace();
         when(mockLoggingEvent.getCallerData()).thenReturn(stackTraceElements);
-        when(mockFactory.createCounter(any(MetricObjects.class), anyString(), anyString(), anyString(), anyString()))
+        when(mockFactory.createCounter(any(MetricObjects.class), anyString(), anyString(), /*anyString(), */anyString()))
                 .thenReturn(mockCounter);
 
         emitToGraphiteLogbackAppender.append(mockLoggingEvent);
 
         verify(mockLoggingEvent).getLevel();
         verify(mockLoggingEvent).getCallerData();
-        final String lineNumber = Integer.toString(stackTraceElements[0].getLineNumber());
+        //final String lineNumber = Integer.toString(stackTraceElements[0].getLineNumber());
         verify(mockFactory).createCounter(
-                mockMetricObjects, SUBSYSTEM, TEST_CLASS_NAME, lineNumber, COUNTER_NAME);
+                mockMetricObjects, SUBSYSTEM, TEST_CLASS_NAME, /*lineNumber, */COUNTER_NAME);
         verify(mockCounter).increment();
     }
 
@@ -246,7 +246,7 @@ public void testStopStartUpMetricIsNotNull() {
     }
 
     private void commonWhensForStart() {
-        when(mockFactory.createCounter(any(MetricObjects.class), anyString(), anyString(), anyString(), anyString()))
+        when(mockFactory.createCounter(any(MetricObjects.class), anyString(), anyString(), /*anyString(), */anyString()))
                 .thenReturn(mockCounter);
         when(mockFactory.createStartUpMetric(any(MetricObjects.class), anyString(), any(Timer.class)))
                 .thenReturn(mockStartUpMetric);
@@ -270,14 +270,14 @@ public void testStopStartUpMetricIsNull() {
     public void testGetCounter() {
         final StackTraceElement stackTraceElement = new StackTraceElement(
                 START_UP_METRIC_CLASS.getName(), METHOD_NAME, FILE_NAME, LINE_NUMBER);
-        when(mockFactory.createCounter(any(MetricObjects.class), anyString(), anyString(), anyString(), anyString()))
+        when(mockFactory.createCounter(any(MetricObjects.class), anyString(), anyString(), /*anyString(), */anyString()))
                 .thenReturn(mockCounter);
 
         final Counter counter1 = emitToGraphiteLogbackAppender.getCounter(Level.ERROR, stackTraceElement);
         final Counter counter2 = emitToGraphiteLogbackAppender.getCounter(Level.ERROR, stackTraceElement);
 
         assertSame(counter1, counter2);
         verify(mockFactory).createCounter(mockMetricObjects, SUBSYSTEM, START_UP_METRIC_FULLY_QUALIFIED_CLASS_NAME,
-                Integer.toString(LINE_NUMBER), Level.ERROR.toString());
+                /*Integer.toString(LINE_NUMBER), */Level.ERROR.toString());
     }
  }
diff --git a/src/test/java/com/expedia/www/haystack/metrics/appenders/logback/StartUpMetricTest.java b/src/test/java/com/expedia/www/haystack/metrics/appenders/logback/StartUpMetricTest.java
@@ -47,7 +47,7 @@ public class StartUpMetricTest {
     private static final String SUBSYSTEM = RANDOM.nextLong() + "SUBSYSTEM";
     private static final String FULLY_QUALIFIED_CLASS_NAME = EmitToGraphiteLogbackAppender.changePeriodsToDashes(
             StartUpMetric.class.getName());
-    private static final String LINE_NUMBER = Integer.toString(RANDOM.nextInt(Integer.MAX_VALUE));
+    //private static final String LINE_NUMBER = Integer.toString(RANDOM.nextInt(Integer.MAX_VALUE));
 
     @Mock
     private Factory mockFactory;
@@ -66,15 +66,15 @@ public class StartUpMetricTest {
 
     @Before
     public void setUp() {
-        when(mockFactory.createCounter(any(MetricObjects.class), anyString(), anyString())).thenReturn(mockCounter);
+        when(mockFactory.createCounter(any(MetricObjects.class), anyString()/*, anyString()*/)).thenReturn(mockCounter);
         startUpMetric = new StartUpMetric(mockTimer, mockFactory, mockMetricObjects, SUBSYSTEM);
         factory = new Factory();
     }
 
     @After
     public void tearDown() {
         verify(mockFactory).createCounter(
-                mockMetricObjects, SUBSYSTEM, LINE_NUMBER_OF_EMIT_METHOD_IN_START_UP_METRIC_CLASS);
+                mockMetricObjects, SUBSYSTEM/*, LINE_NUMBER_OF_EMIT_METHOD_IN_START_UP_METRIC_CLASS*/);
         verifyNoMoreInteractions(mockFactory, mockCounter, mockTimer, mockMetricObjects);
     }
 
@@ -102,9 +102,9 @@ public void testStop() {
 
     @Test
     public void testFactoryCreateCounter() {
-        factory.createCounter(mockMetricObjects, SUBSYSTEM, LINE_NUMBER);
+        factory.createCounter(mockMetricObjects, SUBSYSTEM/*, LINE_NUMBER*/);
 
         verify(mockMetricObjects).createAndRegisterResettingCounter(
-                ERRORS_METRIC_GROUP, SUBSYSTEM, FULLY_QUALIFIED_CLASS_NAME, LINE_NUMBER, ERROR.toString());
+                ERRORS_METRIC_GROUP, SUBSYSTEM, FULLY_QUALIFIED_CLASS_NAME, /*LINE_NUMBER, */ERROR.toString());
     }
 }