From ea1953166ebe9ff02d31223c8cf8abfc6cbaa1b6 Mon Sep 17 00:00:00 2001 From: Andrew Cholewa Date: Fri, 6 Apr 2018 13:57:22 -0500 Subject: [PATCH] Adds a factory for building `ResponseProcessor`. (#663) -- We would like to do a little bit of Druid error analysis to extract some more illuminating error codes out of some of Druid's 500's. The error callback is controlled by the `ResponseProcessor`. -- Unfortunately, the `ResponseProcessor` can't be injected directly because it depends on the `DataApiRequest`, which is built directly in each request. So instead we wrap the creation of `ResponseProcessor` in a factory that can be injected. --- CHANGELOG.md | 4 ++ .../application/AbstractBinderFactory.java | 25 +++++++++++-- .../webservice/web/endpoints/DataServlet.java | 14 +++++-- .../ResponseProcessorFactory.java | 37 +++++++++++++++++++ .../ResultSetResponseProcessorFactory.java | 34 +++++++++++++++++ 5 files changed, 106 insertions(+), 8 deletions(-) create mode 100644 fili-core/src/main/java/com/yahoo/bard/webservice/web/responseprocessors/ResponseProcessorFactory.java create mode 100644 fili-core/src/main/java/com/yahoo/bard/webservice/web/responseprocessors/ResultSetResponseProcessorFactory.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c4bdf70b4..51d9655176 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -131,6 +131,10 @@ Current ### Changed: +- [ResponseProcessor is now injectable.](https://github.com/yahoo/fili/pull/663) + * To add a custom `ResponseProcessor`, implement `ResponseProcessorFactory`, override + `AbstractBinderFactory::buildResponseProcessorFactory` to return your custom `ResponseProcessorFactory.class`. + - [Add config to ignore partial/volatile intervals and cache everything in cache V2](https://github.com/yahoo/fili/pull/645) * In cache V2, user should be able to decide whether partial data or volatile data should be cached or not. This PR adds a config that allows the user to do this. diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/application/AbstractBinderFactory.java b/fili-core/src/main/java/com/yahoo/bard/webservice/application/AbstractBinderFactory.java index c109b97064..c231c32a96 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/application/AbstractBinderFactory.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/application/AbstractBinderFactory.java @@ -73,8 +73,8 @@ import com.yahoo.bard.webservice.druid.util.SketchFieldConverter; import com.yahoo.bard.webservice.metadata.DataSourceMetadataLoadTask; import com.yahoo.bard.webservice.metadata.DataSourceMetadataService; -import com.yahoo.bard.webservice.metadata.RegisteredLookupMetadataLoadTask; import com.yahoo.bard.webservice.metadata.QuerySigningService; +import com.yahoo.bard.webservice.metadata.RegisteredLookupMetadataLoadTask; import com.yahoo.bard.webservice.metadata.RequestedIntervalsFunction; import com.yahoo.bard.webservice.metadata.SegmentIntervalsHashIdGenerator; import com.yahoo.bard.webservice.table.LogicalTableDictionary; @@ -85,9 +85,6 @@ import com.yahoo.bard.webservice.util.SimplifiedIntervalList; import com.yahoo.bard.webservice.web.CsvResponseWriter; import com.yahoo.bard.webservice.web.DataApiRequest; -import com.yahoo.bard.webservice.web.apirequest.DataApiRequestFactory; -import com.yahoo.bard.webservice.web.apirequest.DefaultDataApiRequestFactory; -import com.yahoo.bard.webservice.web.ratelimit.DefaultRateLimiter; import com.yahoo.bard.webservice.web.DefaultResponseFormatResolver; import com.yahoo.bard.webservice.web.DimensionApiRequestMapper; import com.yahoo.bard.webservice.web.DimensionsApiRequest; @@ -106,11 +103,16 @@ import com.yahoo.bard.webservice.web.ResponseWriter; import com.yahoo.bard.webservice.web.SlicesApiRequest; import com.yahoo.bard.webservice.web.TablesApiRequest; +import com.yahoo.bard.webservice.web.apirequest.DataApiRequestFactory; +import com.yahoo.bard.webservice.web.apirequest.DefaultDataApiRequestFactory; import com.yahoo.bard.webservice.web.apirequest.DefaultHavingApiGenerator; import com.yahoo.bard.webservice.web.apirequest.HavingGenerator; import com.yahoo.bard.webservice.web.apirequest.PerRequestDictionaryHavingGenerator; import com.yahoo.bard.webservice.web.handlers.workflow.DruidWorkflow; import com.yahoo.bard.webservice.web.handlers.workflow.RequestWorkflowProvider; +import com.yahoo.bard.webservice.web.ratelimit.DefaultRateLimiter; +import com.yahoo.bard.webservice.web.responseprocessors.ResponseProcessorFactory; +import com.yahoo.bard.webservice.web.responseprocessors.ResultSetResponseProcessorFactory; import com.yahoo.bard.webservice.web.util.QueryWeightUtil; import com.codahale.metrics.Gauge; @@ -339,6 +341,8 @@ protected void configure() { bind(buildResponseFormatResolver()).to(ResponseFormatResolver.class); + bind(buildResponseProcessorFactory()).to(ResponseProcessorFactory.class); + bind(buildRateLimiter()).to(RateLimiter.class); if (DRUID_DIMENSIONS_LOADER.isOn()) { @@ -1200,6 +1204,19 @@ protected ResponseFormatResolver buildResponseFormatResolver() { return new DefaultResponseFormatResolver(); } + /** + * Returns the class to bind to {@link ResponseProcessorFactory}. + *

+ * The ResponseProcessorFactory allows us to inject a custom {@link + * com.yahoo.bard.webservice.web.responseprocessors.ResponseProcessor} despite the fact that these processors depend + * on objects that are built uniquely for each request. + * + * @return A class that implements {@link ResponseProcessorFactory}. + */ + protected Class buildResponseProcessorFactory() { + return ResultSetResponseProcessorFactory.class; + } + /** * Creates a new RateLimiter for the RateLimitFilter. * diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/web/endpoints/DataServlet.java b/fili-core/src/main/java/com/yahoo/bard/webservice/web/endpoints/DataServlet.java index 4e74b5e978..16d750ef0a 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/web/endpoints/DataServlet.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/web/endpoints/DataServlet.java @@ -3,6 +3,7 @@ package com.yahoo.bard.webservice.web.endpoints; import static com.yahoo.bard.webservice.web.handlers.workflow.DruidWorkflow.REQUEST_WORKFLOW_TIMER; + import static javax.ws.rs.core.Response.Status.BAD_REQUEST; import static javax.ws.rs.core.Response.Status.GATEWAY_TIMEOUT; import static javax.ws.rs.core.Response.Status.INTERNAL_SERVER_ERROR; @@ -52,7 +53,8 @@ import com.yahoo.bard.webservice.web.handlers.RequestContext; import com.yahoo.bard.webservice.web.handlers.RequestHandlerUtils; import com.yahoo.bard.webservice.web.handlers.workflow.RequestWorkflowProvider; -import com.yahoo.bard.webservice.web.responseprocessors.ResultSetResponseProcessor; +import com.yahoo.bard.webservice.web.responseprocessors.ResponseProcessor; +import com.yahoo.bard.webservice.web.responseprocessors.ResponseProcessorFactory; import com.yahoo.bard.webservice.web.util.BardConfigResources; import com.codahale.metrics.MetricRegistry; @@ -122,6 +124,7 @@ public class DataServlet extends CORSPreflightServlet implements BardConfigResou private final ObjectMappersSuite objectMappers; private final HttpResponseMaker httpResponseMaker; private final ResponseFormatResolver formatResolver; + private final ResponseProcessorFactory responseProcessorFactory; private final DataApiRequestFactory dataApiRequestFactory; @@ -153,6 +156,7 @@ public class DataServlet extends CORSPreflightServlet implements BardConfigResou * @param formatResolver The formatResolver for determining correct response format * @param dataApiRequestFactory A factory to build dataApiRequests * {@link com.yahoo.bard.webservice.async.preresponses.stores.PreResponseStore} + * @param responseProcessorFactory Builds the object that performs post processing on a Druid response */ @Inject public DataServlet( @@ -172,7 +176,8 @@ public DataServlet( BroadcastChannel preResponseStoredNotifications, HttpResponseMaker httpResponseMaker, ResponseFormatResolver formatResolver, - DataApiRequestFactory dataApiRequestFactory + DataApiRequestFactory dataApiRequestFactory, + ResponseProcessorFactory responseProcessorFactory ) { this.resourceDictionaries = resourceDictionaries; this.druidQueryBuilder = druidQueryBuilder; @@ -192,6 +197,7 @@ public DataServlet( this.httpResponseMaker = httpResponseMaker; this.formatResolver = formatResolver; this.dataApiRequestFactory = dataApiRequestFactory; + this.responseProcessorFactory = responseProcessorFactory; LOG.trace( "Initialized with ResourceDictionaries: {} \n\n" + @@ -426,7 +432,7 @@ public void getData( httpResponseMaker ); - ResultSetResponseProcessor response = new ResultSetResponseProcessor( + ResponseProcessor responseProcessor = responseProcessorFactory.build( apiRequest, queryResultsEmitter, druidResponseParser, @@ -441,7 +447,7 @@ public void getData( // Process the request RequestLog.startTiming(REQUEST_WORKFLOW_TIMER); - boolean complete = dataRequestHandler.handleRequest(context, apiRequest, druidQuery, response); + boolean complete = dataRequestHandler.handleRequest(context, apiRequest, druidQuery, responseProcessor); if (!complete) { throw new IllegalStateException("No request handler accepted request."); } diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/web/responseprocessors/ResponseProcessorFactory.java b/fili-core/src/main/java/com/yahoo/bard/webservice/web/responseprocessors/ResponseProcessorFactory.java new file mode 100644 index 0000000000..358077e40c --- /dev/null +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/web/responseprocessors/ResponseProcessorFactory.java @@ -0,0 +1,37 @@ +// Copyright 2018 Yahoo Inc. +// Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms. +package com.yahoo.bard.webservice.web.responseprocessors; + +import com.yahoo.bard.webservice.application.ObjectMappersSuite; +import com.yahoo.bard.webservice.data.DruidResponseParser; +import com.yahoo.bard.webservice.data.HttpResponseMaker; +import com.yahoo.bard.webservice.web.DataApiRequest; +import com.yahoo.bard.webservice.web.PreResponse; + +import rx.subjects.Subject; + +/** + * A {@link ResponseProcessor} relies on things that are directly constructed at request time (i.e. the + * {@link DataApiRequest}). Therefore, we can't inject a `ResponseProcessor` directly. We can however inject a factory. + */ +public interface ResponseProcessorFactory { + + /** + * Constructs a custom ResponseProcessor. + * + * @param apiRequest The current request + * @param responseEmitter Generates the response to be processed + * @param druidResponseParser Transforms a druid response into a {@link ResultSet} + * @param objectMappers Dictates how to format + * @param httpResponseMaker Crafts an HTTP response to be sent back to the user from a ResultSet or error message + * + * @return An object that handles parsing and post-processing of Druid requests + */ + ResponseProcessor build( + DataApiRequest apiRequest, + Subject responseEmitter, + DruidResponseParser druidResponseParser, + ObjectMappersSuite objectMappers, + HttpResponseMaker httpResponseMaker + ); +} diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/web/responseprocessors/ResultSetResponseProcessorFactory.java b/fili-core/src/main/java/com/yahoo/bard/webservice/web/responseprocessors/ResultSetResponseProcessorFactory.java new file mode 100644 index 0000000000..56eb874709 --- /dev/null +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/web/responseprocessors/ResultSetResponseProcessorFactory.java @@ -0,0 +1,34 @@ +// Copyright 2018 Yahoo Inc. +// Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms. +package com.yahoo.bard.webservice.web.responseprocessors; + +import com.yahoo.bard.webservice.application.ObjectMappersSuite; +import com.yahoo.bard.webservice.data.DruidResponseParser; +import com.yahoo.bard.webservice.data.HttpResponseMaker; +import com.yahoo.bard.webservice.web.DataApiRequest; +import com.yahoo.bard.webservice.web.PreResponse; + +import rx.subjects.Subject; + +/** + * Builds the default Druid response processor: {@link ResultSetResponseProcessor}. + */ +public class ResultSetResponseProcessorFactory implements ResponseProcessorFactory { + + @Override + public ResponseProcessor build( + DataApiRequest apiRequest, + Subject responseEmitter, + DruidResponseParser druidResponseParser, + ObjectMappersSuite objectMappers, + HttpResponseMaker httpResponseMaker + ) { + return new ResultSetResponseProcessor( + apiRequest, + responseEmitter, + druidResponseParser, + objectMappers, + httpResponseMaker + ); + } +}