From 5c276d26656b4dcc248ef93d43cece0278402b3b Mon Sep 17 00:00:00 2001 From: Jiaqi Liu <2257440489@qq.com> Date: Mon, 4 Dec 2017 17:21:29 -0800 Subject: [PATCH 1/5] Debug BardQueryInfo to show query split counting --- .../logging/blocks/BardQueryInfo.java | 50 +++++++++---------- .../AsyncWebServiceRequestHandler.java | 2 +- .../web/handlers/CacheRequestHandler.java | 2 +- .../web/handlers/CacheV2RequestHandler.java | 2 +- .../web/handlers/EtagCacheRequestHandler.java | 2 +- .../handlers/WeightCheckRequestHandler.java | 2 +- .../logging/blocks/BardQueryInfoSpec.groovy | 28 +++++------ .../logging/blocks/BardQueryInfoUtils.groovy | 6 +-- .../AsyncWebServiceRequestHandlerSpec.groovy | 8 +-- .../handlers/CacheRequestHandlerSpec.groovy | 32 ++++++------ .../handlers/CacheV2RequestHandlerSpec.groovy | 36 ++++++------- .../WeightCheckRequestHandlerSpec.groovy | 22 ++++---- 12 files changed, 100 insertions(+), 92 deletions(-) diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/logging/blocks/BardQueryInfo.java b/fili-core/src/main/java/com/yahoo/bard/webservice/logging/blocks/BardQueryInfo.java index a875509724..9122960ffd 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/logging/blocks/BardQueryInfo.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/logging/blocks/BardQueryInfo.java @@ -27,16 +27,8 @@ public class BardQueryInfo implements LogInfo { private static final String FACT_QUERIES = "fact queries count"; private static final String FACT_QUERY_CACHE_HIT = "fact query cache hit count"; - protected static final Map QUERY_COUNTER = Stream.of( - new AbstractMap.SimpleImmutableEntry<>(WEIGHT_CHECK, new AtomicInteger()), - new AbstractMap.SimpleImmutableEntry<>(FACT_QUERIES, new AtomicInteger()), - new AbstractMap.SimpleImmutableEntry<>(FACT_QUERY_CACHE_HIT, new AtomicInteger()) - ).collect(Collectors.toMap( - AbstractMap.SimpleImmutableEntry::getKey, - AbstractMap.SimpleImmutableEntry::getValue - )); - protected final String type; + protected final Map queryCounter; /** * Constructor. @@ -45,48 +37,56 @@ public class BardQueryInfo implements LogInfo { */ public BardQueryInfo(String queryType) { this.type = queryType; + this.queryCounter = Stream.of( + new AbstractMap.SimpleImmutableEntry<>(WEIGHT_CHECK, new AtomicInteger()), + new AbstractMap.SimpleImmutableEntry<>(FACT_QUERIES, new AtomicInteger()), + new AbstractMap.SimpleImmutableEntry<>(FACT_QUERY_CACHE_HIT, new AtomicInteger()) + ).collect(Collectors.toMap( + AbstractMap.SimpleImmutableEntry::getKey, + AbstractMap.SimpleImmutableEntry::getValue + )); + } + + /** + * Retrieves {@link com.yahoo.bard.webservice.logging.blocks.BardQueryInfo} from + * {@link com.yahoo.bard.webservice.logging.RequestLog}. + * + * @return {@link com.yahoo.bard.webservice.logging.blocks.BardQueryInfo} from + * {@link com.yahoo.bard.webservice.logging.RequestLog} + */ + public static BardQueryInfo getBardQueryInfo() { + return ((BardQueryInfo) RequestLog.retrieve(BardQueryInfo.class)); } /** * Increments the number of fact queries. */ - public static void incrementCountFactHits() { + public void incrementCountFactHits() { getBardQueryInfo().incrementCountFor(BardQueryInfo.FACT_QUERIES); } /** * Increments the number of cache-hit queries. */ - public static void incrementCountCacheHits() { + public void incrementCountCacheHits() { getBardQueryInfo().incrementCountFor(BardQueryInfo.FACT_QUERY_CACHE_HIT); } /** * Increments the number of weight check queries. */ - public static void incrementCountWeightCheck() { + public void incrementCountWeightCheck() { getBardQueryInfo().incrementCountFor(BardQueryInfo.WEIGHT_CHECK); } - /** - * Retrieves {@link com.yahoo.bard.webservice.logging.blocks.BardQueryInfo} from - * {@link com.yahoo.bard.webservice.logging.RequestLog}. - * - * @return {@link com.yahoo.bard.webservice.logging.blocks.BardQueryInfo} from - * {@link com.yahoo.bard.webservice.logging.RequestLog} - */ - protected static BardQueryInfo getBardQueryInfo() { - return ((BardQueryInfo) RequestLog.retrieve(BardQueryInfo.class)); - } - /** * Increments the number of a type of query, whose possible type are all specified in * {@link com.yahoo.bard.webservice.logging.blocks.BardQueryInfo}. * * @param queryType The type of the query */ - protected static void incrementCountFor(String queryType) { - AtomicInteger count = QUERY_COUNTER.get(queryType); + protected void incrementCountFor(String queryType) { + AtomicInteger count = queryCounter.get(queryType); if (count == null) { String message = ErrorMessageFormat.RESOURCE_RETRIEVAL_FAILURE.format(queryType); LOG.error(message); diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/AsyncWebServiceRequestHandler.java b/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/AsyncWebServiceRequestHandler.java index 55616c3f7d..67d7db7a92 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/AsyncWebServiceRequestHandler.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/AsyncWebServiceRequestHandler.java @@ -52,7 +52,7 @@ public void invoke(JsonNode rootNode) { HttpErrorCallback error = response.getErrorCallback(druidQuery); FailureCallback failure = response.getFailureCallback(druidQuery); - BardQueryInfo.incrementCountFactHits(); + BardQueryInfo.getBardQueryInfo().incrementCountFactHits(); druidWebService.postDruidQuery(context, success, error, failure, druidQuery); return true; } diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/CacheRequestHandler.java b/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/CacheRequestHandler.java index 80a7d1986d..0dd0341866 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/CacheRequestHandler.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/CacheRequestHandler.java @@ -84,7 +84,7 @@ public boolean handleRequest( if (jsonResult != null) { try { if (context.getNumberOfOutgoing().decrementAndGet() == 0) { - BardQueryInfo.incrementCountCacheHits(); + BardQueryInfo.getBardQueryInfo().incrementCountCacheHits(); RequestLog.stopTiming(REQUEST_WORKFLOW_TIMER); } diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/CacheV2RequestHandler.java b/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/CacheV2RequestHandler.java index 905d1e7df5..b9c298c965 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/CacheV2RequestHandler.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/CacheV2RequestHandler.java @@ -100,7 +100,7 @@ public boolean handleRequest( ) { try { if (context.getNumberOfOutgoing().decrementAndGet() == 0) { - BardQueryInfo.incrementCountCacheHits(); + BardQueryInfo.getBardQueryInfo().incrementCountCacheHits(); RequestLog.stopTiming(REQUEST_WORKFLOW_TIMER); } diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/EtagCacheRequestHandler.java b/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/EtagCacheRequestHandler.java index ec0f323dff..cede5a1f80 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/EtagCacheRequestHandler.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/EtagCacheRequestHandler.java @@ -90,7 +90,7 @@ public boolean handleRequest( ); if (context.getNumberOfOutgoing().decrementAndGet() == 0) { - BardQueryInfo.incrementCountCacheHits(); + BardQueryInfo.getBardQueryInfo().incrementCountCacheHits(); RequestLog.stopTiming(REQUEST_WORKFLOW_TIMER); } diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/WeightCheckRequestHandler.java b/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/WeightCheckRequestHandler.java index ea9343d5b0..d0b21691c4 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/WeightCheckRequestHandler.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/web/handlers/WeightCheckRequestHandler.java @@ -72,7 +72,7 @@ public boolean handleRequest( return next.handleRequest(context, request, druidQuery, response); } - BardQueryInfo.incrementCountWeightCheck(); + BardQueryInfo.getBardQueryInfo().incrementCountWeightCheck(); final WeightCheckResponseProcessor weightCheckResponse = new WeightCheckResponseProcessor(response); final DruidAggregationQuery weightEvaluationQuery = queryWeightUtil.makeWeightEvaluationQuery(druidQuery); Granularity granularity = druidQuery.getInnermostQuery().getGranularity(); diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoSpec.groovy index 0c2f700ce3..11baf3c101 100644 --- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoSpec.groovy +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoSpec.groovy @@ -19,19 +19,19 @@ class BardQueryInfoSpec extends Specification { def "getBardQueryInfo() returns registered BardQueryInfo instance"() { expect: - BardQueryInfo.getBardQueryInfo() == bardQueryInfo + bardQueryInfo.getBardQueryInfo() == bardQueryInfo } @Unroll def "incrementCountFor(#queryType) increments count of #queryType by 1"() { expect: "count for #queryType is 0" - BardQueryInfo.QUERY_COUNTER.get(queryType).get() == 0 + bardQueryInfo.queryCounter.get(queryType).get() == 0 when: "calling incrementCountFor(#queryType)" - BardQueryInfo.incrementCountFor(queryType) + bardQueryInfo.incrementCountFor(queryType) then: "count of #queryType is incremented by 1" - BardQueryInfo.QUERY_COUNTER.get(queryType).get() == 1 + bardQueryInfo.queryCounter.get(queryType).get() == 1 where: queryType | _ @@ -42,7 +42,7 @@ class BardQueryInfoSpec extends Specification { def "incrementCountFor(String) throws IllegalArgumentException on non-existing query type"() { when: "BardQueryInfo is given an unknown query type" - BardQueryInfo.incrementCountFor("nonExistingQueryType") + bardQueryInfo.incrementCountFor("nonExistingQueryType") then: "IllegalArgumentException is thrown with exception message" IllegalArgumentException illegalArgumentException = thrown() @@ -51,18 +51,18 @@ class BardQueryInfoSpec extends Specification { def "incrementCount*() methods increment their corresponding query type counts by 1"() { expect: "all query counts are 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERIES).get() == 0 - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERIES).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "calling incrementCount*() methods for all query types" - BardQueryInfo.incrementCountWeightCheck() - BardQueryInfo.incrementCountFactHits() - BardQueryInfo.incrementCountCacheHits() + bardQueryInfo.incrementCountWeightCheck() + bardQueryInfo.incrementCountFactHits() + bardQueryInfo.incrementCountCacheHits() then: "counts of all query types are incremented by 1" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.WEIGHT_CHECK).get() == 1 - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERIES).get() == 1 - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 + bardQueryInfo.queryCounter.get(BardQueryInfo.WEIGHT_CHECK).get() == 1 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERIES).get() == 1 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 } } diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoUtils.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoUtils.groovy index e1ca6e92cd..4dac9bb226 100644 --- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoUtils.groovy +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoUtils.groovy @@ -27,8 +27,8 @@ class BardQueryInfoUtils { RequestLog.dump() // reset counts of all query types after each individual test - for (Map.Entry entry : BardQueryInfo.QUERY_COUNTER.entrySet()) { - entry.value = new AtomicInteger() - } +// for (Map.Entry entry : BardQueryInfo.queryCounter.entrySet()) { +// entry.value = new AtomicInteger() +// } } } diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/AsyncWebServiceRequestHandlerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/AsyncWebServiceRequestHandlerSpec.groovy index f0d7cfc610..18bbb793e6 100644 --- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/AsyncWebServiceRequestHandlerSpec.groovy +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/AsyncWebServiceRequestHandlerSpec.groovy @@ -21,8 +21,10 @@ import java.util.concurrent.Future class AsyncWebServiceRequestHandlerSpec extends Specification { + BardQueryInfo bardQueryInfo + def setup() { - BardQueryInfoUtils.initializeBardQueryInfo() + bardQueryInfo = BardQueryInfoUtils.initializeBardQueryInfo() } def cleanup() { @@ -47,7 +49,7 @@ class AsyncWebServiceRequestHandlerSpec extends Specification { boolean success expect: - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERIES).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERIES).get() == 0 when: success = handler.handleRequest(rc, request, groupByQuery, response) @@ -61,7 +63,7 @@ class AsyncWebServiceRequestHandlerSpec extends Specification { sc = a1 return Mock(Future) } - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERIES).get() == 1 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERIES).get() == 1 when: sc.invoke(rootNode) diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/CacheRequestHandlerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/CacheRequestHandlerSpec.groovy index 0fb4aab977..81d534864c 100644 --- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/CacheRequestHandlerSpec.groovy +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/CacheRequestHandlerSpec.groovy @@ -46,13 +46,15 @@ class CacheRequestHandlerSpec extends Specification { ObjectMapper mapper = new ObjectMappersSuite().getMapper() + BardQueryInfo bardQueryInfo + def setup() { handler = new CacheRequestHandler(next, dataCache, mapper) containerRequestContext = Mock(ContainerRequestContext) containerRequestContext.getHeaders() >> (["Bard-Testing": "###BYPASS###", "ClientId": "UI"] as MultivaluedHashMap) requestContext = new RequestContext(containerRequestContext, true) - BardQueryInfoUtils.initializeBardQueryInfo() + bardQueryInfo = BardQueryInfoUtils.initializeBardQueryInfo() } def cleanup() { @@ -67,7 +69,7 @@ class CacheRequestHandlerSpec extends Specification { def "Test handle request on cache hit responds to the group by request"() { expect: "The count of fact query cache hit is 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "A groupBy query runs with a valid cache hit" boolean requestProcessed = handler.handleRequest(requestContext, apiRequest, groupByQuery, response) @@ -82,12 +84,12 @@ class CacheRequestHandlerSpec extends Specification { requestProcessed and: "The count of fact query cache hit is incremented by 1" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 } def "Test handle request on cache hit responds to the top N request"() { expect: "The count of fact query cache hit is 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "A topN query runs with a valid cache hit" boolean requestProcessed = handler.handleRequest(requestContext, apiRequest, topNQuery, response) @@ -102,12 +104,12 @@ class CacheRequestHandlerSpec extends Specification { requestProcessed and: "The count of fact query cache hit is incremented by 1" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 } def "Test handle request on cache hit responds to the time series request"() { expect: "The count of fact query cache hit is 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "A timeseries query runs with a valid cache hit" boolean requestProcessed = handler.handleRequest(requestContext, apiRequest, timeseriesQuery, response) @@ -122,12 +124,12 @@ class CacheRequestHandlerSpec extends Specification { requestProcessed and: "The count of fact query cache hit is incremented by 1" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 } def "Test handle request cache miss delegates response to next handler"() { expect: "The count of fact query cache hit is 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "A request is sent that has a cache miss" boolean requestProcessed = handler.handleRequest(requestContext, apiRequest, groupByQuery, response) @@ -145,7 +147,7 @@ class CacheRequestHandlerSpec extends Specification { requestProcessed and: "The count of fact query cache hit is not incremented" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 } def "Test handle request cache skip delegates response to next handler"() { @@ -153,7 +155,7 @@ class CacheRequestHandlerSpec extends Specification { RequestContext requestContext = new RequestContext(containerRequestContext, false) expect: "The count of fact query cache hit is 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "A request is sent that has a cache miss" boolean requestProcessed = handler.handleRequest(requestContext, apiRequest, groupByQuery, response) @@ -171,12 +173,12 @@ class CacheRequestHandlerSpec extends Specification { requestProcessed and: "The count of fact query cache hit is not incremented" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 } def "Test handle request cache json key error delegates to next handler with cache response processor"() { expect: "The count of fact query cache hit is 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "Query that retrieves cache hit with json serialization error" boolean requestProcessed = handler.handleRequest(requestContext, apiRequest, groupByQuery, response) @@ -191,7 +193,7 @@ class CacheRequestHandlerSpec extends Specification { requestProcessed and: "The count of fact query cache hit is incremented by 1" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 } def "Test handle request key parse error delegates to next handler with cache response processor"() { @@ -202,7 +204,7 @@ class CacheRequestHandlerSpec extends Specification { handler = Spy(CacheRequestHandler, constructorArgs: [next, dataCache, mapper]) expect: "The count of fact query cache hit is 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "A request is sent with an invalid cache key" boolean requestProcessed = handler.handleRequest(requestContext, apiRequest, groupByQuery, response) @@ -217,6 +219,6 @@ class CacheRequestHandlerSpec extends Specification { requestProcessed and: "The count of fact query cache hit is not incremented" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 } } diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/CacheV2RequestHandlerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/CacheV2RequestHandlerSpec.groovy index f86f60c787..b1b1afc4f8 100644 --- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/CacheV2RequestHandlerSpec.groovy +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/CacheV2RequestHandlerSpec.groovy @@ -50,6 +50,8 @@ class CacheV2RequestHandlerSpec extends Specification { ObjectMapper mapper = new ObjectMappersSuite().getMapper() + BardQueryInfo bardQueryInfo + def setup() { querySigningService = Mock(SegmentIntervalsHashIdGenerator) querySigningService.getSegmentSetId(_) >> Optional.of(1234L) @@ -58,7 +60,7 @@ class CacheV2RequestHandlerSpec extends Specification { containerRequestContext.getHeaders() >> (["Bard-Testing": "###BYPASS###", "ClientId": "UI"] as MultivaluedHashMap) requestContext = new RequestContext(containerRequestContext, true) - BardQueryInfoUtils.initializeBardQueryInfo() + bardQueryInfo = BardQueryInfoUtils.initializeBardQueryInfo() } def cleanup() { @@ -73,7 +75,7 @@ class CacheV2RequestHandlerSpec extends Specification { def "Test handle request on cache hit responds to the group by request"() { expect: "The count of fact query cache hit is 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "A groupBy query runs with a valid cache hit" boolean requestProcessed = handler.handleRequest(requestContext, apiRequest, groupByQuery, response) @@ -88,12 +90,12 @@ class CacheV2RequestHandlerSpec extends Specification { requestProcessed and: "The count of fact query cache hit is incremented by 1" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 } def "Test handle request on cache hit responds to the top N request"() { expect: "The count of fact query cache hit is 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "A topN query runs with a valid cache hit" boolean requestProcessed = handler.handleRequest(requestContext, apiRequest, topNQuery, response) @@ -108,12 +110,12 @@ class CacheV2RequestHandlerSpec extends Specification { requestProcessed and: "The count of fact query cache hit is incremented by 1" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 } def "Test handle request on cache hit responds to the time series request"() { expect: "The count of fact query cache hit is 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "A timeseries query runs with a valid cache hit" boolean requestProcessed = handler.handleRequest(requestContext, apiRequest, timeseriesQuery, response) @@ -128,12 +130,12 @@ class CacheV2RequestHandlerSpec extends Specification { requestProcessed and: "The count of fact query cache hit is incremented by 1" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 } def "Test handle request cache miss delegates response to next handler"() { expect: "The count of fact query cache hit is 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "A request is sent that has a cache miss" boolean requestProcessed = handler.handleRequest(requestContext, apiRequest, groupByQuery, response) @@ -151,12 +153,12 @@ class CacheV2RequestHandlerSpec extends Specification { requestProcessed and: "The count of fact query cache hit is not incremented" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 } def "Test handle request cache miss due to segment invalidation delegates response to next handler"() { expect: "The count of fact query cache hit is 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "A request is sent that has a cache miss" boolean requestProcessed = handler.handleRequest(requestContext, apiRequest, groupByQuery, response) @@ -174,7 +176,7 @@ class CacheV2RequestHandlerSpec extends Specification { requestProcessed and: "The count of fact query cache hit is not incremented" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 } def "Test handle request cache skip delegates response to next handler"() { @@ -182,7 +184,7 @@ class CacheV2RequestHandlerSpec extends Specification { RequestContext requestContext = new RequestContext(containerRequestContext, false) expect: "The count of fact query cache hit is 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "A request is sent that has a cache miss" boolean requestProcessed = handler.handleRequest(requestContext, apiRequest, groupByQuery, response) @@ -200,12 +202,12 @@ class CacheV2RequestHandlerSpec extends Specification { requestProcessed and: "The count of fact query cache hit is not incremented" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 } def "Test handle request cache json key error delegates to next handler with cache response processor"() { expect: "The count of fact query cache hit is 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "Query that retrieves cache hit with json serialization error" boolean requestProcessed = handler.handleRequest(requestContext, apiRequest, groupByQuery, response) @@ -220,7 +222,7 @@ class CacheV2RequestHandlerSpec extends Specification { requestProcessed and: "The count of fact query cache hit is not incremented" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 } def "Test handle request key parse error delegates to next handler with original processor"() { @@ -231,7 +233,7 @@ class CacheV2RequestHandlerSpec extends Specification { handler = Spy(CacheV2RequestHandler, constructorArgs: [next, dataCache, querySigningService, mapper]) expect: "The count of fact query cache hit is 0" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 when: "A request is sent with an invalid cache key" boolean requestProcessed = handler.handleRequest(requestContext, apiRequest, groupByQuery, response) @@ -246,6 +248,6 @@ class CacheV2RequestHandlerSpec extends Specification { requestProcessed and: "The count of fact query cache hit is not incremented" - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 } } diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/WeightCheckRequestHandlerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/WeightCheckRequestHandlerSpec.groovy index 4a991cd876..02635524d9 100644 --- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/WeightCheckRequestHandlerSpec.groovy +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/handlers/WeightCheckRequestHandlerSpec.groovy @@ -42,6 +42,8 @@ class WeightCheckRequestHandlerSpec extends Specification { GroupByQuery groupByQuery ResponseProcessor response + BardQueryInfo bardQueryInfo + def setup() { next = Mock(DataRequestHandler) webService = Mock(DruidWebService) @@ -54,7 +56,7 @@ class WeightCheckRequestHandlerSpec extends Specification { groupByQuery = Mock(GroupByQuery) groupByQuery.getInnermostQuery() >> groupByQuery response = Mock(WeightCheckResponseProcessor) - BardQueryInfoUtils.initializeBardQueryInfo() + bardQueryInfo = BardQueryInfoUtils.initializeBardQueryInfo() } def cleanup() { @@ -92,7 +94,7 @@ class WeightCheckRequestHandlerSpec extends Specification { handler.handleRequest(context, request, groupByQuery, response) and: - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 } def "Test handleRequest without building callback"() { @@ -135,7 +137,7 @@ class WeightCheckRequestHandlerSpec extends Specification { handler.handleRequest(context, request, groupByQuery, response) and: - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.WEIGHT_CHECK).get() == 1 + bardQueryInfo.queryCounter.get(BardQueryInfo.WEIGHT_CHECK).get() == 1 } def "Test handleRequest without building callback with json error"() { @@ -176,7 +178,7 @@ class WeightCheckRequestHandlerSpec extends Specification { handler.handleRequest(context, request, groupByQuery, response) and: - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.WEIGHT_CHECK).get() == 1 + bardQueryInfo.queryCounter.get(BardQueryInfo.WEIGHT_CHECK).get() == 1 } @@ -204,7 +206,7 @@ class WeightCheckRequestHandlerSpec extends Specification { JsonNode jsonResult = MAPPER.readTree(parser) expect: - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 when: success.invoke(jsonResult) @@ -213,7 +215,7 @@ class WeightCheckRequestHandlerSpec extends Specification { 1 * next.handleRequest(context, request, groupByQuery, response) and: - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 } def "Test build and invoke success callback count too high"() { @@ -241,7 +243,7 @@ class WeightCheckRequestHandlerSpec extends Specification { HttpErrorCallback ec = Mock(HttpErrorCallback) expect: - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 when: success.invoke(jsonResult) @@ -252,7 +254,7 @@ class WeightCheckRequestHandlerSpec extends Specification { 1 * ec.dispatch(507, _, _) and: - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 } def "Test build and invoke success callback invalid json"() { @@ -280,7 +282,7 @@ class WeightCheckRequestHandlerSpec extends Specification { FailureCallback fc = Mock(FailureCallback) expect: - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 when: success.invoke(jsonResult) @@ -291,6 +293,6 @@ class WeightCheckRequestHandlerSpec extends Specification { 1 * fc.dispatch(_) and: - BardQueryInfo.QUERY_COUNTER.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 + bardQueryInfo.queryCounter.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 } } From c30b6789861c9c2d2742bf65482ce79afa9d2147 Mon Sep 17 00:00:00 2001 From: Jiaqi Liu <2257440489@qq.com> Date: Mon, 4 Dec 2017 17:38:02 -0800 Subject: [PATCH 2/5] Add change log --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9ed272554..830188aa5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -203,6 +203,11 @@ Current ### Fixed: +- [Debug BardQueryInfo to show query split counting](https://github.com/yahoo/fili/pull/596/files) + * Query counter in `BardQueryInfo` does not show up in logging because the counter used to be static and JSON + serializer does not serialize static fields. + * This patch changes the counter to an instance variable, which JSON serializer will pick up and serialize. + - [Fix intermittent class scanner error on DataSourceConstraint equal](https://github.com/yahoo/fili/pull/573) * Class Scanner Spec was injecting an improper dependant field due to type erasure. Made field type explicit. From de48b2ebb288b2929af070d50f0aa8a2a74b8dc8 Mon Sep 17 00:00:00 2001 From: Jiaqi Liu <2257440489@qq.com> Date: Tue, 5 Dec 2017 05:41:26 -0800 Subject: [PATCH 3/5] Self-review --- .../webservice/logging/blocks/BardQueryInfoUtils.groovy | 7 ------- 1 file changed, 7 deletions(-) diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoUtils.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoUtils.groovy index 4dac9bb226..6d36296fbb 100644 --- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoUtils.groovy +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoUtils.groovy @@ -4,8 +4,6 @@ package com.yahoo.bard.webservice.logging.blocks import com.yahoo.bard.webservice.logging.RequestLog -import java.util.concurrent.atomic.AtomicInteger - class BardQueryInfoUtils { /** * Constructs and returns a testing BardQueryInfo instance without a query type. @@ -25,10 +23,5 @@ class BardQueryInfoUtils { */ static void resetBardQueryInfo() { RequestLog.dump() - - // reset counts of all query types after each individual test -// for (Map.Entry entry : BardQueryInfo.queryCounter.entrySet()) { -// entry.value = new AtomicInteger() -// } } } From 84e384452379ce27086cd6a9260da07bda9d2a77 Mon Sep 17 00:00:00 2001 From: Michael McLawhorn Date: Tue, 5 Dec 2017 12:25:45 -0600 Subject: [PATCH 4/5] Simplified BardQueryInfo --- CHANGELOG.md | 2 +- .../logging/blocks/BardQueryInfo.java | 58 ++++++++----------- .../logging/blocks/BardQueryInfoSpec.groovy | 54 +++++++---------- .../logging/blocks/BardQueryInfoUtils.groovy | 2 +- 4 files changed, 49 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 830188aa5e..1447ca604b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -206,7 +206,7 @@ Current - [Debug BardQueryInfo to show query split counting](https://github.com/yahoo/fili/pull/596/files) * Query counter in `BardQueryInfo` does not show up in logging because the counter used to be static and JSON serializer does not serialize static fields. - * This patch changes the counter to an instance variable, which JSON serializer will pick up and serialize. + * This externalizes the state via a getter for serialization. - [Fix intermittent class scanner error on DataSourceConstraint equal](https://github.com/yahoo/fili/pull/573) * Class Scanner Spec was injecting an improper dependant field due to type erasure. Made field type explicit. diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/logging/blocks/BardQueryInfo.java b/fili-core/src/main/java/com/yahoo/bard/webservice/logging/blocks/BardQueryInfo.java index 9122960ffd..01555107ed 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/logging/blocks/BardQueryInfo.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/logging/blocks/BardQueryInfo.java @@ -4,7 +4,6 @@ import com.yahoo.bard.webservice.logging.LogInfo; import com.yahoo.bard.webservice.logging.RequestLog; -import com.yahoo.bard.webservice.web.ErrorMessageFormat; import com.fasterxml.jackson.annotation.JsonAutoDetect; @@ -20,15 +19,17 @@ /** * Main log of a request served by the TablesServlet. */ -@JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY) +@JsonAutoDetect(getterVisibility = JsonAutoDetect.Visibility.PUBLIC_ONLY) public class BardQueryInfo implements LogInfo { private static final Logger LOG = LoggerFactory.getLogger(BardQueryInfo.class); - private static final String WEIGHT_CHECK = "weight check queries count"; - private static final String FACT_QUERIES = "fact queries count"; - private static final String FACT_QUERY_CACHE_HIT = "fact query cache hit count"; + public static final String WEIGHT_CHECK = "weightCheckQueries"; + public static final String FACT_QUERIES = "factQueryCount"; + public static final String FACT_QUERY_CACHE_HIT = "factCacheHits"; - protected final String type; - protected final Map queryCounter; + private final String type; + private final AtomicInteger weightCheckCount = new AtomicInteger(); + private final AtomicInteger factQueryCount = new AtomicInteger(); + private final AtomicInteger factCacheHitCount = new AtomicInteger(); /** * Constructor. @@ -37,10 +38,17 @@ public class BardQueryInfo implements LogInfo { */ public BardQueryInfo(String queryType) { this.type = queryType; - this.queryCounter = Stream.of( - new AbstractMap.SimpleImmutableEntry<>(WEIGHT_CHECK, new AtomicInteger()), - new AbstractMap.SimpleImmutableEntry<>(FACT_QUERIES, new AtomicInteger()), - new AbstractMap.SimpleImmutableEntry<>(FACT_QUERY_CACHE_HIT, new AtomicInteger()) + } + + public String getType() { + return type; + } + + public Map getQueryCounter() { + return Stream.of( + new AbstractMap.SimpleImmutableEntry<>(WEIGHT_CHECK, weightCheckCount), + new AbstractMap.SimpleImmutableEntry<>(FACT_QUERIES, factQueryCount), + new AbstractMap.SimpleImmutableEntry<>(FACT_QUERY_CACHE_HIT, factCacheHitCount) ).collect(Collectors.toMap( AbstractMap.SimpleImmutableEntry::getKey, AbstractMap.SimpleImmutableEntry::getValue @@ -61,37 +69,21 @@ public static BardQueryInfo getBardQueryInfo() { /** * Increments the number of fact queries. */ - public void incrementCountFactHits() { - getBardQueryInfo().incrementCountFor(BardQueryInfo.FACT_QUERIES); + public static void incrementCountFactHits() { + getBardQueryInfo().factQueryCount.incrementAndGet(); } /** * Increments the number of cache-hit queries. */ - public void incrementCountCacheHits() { - getBardQueryInfo().incrementCountFor(BardQueryInfo.FACT_QUERY_CACHE_HIT); + public static void incrementCountCacheHits() { + getBardQueryInfo().factCacheHitCount.incrementAndGet(); } /** * Increments the number of weight check queries. */ - public void incrementCountWeightCheck() { - getBardQueryInfo().incrementCountFor(BardQueryInfo.WEIGHT_CHECK); - } - - /** - * Increments the number of a type of query, whose possible type are all specified in - * {@link com.yahoo.bard.webservice.logging.blocks.BardQueryInfo}. - * - * @param queryType The type of the query - */ - protected void incrementCountFor(String queryType) { - AtomicInteger count = queryCounter.get(queryType); - if (count == null) { - String message = ErrorMessageFormat.RESOURCE_RETRIEVAL_FAILURE.format(queryType); - LOG.error(message); - throw new IllegalArgumentException(message); - } - count.incrementAndGet(); + public static void incrementCountWeightCheck() { + getBardQueryInfo().weightCheckCount.incrementAndGet(); } } diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoSpec.groovy index 11baf3c101..62203b7be4 100644 --- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoSpec.groovy +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoSpec.groovy @@ -2,15 +2,21 @@ // Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms. package com.yahoo.bard.webservice.logging.blocks -import com.yahoo.bard.webservice.web.ErrorMessageFormat +import com.yahoo.bard.webservice.application.ObjectMappersSuite +import com.yahoo.bard.webservice.logging.RequestLog + import spock.lang.Specification import spock.lang.Unroll +import java.util.concurrent.atomic.AtomicInteger + class BardQueryInfoSpec extends Specification { BardQueryInfo bardQueryInfo def setup() { bardQueryInfo = BardQueryInfoUtils.initializeBardQueryInfo() + RequestLog.getId() // initialize RequestLog + RequestLog.record(bardQueryInfo) } def cleanup() { @@ -24,45 +30,29 @@ class BardQueryInfoSpec extends Specification { @Unroll def "incrementCountFor(#queryType) increments count of #queryType by 1"() { + setup: + AtomicInteger counter = BardQueryInfo.bardQueryInfo.queryCounter.get(queryType); + expect: "count for #queryType is 0" - bardQueryInfo.queryCounter.get(queryType).get() == 0 + counter.get() == 0 when: "calling incrementCountFor(#queryType)" - bardQueryInfo.incrementCountFor(queryType) + incrementor() then: "count of #queryType is incremented by 1" - bardQueryInfo.queryCounter.get(queryType).get() == 1 + counter.get() == 1 where: - queryType | _ - BardQueryInfo.WEIGHT_CHECK | _ - BardQueryInfo.FACT_QUERIES | _ - BardQueryInfo.FACT_QUERY_CACHE_HIT | _ + queryType | incrementor + BardQueryInfo.WEIGHT_CHECK | BardQueryInfo.&incrementCountWeightCheck + BardQueryInfo.FACT_QUERIES | BardQueryInfo.&incrementCountFactHits + BardQueryInfo.FACT_QUERY_CACHE_HIT | BardQueryInfo.&incrementCountCacheHits } - def "incrementCountFor(String) throws IllegalArgumentException on non-existing query type"() { - when: "BardQueryInfo is given an unknown query type" - bardQueryInfo.incrementCountFor("nonExistingQueryType") - - then: "IllegalArgumentException is thrown with exception message" - IllegalArgumentException illegalArgumentException = thrown() - illegalArgumentException.message == ErrorMessageFormat.RESOURCE_RETRIEVAL_FAILURE.format("nonExistingQueryType") - } - - def "incrementCount*() methods increment their corresponding query type counts by 1"() { - expect: "all query counts are 0" - bardQueryInfo.queryCounter.get(BardQueryInfo.WEIGHT_CHECK).get() == 0 - bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERIES).get() == 0 - bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 0 - - when: "calling incrementCount*() methods for all query types" - bardQueryInfo.incrementCountWeightCheck() - bardQueryInfo.incrementCountFactHits() - bardQueryInfo.incrementCountCacheHits() - - then: "counts of all query types are incremented by 1" - bardQueryInfo.queryCounter.get(BardQueryInfo.WEIGHT_CHECK).get() == 1 - bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERIES).get() == 1 - bardQueryInfo.queryCounter.get(BardQueryInfo.FACT_QUERY_CACHE_HIT).get() == 1 + def "Object serializes with type and map"() { + expect: + new ObjectMappersSuite().jsonMapper.writeValueAsString( + bardQueryInfo + ) == """{"type":"test","queryCounter":{"factQueryCount":0,"weightCheckQueries":0,"factCacheHits":0}}"""; } } diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoUtils.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoUtils.groovy index 6d36296fbb..33c3ea5355 100644 --- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoUtils.groovy +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoUtils.groovy @@ -12,7 +12,7 @@ class BardQueryInfoUtils { */ static BardQueryInfo initializeBardQueryInfo() { resetBardQueryInfo() - BardQueryInfo bardQueryInfo = new BardQueryInfo(null) + BardQueryInfo bardQueryInfo = new BardQueryInfo("test") RequestLog.getId() // initialize RequestLog RequestLog.record(bardQueryInfo) return bardQueryInfo From 7e6f2151d359bfea1f6341d79d9d626fed82b64c Mon Sep 17 00:00:00 2001 From: Michael McLawhorn Date: Tue, 5 Dec 2017 17:20:23 -0600 Subject: [PATCH 5/5] cleanup test code. --- .../webservice/logging/blocks/BardQueryInfoSpec.groovy | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoSpec.groovy index 62203b7be4..dfef9725bf 100644 --- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoSpec.groovy +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/logging/blocks/BardQueryInfoSpec.groovy @@ -3,7 +3,6 @@ package com.yahoo.bard.webservice.logging.blocks import com.yahoo.bard.webservice.application.ObjectMappersSuite -import com.yahoo.bard.webservice.logging.RequestLog import spock.lang.Specification import spock.lang.Unroll @@ -15,8 +14,6 @@ class BardQueryInfoSpec extends Specification { def setup() { bardQueryInfo = BardQueryInfoUtils.initializeBardQueryInfo() - RequestLog.getId() // initialize RequestLog - RequestLog.record(bardQueryInfo) } def cleanup() { @@ -25,11 +22,11 @@ class BardQueryInfoSpec extends Specification { def "getBardQueryInfo() returns registered BardQueryInfo instance"() { expect: - bardQueryInfo.getBardQueryInfo() == bardQueryInfo + BardQueryInfo.getBardQueryInfo() == bardQueryInfo } @Unroll - def "incrementCountFor(#queryType) increments count of #queryType by 1"() { + def "increment Count For #queryType increments counter by 1"() { setup: AtomicInteger counter = BardQueryInfo.bardQueryInfo.queryCounter.get(queryType);