Cleaned up id prefix.

yahoo · Oct 14, 2016 · efae917 · efae917
1 parent 4374c78
commit efae917
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 34 deletions.
diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/logging/RequestLog.java b/fili-core/src/main/java/com/yahoo/bard/webservice/logging/RequestLog.java
@@ -60,7 +60,6 @@ public class RequestLog {
     private static final List<String> LOGINFO_ORDER = generateLogInfoOrder(LOGINFO_ORDER_STRING);
 
     private String logId;
-    private String idPrefix;
     private LogBlock info;
     private TimedPhase mostRecentTimer;
     private final Map<String, TimedPhase> times;
@@ -79,7 +78,6 @@ private RequestLog() {
         mapper.registerModule((new JodaModule()).addSerializer(Interval.class, new ToStringSerializer()));
         mapper.registerModule(new Jdk8Module().configureAbsentsAsNulls(false));
         MDC.remove(ID_KEY);
-        idPrefix = "";
     }
 
     /**
@@ -94,7 +92,6 @@ private RequestLog(RequestLog rl) {
         times = new LinkedHashMap<>(rl.times);
         threadIds = new LinkedHashSet<>(rl.threadIds);
         MDC.put(ID_KEY, logId);
-        idPrefix = "";
     }
 
     /**
@@ -155,7 +152,6 @@ private void clear() {
         times.clear();
         threadIds.clear();
         MDC.remove(ID_KEY);
-        idPrefix = "";
     }
 
     /**
@@ -185,7 +181,6 @@ private void init() {
         threadIds.clear();
         threadIds.add(Thread.currentThread().getName());
         MDC.put(ID_KEY, logId);
-        idPrefix = "";
     }
 
     /**
@@ -402,20 +397,19 @@ public static String getId() {
         if (current.info == null) {
             current.init();
         }
-        return current.idPrefix + current.logId;
+        return current.logId;
     }
 
     /**
      * Set id prefix.
      *
      * @param idPrefix  Prefix for queryId sent to druid
      */
-    public static void setIdPrefix(String idPrefix) {
+    public static void addIdPrefix(String idPrefix) {
         RequestLog current = RLOG.get();
-        if (current.info == null) {
-            current.init();
-        }
-        current.idPrefix = (idPrefix == null) ? "" : idPrefix;
+        String newId = idPrefix + getId();
+        current.logId = newId;
+        MDC.put(ID_KEY, newId);
     }
 
     /**

diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/web/filters/BardLoggingFilter.java b/fili-core/src/main/java/com/yahoo/bard/webservice/web/filters/BardLoggingFilter.java
@@ -84,7 +84,7 @@ public class BardLoggingFilter implements ContainerRequestFilter, ContainerRespo
     @Override
     public void filter(ContainerRequestContext request) throws IOException {
 
-        RequestLog.setIdPrefix(request.getHeaders().getFirst(X_REQUEST_ID_HEADER));
+        RequestLog.addIdPrefix(request.getHeaders().getFirst(X_REQUEST_ID_HEADER));
         RequestLog.startTiming(TOTAL_TIMER);
         RequestLog.startTiming(this);
         RequestLog.record(new Preface(request));
@@ -110,7 +110,7 @@ public void filter(ContainerRequestContext request) throws IOException {
     @Override
     public void filter(ContainerRequestContext request, ContainerResponseContext response)
         throws IOException {
-        RequestLog.setIdPrefix(request.getHeaders().getFirst(X_REQUEST_ID_HEADER));
+        RequestLog.addIdPrefix(request.getHeaders().getFirst(X_REQUEST_ID_HEADER));
         RequestLog.startTiming(this);
         StringBuilder debugMsgBuilder = new StringBuilder();
 
@@ -171,7 +171,7 @@ public void filter(ContainerRequestContext request, ContainerResponseContext res
      */
     @Override
     public void filter(ClientRequestContext request) throws IOException {
-        RequestLog.setIdPrefix(request.getStringHeaders().getFirst(X_REQUEST_ID_HEADER));
+        RequestLog.addIdPrefix(request.getStringHeaders().getFirst(X_REQUEST_ID_HEADER));
         RequestLog.startTiming(CLIENT_TOTAL_TIMER);
         request.setProperty(PROPERTY_NANOS, System.nanoTime());
     }

diff --git a/...c/test/groovy/com/yahoo/bard/webservice/web/endpoints/BaseDataServletComponentSpec.groovy b/...c/test/groovy/com/yahoo/bard/webservice/web/endpoints/BaseDataServletComponentSpec.groovy
@@ -21,6 +21,7 @@ import org.slf4j.LoggerFactory
 import spock.lang.Specification
 import spock.lang.Timeout
 
+import javax.ws.rs.core.MultivaluedHashMap
 import javax.ws.rs.core.MultivaluedMap
 import javax.ws.rs.core.Response
 
@@ -71,6 +72,10 @@ abstract class BaseDataServletComponentSpec extends Specification {
         populatePhysicalTableAvailability()
     }
 
+    MultivaluedHashMap<String, String> getAdditionalHeaders() {
+        return [:]
+    }
+
     /**
      * Populates the interval availability of the physical tables.
      * <p>
@@ -206,7 +211,7 @@ abstract class BaseDataServletComponentSpec extends Specification {
         }
 
         // Make the call
-        Response response = httpCall.request().get()
+        Response response = httpCall.request().headers(getAdditionalHeaders()).get()
         if (response.status != 200) {
             LOG.error( "***  *** Response status: ${response.status}: ${response.readEntity(String)}")
         }

diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/endpoints/DruidQueryIdSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/endpoints/DruidQueryIdSpec.groovy
@@ -8,7 +8,7 @@ import com.yahoo.bard.webservice.util.JsonSlurper
 import com.yahoo.bard.webservice.util.JsonSortStrategy
 import com.yahoo.bard.webservice.web.filters.BardLoggingFilter
 
-import javax.ws.rs.core.Response
+import javax.ws.rs.core.MultivaluedHashMap
 
 
 class DruidQueryIdSpec extends BaseDataServletComponentSpec {
@@ -27,8 +27,7 @@ class DruidQueryIdSpec extends BaseDataServletComponentSpec {
 
     @Override
     Class<?>[] getResourceClasses() {
-        [ DataServlet.class
-        , BardLoggingFilter.class ]
+        [ DataServlet.class, BardLoggingFilter.class ]
     }
 
     @Override
@@ -40,7 +39,7 @@ class DruidQueryIdSpec extends BaseDataServletComponentSpec {
     Map<String, List<String>> getQueryParams() {
         [
                 "metrics": ["width","depth"],
-                "dateTime": ["2014-06-02%2F2014-06-30"],
+                "dateTime": ["2014-06-02/2014-06-30"],
                 "sort": ["width|desc","depth|asc"]
         ]
     }
@@ -76,20 +75,7 @@ class DruidQueryIdSpec extends BaseDataServletComponentSpec {
     }
 
     @Override
-    Response makeAbstractRequest(Closure queryParams=this.&getQueryParams) {
-        // Set target of call
-        def httpCall = jtb.getHarness().target(getTarget())
-
-        // Add query params to call
-        queryParams().each { String key, List<String> values ->
-            httpCall = httpCall.queryParam(key, values.join(","))
-        }
-
-        // Make the call
-        Response response = httpCall.request().header("X-Request-ID", prefixId).get()
-        if (response.status != 200) {
-            LOG.error( "***  *** Response status: ${response.status}: ${response.readEntity(String)}")
-        }
-        response
+    MultivaluedHashMap<String, String> getAdditionalHeaders() {
+        return ["x-request-id": prefixId]
     }
 }