Skip to content

Commit

Permalink
refactor(web): Move request logger to a servlet filter (#1116)
Browse files Browse the repository at this point in the history
  • Loading branch information
robzienert committed Mar 26, 2020
1 parent 71c6f1a commit 5685f79
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 105 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import com.netflix.spinnaker.filters.AuthenticatedRequestFilter
import com.netflix.spinnaker.gate.config.PostConnectionConfiguringJedisConnectionFactory.ConnectionPostProcessor
import com.netflix.spinnaker.gate.converters.JsonHttpMessageConverter
import com.netflix.spinnaker.gate.converters.YamlHttpMessageConverter
import com.netflix.spinnaker.gate.filters.RequestLoggingFilter
import com.netflix.spinnaker.gate.plugins.deck.DeckPluginConfiguration
import com.netflix.spinnaker.gate.plugins.publish.PluginPublishConfiguration
import com.netflix.spinnaker.gate.retrofit.Slf4jRetrofitLogger
Expand Down Expand Up @@ -95,6 +96,9 @@ class GateConfig extends RedisHttpSessionConfiguration {
@Value('${retrofit.logLevel:BASIC}')
String retrofitLogLevel

@Value('${request-logging.enabled:false}')
Boolean requestLogging

@Autowired
RequestInterceptor spinnakerRequestInterceptor

Expand Down Expand Up @@ -377,7 +381,7 @@ class GateConfig extends RedisHttpSessionConfiguration {
@Bean
FilterRegistrationBean authenticatedRequestFilter() {
def frb = new FilterRegistrationBean(new AuthenticatedRequestFilter(false, true, true))
frb.order = Ordered.LOWEST_PRECEDENCE
frb.order = Ordered.LOWEST_PRECEDENCE - 1
return frb
}

Expand All @@ -395,6 +399,17 @@ class GateConfig extends RedisHttpSessionConfiguration {
return frb
}

/**
* Request logging filter runs immediately after the AuthenticatedRequestFilter (so that the MDCs set by this
* filter are also present in the request log).
*/
@Bean
FilterRegistrationBean requestLoggingFilter() {
def frb = new FilterRegistrationBean(new RequestLoggingFilter())
frb.order = Ordered.LOWEST_PRECEDENCE
return frb
}

@Bean
FiatStatus fiatStatus(DynamicConfigService dynamicConfigService,
Registry registry,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package com.netflix.spinnaker.gate.config
import com.netflix.spectator.api.Registry
import com.netflix.spinnaker.gate.interceptors.RequestContextInterceptor
import com.netflix.spinnaker.gate.interceptors.RequestIdInterceptor
import com.netflix.spinnaker.gate.interceptors.RequestLoggingInterceptor

import com.netflix.spinnaker.gate.interceptors.RequestSheddingInterceptor
import com.netflix.spinnaker.gate.ratelimit.RateLimitPrincipalProvider
import com.netflix.spinnaker.gate.ratelimit.RateLimiter
Expand Down Expand Up @@ -68,9 +68,6 @@ public class GateWebConfig implements WebMvcConfigurer {
@Value('${rate-limit.learning:true}')
Boolean rateLimitLearningMode

@Value('${request-logging.enabled:false}')
Boolean requestLogging

@Override
public void addInterceptors(InterceptorRegistry registry) {
registry.addInterceptor(
Expand All @@ -81,10 +78,6 @@ public class GateWebConfig implements WebMvcConfigurer {

registry.addInterceptor(new RequestIdInterceptor())

if (requestLogging) {
registry.addInterceptor(new RequestLoggingInterceptor())
}

if (rateLimiter != null) {
registry.addInterceptor(new RateLimitingInterceptor(rateLimiter, spectatorRegistry, rateLimiterPrincipalProvider))
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
* Copyright 2020 Netflix, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.netflix.spinnaker.gate.filters;

import static net.logstash.logback.argument.StructuredArguments.value;

import java.io.IOException;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import javax.servlet.*;
import javax.servlet.http.HttpFilter;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.MDC;

public class RequestLoggingFilter extends HttpFilter {
private Logger log = LoggerFactory.getLogger(getClass());

@Override
protected void doFilter(
HttpServletRequest request, HttpServletResponse response, FilterChain chain)
throws IOException, ServletException {
long startTime = System.nanoTime();
try {
chain.doFilter(request, response);
} finally {
try {
MDC.put("requestDuration", getRequestDuration(startTime));
MDC.put("requestUserAgent", request.getHeader("User-Agent"));
MDC.put("requestPort", String.valueOf(request.getServerPort()));

// 127.0.0.1 "GET /limecat.jpg HTTP/1.0" 200 2326
log.debug(
"{} \"{} {} {}\" {} {}",
value("sourceIp", sourceIpAddress(request)),
value("requestMethod", request.getMethod()),
value("requestEndpoint", getRequestEndpoint(request)),
value("requestProtocol", request.getProtocol()),
value("responseStatus", response.getStatus()),
value("responseSize", getResponseSize(response)));
} finally {
MDC.remove("requestDuration");
MDC.remove("requestUserAgent");
MDC.remove("requestPort");
}
}
}

private static String getRequestDuration(long startTime) {
return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTime) + "ms";
}

private static String sourceIpAddress(HttpServletRequest request) {
String ip = request.getHeader("X-FORWARDED-FOR");
if (ip == null) {
return request.getRemoteAddr();
}
return ip;
}

private static String getRequestEndpoint(HttpServletRequest request) {
String endpoint = request.getRequestURI();
if (request.getQueryString() != null) {
return endpoint + "?" + request.getQueryString();
}
return endpoint;
}

private static int getResponseSize(HttpServletResponse response) {
return Optional.ofNullable(response.getHeader("Content-Length"))
.map(Integer::valueOf)
.orElse(-1);
}
}

This file was deleted.

0 comments on commit 5685f79

Please sign in to comment.