From efae917e89c4d0612eba2c1756717caf7ee19d9f Mon Sep 17 00:00:00 2001 From: "Dennis J. McWherter Jr" Date: Fri, 14 Oct 2016 15:21:42 -0500 Subject: [PATCH] Cleaned up id prefix. --- .../bard/webservice/logging/RequestLog.java | 16 ++++--------- .../web/filters/BardLoggingFilter.java | 6 ++--- .../BaseDataServletComponentSpec.groovy | 7 +++++- .../web/endpoints/DruidQueryIdSpec.groovy | 24 ++++--------------- 4 files changed, 19 insertions(+), 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 index e3575faec1..093b448af4 100644 --- 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 LOGINFO_ORDER = generateLogInfoOrder(LOGINFO_ORDER_STRING); private String logId; - private String idPrefix; private LogBlock info; private TimedPhase mostRecentTimer; private final Map 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,7 +397,7 @@ public static String getId() { if (current.info == null) { current.init(); } - return current.idPrefix + current.logId; + return current.logId; } /** @@ -410,12 +405,11 @@ public static String getId() { * * @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 index ad776a6e24..c292481247 100644 --- 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/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/endpoints/BaseDataServletComponentSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/endpoints/BaseDataServletComponentSpec.groovy index aa4a7ef8b7..5c4e3506a4 100644 --- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/endpoints/BaseDataServletComponentSpec.groovy +++ b/fili-core/src/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 getAdditionalHeaders() { + return [:] + } + /** * Populates the interval availability of the physical tables. *

@@ -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 index 4d9c449be2..bbb182dbc2 100644 --- 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> 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 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 getAdditionalHeaders() { + return ["x-request-id": prefixId] } }