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 log debug headers + config testing #217

Open
wants to merge 40 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
6718de0
add log debug headers
anuvedverma Jul 1, 2024
28bde1d
use error log level
anuvedverma Jul 1, 2024
2773cf3
add logging for error response header
anuvedverma Jul 1, 2024
8bc1af9
other config changes
riteshvaryani Jul 2, 2024
c4e3200
config testing
riteshvaryani Jul 2, 2024
d1ad10a
test
riteshvaryani Jul 2, 2024
365e481
add more changes
riteshvaryani Jul 2, 2024
57e0995
more config changes
riteshvaryani Jul 2, 2024
3eb9da8
add logging for error request/response header size
anuvedverma Jul 2, 2024
7bd21d9
add logging for success at 1%
anuvedverma Jul 2, 2024
e781b3f
remove full header debug from success
anuvedverma Jul 2, 2024
fba8a40
more updates
riteshvaryani Jul 2, 2024
603ff4d
fix
anuvedverma Jul 2, 2024
c17fbf7
fix style
anuvedverma Jul 2, 2024
ccde25e
add check for header not just content size
anuvedverma Jul 2, 2024
a3eb319
Update ProxyHandler.java
riteshvaryani Jul 2, 2024
8f72c71
dropwizard update
riteshvaryani Jul 2, 2024
68d4cf9
Merge branch 'sev-16337-add-header-logging' of github.com:lyft/presto…
riteshvaryani Jul 2, 2024
f488a04
compile fix
riteshvaryani Jul 2, 2024
60db65e
more fixes
riteshvaryani Jul 2, 2024
7092cdb
lint fixes
riteshvaryani Jul 2, 2024
fd6b9f6
fix header logging
anuvedverma Jul 2, 2024
765f61d
Update ProxyHandler.java
riteshvaryani Jul 2, 2024
b145513
Update ProxyHandler.java
riteshvaryani Jul 2, 2024
1ce2104
add logging in preconnection hook
anuvedverma Jul 2, 2024
80ac96c
checkstyle fixes
anuvedverma Jul 2, 2024
5911d0b
adding error counters
jchoi614 Jul 2, 2024
4d1c079
forgot to uncomment
jchoi614 Jul 2, 2024
29613c4
fix checkstyle
jchoi614 Jul 2, 2024
d9cfe8d
update dropwizard ver
jchoi614 Jul 2, 2024
0fee702
updating dropwizard dependency
jchoi614 Jul 2, 2024
94ed9d8
update
riteshvaryani Jul 3, 2024
2cb6d66
pom changes
riteshvaryani Jul 3, 2024
1c25d75
try request logging
riteshvaryani Jul 3, 2024
76f038b
add handler
riteshvaryani Jul 3, 2024
7d60671
add stats handler
anuvedverma Jul 3, 2024
2cd95fe
add StatsHandler jetty logs
anuvedverma Jul 3, 2024
6343539
Merge pull request #218 from lyft/add-stats-handler
anuvedverma Jul 3, 2024
a2303ac
fix debug log
anuvedverma Jul 3, 2024
c772828
shutdown scheduler thread on close
anuvedverma Jul 3, 2024
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
2 changes: 1 addition & 1 deletion baseapp/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<maven.compiler.target>1.8</maven.compiler.target>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
<dropwizard.version>1.3.29</dropwizard.version>
<dropwizard.version>1.3.7</dropwizard.version>
<guice.version>4.1.0</guice.version>
<reflections.version>0.9.10</reflections.version>
</properties>
Expand Down
4 changes: 4 additions & 0 deletions gateway-ha/gateway-ha-config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ requestRouter:
port: 8080
name: prestoRouter
historySize: 1000
requestHeaderSize: 2048000
responseHeaderSize: 2048000
requestBufferSize: 2048000
responseBufferSize: 2048000

dataStore:
jdbcUrl: jdbc:mysql://127.0.0.1:3306/prestogateway
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package com.lyft.data.gateway.ha.handler;

import static com.codahale.metrics.MetricRegistry.name;

import com.codahale.metrics.Counter;
import com.codahale.metrics.Meter;
import com.codahale.metrics.MetricRegistry;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.base.Strings;
import com.google.common.io.CharStreams;
Expand Down Expand Up @@ -28,6 +32,7 @@
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.util.Callback;


@Slf4j
public class QueryIdCachingProxyHandler extends ProxyHandler {
public static final String PROXY_TARGET_HEADER = "proxytarget";
Expand Down Expand Up @@ -57,13 +62,19 @@ public class QueryIdCachingProxyHandler extends ProxyHandler {
private final Meter requestMeter;
private final int serverApplicationPort;

private final Counter errorCounter4xx;
private final Counter errorCounter5xx;

public QueryIdCachingProxyHandler(
QueryHistoryManager queryHistoryManager,
RoutingManager routingManager,
RoutingGroupSelector routingGroupSelector,
int serverApplicationPort,
Meter requestMeter) {
Meter requestMeter,
MetricRegistry metrics) {
this.requestMeter = requestMeter;
this.errorCounter4xx = metrics.counter(name(QueryIdCachingProxyHandler.class, "4xx"));
this.errorCounter5xx = metrics.counter(name(QueryIdCachingProxyHandler.class, "5xx"));
this.routingManager = routingManager;
this.routingGroupSelector = routingGroupSelector;
this.queryHistoryManager = queryHistoryManager;
Expand Down Expand Up @@ -270,6 +281,11 @@ protected void postConnectionHook(
} catch (Exception e) {
log.error("Error in proxying falling back to super call", e);
}
if (response.getStatus() >= 500) {
errorCounter5xx.inc();
} else if (response.getStatus() >= 400) {
errorCounter4xx.inc();
}
super.postConnectionHook(request, response, buffer, offset, length, callback);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.lyft.data.gateway.ha.module;

import com.codahale.metrics.Meter;
import com.codahale.metrics.MetricRegistry;
import com.google.inject.Provides;
import com.google.inject.Singleton;
import com.lyft.data.baseapp.AppModule;
Expand All @@ -23,6 +24,7 @@
import com.lyft.data.proxyserver.ProxyServerConfiguration;
import io.dropwizard.setup.Environment;


public class HaGatewayProviderModule extends AppModule<HaGatewayConfiguration, Environment> {

private final ResourceGroupsManager resourceGroupsManager;
Expand All @@ -42,9 +44,9 @@ public HaGatewayProviderModule(HaGatewayConfiguration configuration, Environment
}

protected ProxyHandler getProxyHandler() {
MetricRegistry metrics = getEnvironment().metrics();
Meter requestMeter =
getEnvironment()
.metrics()
metrics
.meter(getConfiguration().getRequestRouter().getName() + ".requests");

// By default, use routing group header to route
Expand All @@ -61,7 +63,8 @@ protected ProxyHandler getProxyHandler() {
getRoutingManager(),
routingGroupSelector,
getApplicationPort(),
requestMeter);
requestMeter,
metrics);
}

@Provides
Expand All @@ -81,6 +84,7 @@ public ProxyServer provideGateway() {
routerProxyConfig.setKeystorePass(routerConfiguration.getKeystorePass());
routerProxyConfig.setForwardKeystore(routerConfiguration.isForwardKeystore());
routerProxyConfig.setPreserveHost("false");

ProxyHandler proxyHandler = getProxyHandler();
gateway = new ProxyServer(routerProxyConfig, proxyHandler);
}
Expand Down
6 changes: 6 additions & 0 deletions proxyserver/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,16 @@
<maven.compiler.target>1.8</maven.compiler.target>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
<dropwizard.version>1.3.7</dropwizard.version>
</properties>


<dependencies>
<dependency>
<groupId>io.dropwizard</groupId>
<artifactId>dropwizard-core</artifactId>
<version>${dropwizard.version}</version>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-server</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ protected String rewriteTarget(HttpServletRequest request) {
*/
public void preConnectionHook(HttpServletRequest request, Request proxyRequest) {
// you may override it.

// [sev-16337] with a 10% probably, log the request headers for debugging
if (Math.random() < 0.10) {
log.debug("(preConnectionHook) Request URL: {} , request URI {} , servlet path {} ,"
+ "toString {}, getContentLength {}, getRequestHeaderSize {}, requestHeaders {}",
request.getRequestURL(), request.getRequestURI(), request.getServletPath(),
request.toString(), request.getContentLength(), getRequestHeaderSize(request),
errorLogHeaders(request));
}
}

/**
Expand All @@ -60,16 +69,74 @@ protected void postConnectionHook(
request.getRequestURL());
}
response.getOutputStream().write(buffer, offset, length);
// [sev-16337] with a 10% probably, log the request and response headers
// and size for debugging
if (Math.random() < 0.10) {
log.debug("(postConnectionHook) Request URL: {} , request URI {} , servlet path {} , "
+ "toString {}, getContentLength {}, getRequestHeaderSize {}, "
+ "getResponseHeaderSize {}, requestHeaders {}, responseHeaders {}",
request.getRequestURL(), request.getRequestURI(), request.getServletPath(),
request.toString(), request.getContentLength(), getRequestHeaderSize(request),
getResponseHeaderSize(response), errorLogHeaders(request),
errorLogHeaders(response));
}

callback.succeeded();
} catch (Throwable var9) {
log.error("Exception occurred while processing request URL: {} , request URI {} ,"
+ " servlet path {} , toString {}", request.getRequestURL(),
request.getRequestURI(), request.getServletPath(), request.toString(), var9);
log.error("(postConnectionHook) Exception occurred while processing request URL: {} , "
+ "request URI {} , servlet path {} , toString {}, getContentLength {}, "
+ "getRequestHeaderSize {}, getResponseHeaderSize {}, requestHeaders {}, "
+ "responseHeaders {}",
request.getRequestURL(), request.getRequestURI(), request.getServletPath(),
request.toString(), request.getContentLength(), getRequestHeaderSize(request),
getResponseHeaderSize(response), errorLogHeaders(request),
errorLogHeaders(response), var9);

callback.failed(var9);
}
}

protected int getRequestHeaderSize(HttpServletRequest request) {
int headerSize = 0;
Enumeration<String> headerNames = request.getHeaderNames();
while (headerNames.hasMoreElements()) {
String headerName = headerNames.nextElement();
String headerValue = request.getHeader(headerName);
headerSize += headerName.length() + headerValue.length();
}
return headerSize;
}

private int getResponseHeaderSize(HttpServletResponse response) {
int headerSize = 0;
for (String headerName : response.getHeaderNames()) {
String headerValue = response.getHeader(headerName);
headerSize += headerName.length() + headerValue.length();
}
return headerSize;
}

protected String errorLogHeaders(HttpServletRequest request) {
StringBuilder sb = new StringBuilder("------- error HttpServletRequest headers---------");
Enumeration<String> headers = request.getHeaderNames();
while (headers.hasMoreElements()) {
String header = headers.nextElement();
sb.append(header + "->" + request.getHeader(header) + "\n");
}

return sb.toString();
}

protected String errorLogHeaders(HttpServletResponse response) {
StringBuilder sb = new StringBuilder("------- error HttpServletResponse headers---------");
Collection<String> headers = response.getHeaderNames();
for (String header : headers) {
sb.append(header + "->" + response.getHeader(header) + "\n");
}

return sb.toString();
}

protected void debugLogHeaders(HttpServletRequest request) {
if (log.isDebugEnabled()) {
log.debug("-------HttpServletRequest headers---------");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,32 @@
import java.io.Closeable;
import java.io.File;
import java.util.EnumSet;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;

import javax.servlet.DispatcherType;
import javax.servlet.Filter;

import lombok.extern.slf4j.Slf4j;

import org.apache.http.client.ResponseHandler;
import org.apache.http.util.TextUtils;
import org.eclipse.jetty.http.HttpParser;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.proxy.ConnectHandler;
import org.eclipse.jetty.server.CustomRequestLog;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.SecureRequestCustomizer;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.SslConnectionFactory;
import org.eclipse.jetty.server.handler.HandlerCollection;
import org.eclipse.jetty.server.handler.RequestLogHandler;
import org.eclipse.jetty.server.handler.StatisticsHandler;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.eclipse.jetty.util.ssl.SslContextFactory;
Expand All @@ -29,6 +38,9 @@ public class ProxyServer implements Closeable {
private final Server server;
private final ProxyServletImpl proxy;
private final ProxyHandler proxyHandler;
private final ScheduledExecutorService scheduler =
Executors.newScheduledThreadPool(1);

private ServletContextHandler context;

public ProxyServer(ProxyServerConfiguration config, ProxyHandler proxyHandler) {
Expand Down Expand Up @@ -59,6 +71,7 @@ private void setupContext(ProxyServerConfiguration config) {
sslContextFactory.setStopTimeout(TimeUnit.SECONDS.toMillis(15));
sslContextFactory.setSslSessionTimeout((int) TimeUnit.SECONDS.toMillis(15));


if (!TextUtils.isBlank(keystorePath)) {
sslContextFactory.setKeyStorePath(keystoreFile.getAbsolutePath());
sslContextFactory.setKeyStorePassword(keystorePass);
Expand All @@ -68,12 +81,16 @@ private void setupContext(ProxyServerConfiguration config) {
HttpConfiguration httpsConfig = new HttpConfiguration();
httpsConfig.setSecureScheme(HttpScheme.HTTPS.asString());
httpsConfig.setSecurePort(config.getLocalPort());
httpsConfig.setIdleTimeout(150000);
httpsConfig.setOutputBufferSize(32768);
httpsConfig.setRequestHeaderSize(2048000);
httpsConfig.setResponseHeaderSize(2048000);

SecureRequestCustomizer src = new SecureRequestCustomizer();
src.setStsMaxAge(TimeUnit.SECONDS.toSeconds(2000));
src.setStsIncludeSubDomains(true);
httpsConfig.addCustomizer(src);
httpsConfig.addCustomizer(new org.eclipse.jetty.server.ForwardedRequestCustomizer());
connector =
new ServerConnector(
server,
Expand All @@ -86,26 +103,41 @@ private void setupContext(ProxyServerConfiguration config) {
connector.setPort(config.getLocalPort());
connector.setName(config.getName());
connector.setAccepting(true);
connector.setIdleTimeout(150000L);
connector.setAcceptQueueSize(1024);
this.server.addConnector(connector);

// Setup proxy handler to handle CONNECT methods
ConnectHandler proxyConnectHandler = new ConnectHandler();
this.server.setHandler(proxyConnectHandler);

HandlerCollection handlers = new HandlerCollection();

RequestLogHandler requestLogHandler = new RequestLogHandler();
StatisticsHandler statsHandler = new StatisticsHandler();
statsHandler.setHandler(proxyConnectHandler);

//possible not needed
//requestLogHandler.setRequestLog(customRequestLog);
handlers.setHandlers(new Handler[] { requestLogHandler, statsHandler, proxyConnectHandler });

this.server.setHandler(handlers);

if (proxyHandler != null) {
proxy.setProxyHandler(proxyHandler);

}

ServletHolder proxyServlet = new ServletHolder(config.getName(), proxy);

proxyServlet.setInitParameter("proxyTo", config.getProxyTo());
proxyServlet.setInitParameter("prefix", config.getPrefix());
proxyServlet.setInitParameter("trustAll", config.getTrustAll());
proxyServlet.setInitParameter("preserveHost", config.getPreserveHost());
proxyServlet.setInitParameter("timeout", "120000");


// Setup proxy servlet
this.context =
new ServletContextHandler(proxyConnectHandler, "/", ServletContextHandler.SESSIONS);
new ServletContextHandler(handlers, "/", ServletContextHandler.SESSIONS);
this.context.addServlet(proxyServlet, "/*");
this.context.addFilter(RequestFilter.class, "/*", EnumSet.allOf(DispatcherType.class));
}
Expand All @@ -118,6 +150,16 @@ public void start() {

try {
this.server.start();

// Schedule a task to log metrics at a fixed rate
StatisticsHandler stats = this.server.getChildHandlerByClass(StatisticsHandler.class);
this.scheduler.scheduleAtFixedRate(() -> {
log.debug("(jetty) Num requests: " + stats.getRequests());
log.debug("(jetty) Num active requests: " + stats.getRequestsActive());
log.debug("(jetty) Responses with 4xx status: " + stats.getResponses4xx());
log.debug("(jetty) Responses with 5xx status: " + stats.getResponses5xx());
// Log other metrics as needed
}, 0, 5, TimeUnit.MINUTES);
} catch (Exception e) {
log.error("Error starting proxy server", e);
throw new IllegalStateException(e);
Expand All @@ -128,6 +170,7 @@ public void start() {
public void close() {
try {
this.server.stop();
this.scheduler.shutdown();
} catch (Exception e) {
log.error("Could not close the proxy server", e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ protected HttpClient newHttpClient() {

HttpClient httpClient = new HttpClient(sslFactory);
httpClient.setMaxConnectionsPerDestination(10000);
httpClient.setConnectTimeout(TimeUnit.SECONDS.toMillis(60));
httpClient.setConnectTimeout(TimeUnit.SECONDS.toMillis(65));
httpClient.setIdleTimeout(TimeUnit.SECONDS.toMillis(65));
httpClient.setRequestBufferSize(2048000);
httpClient.setResponseBufferSize(2048000);

return httpClient;
}

Expand Down
Loading