Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a config option to enable/disable the request tracking feature. #341

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ final class ServerConfigSchema {
optional("responseInfoHeaderFormat", string()),
optional("httpPipeline", object(opaque())),
optional("logFormat", string()),
optional("userDefined", object(opaque()))
optional("userDefined", object(opaque())),
optional("requestTracking", bool())
))
.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,18 @@ public class InterceptorPipelineBuilder {
private final Environment environment;
private final Iterable<NamedPlugin> plugins;
private final HttpHandler handler;
private final boolean trackRequests;

public InterceptorPipelineBuilder(Environment environment, Iterable<NamedPlugin> plugins, HttpHandler handler) {
public InterceptorPipelineBuilder(Environment environment, Iterable<NamedPlugin> plugins, HttpHandler handler, boolean trackRequests) {
this.environment = requireNonNull(environment);
this.plugins = requireNonNull(plugins);
this.handler = requireNonNull(handler);
this.trackRequests = trackRequests;
}

public HttpHandler build() {
List<HttpInterceptor> interceptors = ImmutableList.copyOf(instrument(plugins, environment));
return new HttpInterceptorPipeline(interceptors, handler);
return new HttpInterceptorPipeline(interceptors, handler, trackRequests);
}

private static List<InstrumentedPlugin> instrument(Iterable<NamedPlugin> namedPlugins, Environment environment) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@
package com.hotels.styx.proxy;

import com.codahale.metrics.Histogram;
import com.hotels.styx.server.HttpErrorStatusListener;
import com.hotels.styx.api.HttpHandler;
import com.hotels.styx.api.MetricRegistry;
import com.hotels.styx.server.RequestStatsCollector;
import com.hotels.styx.proxy.encoders.ConfigurableUnwiseCharsEncoder;
import com.hotels.styx.server.HttpConnectorConfig;
import com.hotels.styx.server.HttpErrorStatusListener;
import com.hotels.styx.server.HttpsConnectorConfig;
import com.hotels.styx.server.RequestStatsCollector;
import com.hotels.styx.server.netty.NettyServerConfig;
import com.hotels.styx.server.netty.ServerConnector;
import com.hotels.styx.server.netty.ServerConnectorFactory;
Expand All @@ -33,6 +33,7 @@
import com.hotels.styx.server.netty.handlers.ExcessConnectionRejector;
import com.hotels.styx.server.netty.handlers.RequestTimeoutHandler;
import com.hotels.styx.server.track.CurrentRequestTracker;
import com.hotels.styx.server.track.RequestTracker;
import io.netty.channel.Channel;
import io.netty.channel.ChannelDuplexHandler;
import io.netty.channel.ChannelHandlerContext;
Expand Down Expand Up @@ -64,27 +65,30 @@ class ProxyConnectorFactory implements ServerConnectorFactory {
private final NettyServerConfig serverConfig;
private final String unwiseCharacters;
private final ResponseEnhancer responseEnhancer;
private final boolean requestTracking;

ProxyConnectorFactory(NettyServerConfig serverConfig,
MetricRegistry metrics,
HttpErrorStatusListener errorStatusListener,
String unwiseCharacters,
ResponseEnhancer responseEnhancer) {
ResponseEnhancer responseEnhancer,
boolean requestTracking) {
this.serverConfig = requireNonNull(serverConfig);
this.metrics = requireNonNull(metrics);
this.errorStatusListener = requireNonNull(errorStatusListener);
this.unwiseCharacters = requireNonNull(unwiseCharacters);
this.responseEnhancer = requireNonNull(responseEnhancer);
this.requestTracking = requestTracking;
}

@Override
public ServerConnector create(HttpConnectorConfig config) {
return new ProxyConnector(config, serverConfig, metrics, errorStatusListener, unwiseCharacters, responseEnhancer);
return new ProxyConnector(config, serverConfig, metrics, errorStatusListener, unwiseCharacters, responseEnhancer, requestTracking);
}

@Override
public ServerConnector create(HttpsConnectorConfig config) {
return new ProxyConnector(config, serverConfig, metrics, errorStatusListener, unwiseCharacters, responseEnhancer);
return new ProxyConnector(config, serverConfig, metrics, errorStatusListener, unwiseCharacters, responseEnhancer, requestTracking);
}

private static final class ProxyConnector implements ServerConnector {
Expand All @@ -98,13 +102,15 @@ private static final class ProxyConnector implements ServerConnector {
private final ConfigurableUnwiseCharsEncoder unwiseCharEncoder;
private final Optional<SslContext> sslContext;
private final ResponseEnhancer responseEnhancer;
private final RequestTracker requestTracker;

private ProxyConnector(HttpConnectorConfig config,
NettyServerConfig serverConfig,
MetricRegistry metrics,
HttpErrorStatusListener errorStatusListener,
String unwiseCharacters,
ResponseEnhancer responseEnhancer) {
ResponseEnhancer responseEnhancer,
boolean requestTracking) {
this.responseEnhancer = requireNonNull(responseEnhancer);
this.config = requireNonNull(config);
this.serverConfig = requireNonNull(serverConfig);
Expand All @@ -119,6 +125,7 @@ private ProxyConnector(HttpConnectorConfig config,
} else {
this.sslContext = Optional.empty();
}
this.requestTracker = requestTracking ? CurrentRequestTracker.INSTANCE : RequestTracker.NO_OP;
}

@Override
Expand Down Expand Up @@ -162,7 +169,7 @@ public void configure(Channel channel, HttpHandler httpPipeline) {
.progressListener(requestStatsCollector)
.metricRegistry(metrics)
.secure(sslContext.isPresent())
.requestTracker(CurrentRequestTracker.INSTANCE)
.requestTracker(requestTracker)
.build());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,10 @@ public ProxyServerBuilder(Environment environment) {
public HttpServer build() {
ProxyServerConfig proxyConfig = environment.styxConfig().proxyServerConfig();
String unwiseCharacters = environment.styxConfig().get(ENCODE_UNWISECHARS).orElse("");
boolean requestTracking = environment.configuration().get("requestTracking", Boolean.class).orElse(false);

return new NettyServerBuilderSpec("Proxy", environment.serverEnvironment(),
new ProxyConnectorFactory(proxyConfig, environment.metricRegistry(), environment.errorListener(), unwiseCharacters, this::addInfoHeader))
new ProxyConnectorFactory(proxyConfig, environment.metricRegistry(), environment.errorListener(), unwiseCharacters, this::addInfoHeader, requestTracking))
.toNettyServerBuilder(proxyConfig)
.httpHandler(httpHandler)
// register health check
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,26 @@ public class StaticPipelineFactory implements HttpPipelineFactory {
private final Environment environment;
private final Registry<BackendService> registry;
private final Iterable<NamedPlugin> plugins;
private final boolean trackRequests;

@VisibleForTesting
StaticPipelineFactory(BackendServiceClientFactory clientFactory, Environment environment, Registry<BackendService> registry, Iterable<NamedPlugin> plugins) {
StaticPipelineFactory(BackendServiceClientFactory clientFactory,
Environment environment,
Registry<BackendService> registry,
Iterable<NamedPlugin> plugins,
boolean trackRequests) {
this.clientFactory = clientFactory;
this.environment = environment;
this.registry = registry;
this.plugins = plugins;
this.trackRequests = trackRequests;
}

public StaticPipelineFactory(Environment environment, Registry<BackendService> registry, Iterable<NamedPlugin> plugins) {
this(createClientFactory(environment), environment, registry, plugins);
public StaticPipelineFactory(Environment environment,
Registry<BackendService> registry,
Iterable<NamedPlugin> plugins,
boolean trackRequests) {
this(createClientFactory(environment), environment, registry, plugins, trackRequests);
}

private static BackendServiceClientFactory createClientFactory(Environment environment) {
Expand All @@ -59,6 +68,6 @@ public HttpHandler build() {
registry.addListener(backendServicesRouter);
RouteHandlerAdapter router = new RouteHandlerAdapter(backendServicesRouter);

return new InterceptorPipelineBuilder(environment, plugins, router).build();
return new InterceptorPipelineBuilder(environment, plugins, router, trackRequests).build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import com.hotels.styx.routing.config.RouteHandlerDefinition;
import com.hotels.styx.routing.config.RouteHandlerFactory;
import com.hotels.styx.routing.config.RouteHandlerReference;
import com.hotels.styx.server.track.CurrentRequestTracker;
import com.hotels.styx.server.track.RequestTracker;

import java.util.List;
import java.util.Map;
Expand All @@ -50,8 +52,9 @@
public class HttpInterceptorPipeline implements HttpHandler {
private final StandardHttpPipeline handler;

public HttpInterceptorPipeline(List<HttpInterceptor> interceptors, HttpHandler handler) {
this.handler = new StandardHttpPipeline(interceptors, handler);
public HttpInterceptorPipeline(List<HttpInterceptor> interceptors, HttpHandler handler, boolean trackRequests) {
RequestTracker tracker = trackRequests ? CurrentRequestTracker.INSTANCE : RequestTracker.NO_OP;
this.handler = new StandardHttpPipeline(interceptors, handler, tracker);
}

@Override
Expand All @@ -65,10 +68,12 @@ public Eventual<LiveHttpResponse> handle(LiveHttpRequest request, HttpIntercepto
public static class ConfigFactory implements HttpHandlerFactory {
private final Map<String, NamedPlugin> interceptors;
private final BuiltinInterceptorsFactory interceptorFactory;
private final boolean requestTracking;

public ConfigFactory(Iterable<NamedPlugin> interceptors, BuiltinInterceptorsFactory interceptorFactory) {
public ConfigFactory(Iterable<NamedPlugin> interceptors, BuiltinInterceptorsFactory interceptorFactory, boolean requestTracking) {
this.interceptors = toMap(interceptors);
this.interceptorFactory = interceptorFactory;
this.requestTracking = requestTracking;
}

private static List<RouteHandlerConfig> styxHttpPipeline(JsonNode pipeline) {
Expand Down Expand Up @@ -100,7 +105,10 @@ public HttpHandler build(List<String> parents, RouteHandlerFactory builtinsFacto
.get("handler", RouteHandlerDefinition.class)
.orElseThrow(() -> missingAttributeError(configBlock, join(".", parents), "handler"));

return new HttpInterceptorPipeline(interceptors, builtinsFactory.build(append(parents, "handler"), handlerConfig));
return new HttpInterceptorPipeline(
interceptors,
builtinsFactory.build(append(parents, "handler"), handlerConfig),
requestTracking);
}

private List<HttpInterceptor> getHttpInterceptors(List<String> parents, JsonNode pipeline) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
import com.hotels.styx.api.HttpInterceptor;
import com.hotels.styx.api.LiveHttpRequest;
import com.hotels.styx.api.LiveHttpResponse;
import com.hotels.styx.server.track.CurrentRequestTracker;

import com.hotels.styx.server.track.RequestTracker;
import rx.Observable;

/**
Expand All @@ -39,19 +39,21 @@
class StandardHttpPipeline implements HttpHandler {
private final List<HttpInterceptor> interceptors;
private final HttpHandler handler;
private final RequestTracker requestTracker;

public StandardHttpPipeline(HttpHandler handler) {
this(emptyList(), handler);
this(emptyList(), handler, RequestTracker.NO_OP);
}

public StandardHttpPipeline(List<HttpInterceptor> interceptors, HttpHandler handler) {
public StandardHttpPipeline(List<HttpInterceptor> interceptors, HttpHandler handler, RequestTracker requestTracker) {
this.interceptors = requireNonNull(interceptors);
this.handler = requireNonNull(handler);
this.requestTracker = requireNonNull(requestTracker);
}

@Override
public Eventual<LiveHttpResponse> handle(LiveHttpRequest request, HttpInterceptor.Context context) {
HttpInterceptorChain interceptorsChain = new HttpInterceptorChain(interceptors, 0, handler, context);
HttpInterceptorChain interceptorsChain = new HttpInterceptorChain(interceptors, 0, handler, context, requestTracker);

return interceptorsChain.proceed(request);
}
Expand All @@ -61,16 +63,18 @@ static final class HttpInterceptorChain implements HttpInterceptor.Chain {
private final int index;
private final HttpHandler client;
private final HttpInterceptor.Context context;
private final RequestTracker requestTracker;

HttpInterceptorChain(List<HttpInterceptor> interceptors, int index, HttpHandler client, HttpInterceptor.Context context) {
HttpInterceptorChain(List<HttpInterceptor> interceptors, int index, HttpHandler client, HttpInterceptor.Context context, RequestTracker requestTracker) {
this.interceptors = interceptors;
this.index = index;
this.client = client;
this.context = context;
this.requestTracker = requireNonNull(requestTracker);
}

HttpInterceptorChain(HttpInterceptorChain adapter, int index) {
this(adapter.interceptors, index, adapter.client, adapter.context);
this(adapter.interceptors, index, adapter.client, adapter.context, adapter.requestTracker);
}

@Override
Expand All @@ -80,7 +84,7 @@ public HttpInterceptor.Context context() {

@Override
public Eventual<LiveHttpResponse> proceed(LiveHttpRequest request) {
CurrentRequestTracker.INSTANCE.trackRequest(request);
requestTracker.trackRequest(request);

if (index < interceptors.size()) {
HttpInterceptor.Chain chain = new HttpInterceptorChain(this, index + 1);
Expand All @@ -93,7 +97,7 @@ public Eventual<LiveHttpResponse> proceed(LiveHttpRequest request) {
}
}

CurrentRequestTracker.INSTANCE.markRequestAsSent(request);
requestTracker.markRequestAsSent(request);

return new Eventual<>(toPublisher(toObservable(client.handle(request, this.context))
.compose(StandardHttpPipeline::sendErrorOnDoubleSubscription)));
Expand All @@ -112,4 +116,4 @@ private static Observable<LiveHttpResponse> sendErrorOnDoubleSubscription(Observ
});
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,20 +64,24 @@ public HttpHandler create(StyxServerComponents config) {
BuiltinInterceptorsFactory builtinInterceptorsFactory = new BuiltinInterceptorsFactory(
ImmutableMap.of("Rewrite", new RewriteInterceptor.ConfigFactory()));

boolean requestTracking = config.environment().configuration().get("requestTracking", Boolean.class).orElse(false);

Map<String, HttpHandlerFactory> objectFactories = createBuiltinRoutingObjectFactories(
config.environment(),
config.services(),
config.plugins(),
builtinInterceptorsFactory);
builtinInterceptorsFactory,
requestTracking);

RouteHandlerFactory routeHandlerFactory = new RouteHandlerFactory(objectFactories, new ConcurrentHashMap<>());

return styxHttpPipeline(
config.environment().styxConfig(),
configuredPipeline(config.environment(), config.services(), config.plugins(), routeHandlerFactory));
configuredPipeline(config.environment(), config.services(), config.plugins(), routeHandlerFactory),
requestTracking);
}

private static HttpHandler styxHttpPipeline(StyxConfig config, HttpHandler interceptorsPipeline) {
private static HttpHandler styxHttpPipeline(StyxConfig config, HttpHandler interceptorsPipeline, boolean requestTracking) {
ImmutableList.Builder<HttpInterceptor> builder = ImmutableList.builder();

boolean loggingEnabled = config.get("request-logging.inbound.enabled", Boolean.class)
Expand All @@ -98,7 +102,7 @@ private static HttpHandler styxHttpPipeline(StyxConfig config, HttpHandler inter
.add(new HopByHopHeadersRemovingInterceptor())
.add(new RequestEnrichingInterceptor(config.styxHeaderConfig()));

return new HttpInterceptorPipeline(builder.build(), interceptorsPipeline);
return new HttpInterceptorPipeline(builder.build(), interceptorsPipeline, requestTracking);
}

private static HttpHandler configuredPipeline(
Expand All @@ -109,14 +113,16 @@ private static HttpHandler configuredPipeline(
) {
HttpPipelineFactory pipelineBuilder;

boolean requestTracking = environment.configuration().get("requestTracking", Boolean.class).orElse(false);

if (environment.configuration().get("httpPipeline", RouteHandlerDefinition.class).isPresent()) {
pipelineBuilder = () -> {
RouteHandlerDefinition pipelineConfig = environment.configuration().get("httpPipeline", RouteHandlerDefinition.class).get();
return routeHandlerFactory.build(ImmutableList.of("httpPipeline"), pipelineConfig);
};
} else {
Registry<BackendService> backendServicesRegistry = (Registry<BackendService>) servicesFromConfig.get("backendServiceRegistry");
pipelineBuilder = new StaticPipelineFactory(environment, backendServicesRegistry, plugins);
pipelineBuilder = new StaticPipelineFactory(environment, backendServicesRegistry, plugins, requestTracking);
}

return pipelineBuilder.build();
Expand All @@ -126,13 +132,13 @@ private static ImmutableMap<String, HttpHandlerFactory> createBuiltinRoutingObje
Environment environment,
Map<String, StyxService> servicesFromConfig,
Iterable<NamedPlugin> plugins,
BuiltinInterceptorsFactory builtinInterceptorsFactory
) {
BuiltinInterceptorsFactory builtinInterceptorsFactory,
boolean requestTracking) {
return ImmutableMap.of(
"StaticResponseHandler", new StaticResponseHandler.ConfigFactory(),
"ConditionRouter", new ConditionRouter.ConfigFactory(),
"BackendServiceProxy", new BackendServiceProxy.ConfigFactory(environment, backendRegistries(servicesFromConfig)),
"InterceptorPipeline", new HttpInterceptorPipeline.ConfigFactory(plugins, builtinInterceptorsFactory),
"InterceptorPipeline", new HttpInterceptorPipeline.ConfigFactory(plugins, builtinInterceptorsFactory, requestTracking),
"ProxyToBackend", new ProxyToBackend.ConfigFactory(environment, new StyxBackendServiceClientFactory(environment))
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,9 @@
*/
package com.hotels.styx;

import com.hotels.styx.infrastructure.configuration.yaml.YamlConfig;
import com.hotels.styx.proxy.ProxyServerConfig;
import org.testng.annotations.Test;

import static com.hotels.styx.StartupConfig.defaultStartupConfig;
import static com.hotels.styx.support.matchers.IsOptional.isValue;
import static java.lang.Runtime.getRuntime;
import static org.hamcrest.MatcherAssert.assertThat;
Expand Down
Loading