Repository: pinpoint-apm/pinpoint
Issue: Add configuration for tracing request parameters received by Tomcat
Labels: enhancement
module:plugin

HTTP request parameters may contain sensitive information.
There needs to be configuration to enable/disable tracing request parameters received by Tomcat.


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/agent/src/main/resources/pinpoint.config b/agent/src/main/resources/pinpoint.config
index 25e075431723..57d19c34319f 100644
--- a/agent/src/main/resources/pinpoint.config
+++ b/agent/src/main/resources/pinpoint.config
@@ -98,6 +98,7 @@ profiler.user.include=
 # Hide pinpoint headers.
 profiler.tomcat.hidepinpointheader=true
 profiler.tomcat.excludeurl=/aa/test.html, /bb/exclude.html
+profiler.tomcat.tracerequestparam=true
 
 # original IP address header
 # https://en.wikipedia.org/wiki/X-Forwarded-For
diff --git a/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/config/DefaultProfilerConfig.java b/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/config/DefaultProfilerConfig.java
index 66edc7e2b709..0d62daa431f8 100644
--- a/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/config/DefaultProfilerConfig.java
+++ b/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/config/DefaultProfilerConfig.java
@@ -112,6 +112,7 @@ public static ProfilerConfig load(String pinpointConfigFileName) throws IOExcept
     private int maxSqlBindValueSize = 1024;
 
     private boolean tomcatHidePinpointHeader = true;
+    private boolean tomcatTraceRequestParam = true;
     private Filter<String> tomcatExcludeUrlFilter = new SkipFilter<String>();
     private String tomcatRealIpHeader;
     private String tomcatRealIpEmptyValue;
@@ -325,6 +326,11 @@ public boolean isTomcatHidePinpointHeader() {
         return tomcatHidePinpointHeader;
     }
 
+    @Override
+    public boolean isTomcatTraceRequestParam() {
+        return tomcatTraceRequestParam;
+    }
+
     @Override
     public Filter<String> getTomcatExcludeUrlFilter() {
         return tomcatExcludeUrlFilter;
@@ -560,6 +566,7 @@ void readPropertyValues() {
         this.traceSqlBindValue = readBoolean("profiler.jdbc.tracesqlbindvalue", false);
 
         this.tomcatHidePinpointHeader = readBoolean("profiler.tomcat.hidepinpointheader", true);
+        this.tomcatTraceRequestParam = readBoolean("profiler.tomcat.tracerequestparam", true);
         final String tomcatExcludeURL = readString("profiler.tomcat.excludeurl", "");
         if (!tomcatExcludeURL.isEmpty()) {
             this.tomcatExcludeUrlFilter = new ExcludeUrlFilter(tomcatExcludeURL);
@@ -785,6 +792,8 @@ public String toString() {
         builder.append(maxSqlBindValueSize);
         builder.append(", tomcatHidePinpointHeader=");
         builder.append(tomcatHidePinpointHeader);
+        builder.append(", tomcatTraceRequestParam=");
+        builder.append(tomcatTraceRequestParam);
         builder.append(", tomcatExcludeUrlFilter=");
         builder.append(tomcatExcludeUrlFilter);
         builder.append(", tomcatExcludeProfileMethodFilter=");
diff --git a/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/config/ProfilerConfig.java b/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/config/ProfilerConfig.java
index 75e44cf80abe..8806047ead0d 100644
--- a/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/config/ProfilerConfig.java
+++ b/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/config/ProfilerConfig.java
@@ -77,6 +77,8 @@ public interface ProfilerConfig {
 
     boolean isTomcatHidePinpointHeader();
 
+    boolean isTomcatTraceRequestParam();
+
     Filter<String> getTomcatExcludeUrlFilter();
 
     String getTomcatRealIpHeader();
diff --git a/plugins/tomcat/src/main/java/com/navercorp/pinpoint/plugin/tomcat/interceptor/StandardHostValveInvokeInterceptor.java b/plugins/tomcat/src/main/java/com/navercorp/pinpoint/plugin/tomcat/interceptor/StandardHostValveInvokeInterceptor.java
index 3321ba89de8d..548ac2028a13 100644
--- a/plugins/tomcat/src/main/java/com/navercorp/pinpoint/plugin/tomcat/interceptor/StandardHostValveInvokeInterceptor.java
+++ b/plugins/tomcat/src/main/java/com/navercorp/pinpoint/plugin/tomcat/interceptor/StandardHostValveInvokeInterceptor.java
@@ -52,31 +52,32 @@ public class StandardHostValveInvokeInterceptor implements AroundInterceptor {
     private final boolean isDebug = logger.isDebugEnabled();
     private final boolean isTrace = logger.isTraceEnabled();
 
+    private final boolean isTraceRequestParam;
+    private final Filter<String> excludeUrlFilter;
+    private final Filter<String> excludeProfileMethodFilter;
+    private final RemoteAddressResolver<HttpServletRequest> remoteAddressResolver;
+
     private MethodDescriptor methodDescriptor;
     private TraceContext traceContext;
 
-    private Filter<String> excludeUrlFilter;
-	private Filter<String> excludeProfileMethodFilter;
-
-    private RemoteAddressResolver<HttpServletRequest> remoteAddressResolver;
-
     public StandardHostValveInvokeInterceptor(TraceContext traceContext, MethodDescriptor descriptor, Filter<String> excludeFilter) {
         this.traceContext = traceContext;
         this.methodDescriptor = descriptor;
         this.excludeUrlFilter = excludeFilter;
-        this.excludeProfileMethodFilter = traceContext.getProfilerConfig().getTomcatExcludeProfileMethodFilter();
-
-        traceContext.cacheApi(SERVLET_ASYNCHRONOUS_API_TAG);
-        traceContext.cacheApi(SERVLET_SYNCHRONOUS_API_TAG);
 
         ProfilerConfig profilerConfig = traceContext.getProfilerConfig();
         final String proxyIpHeader = profilerConfig.getTomcatRealIpHeader();
         if (proxyIpHeader == null || proxyIpHeader.isEmpty()) {
-            remoteAddressResolver = new Bypass<HttpServletRequest>();
+            this.remoteAddressResolver = new Bypass<HttpServletRequest>();
         } else {
             final String tomcatRealIpEmptyValue = profilerConfig.getTomcatRealIpEmptyValue();
-            remoteAddressResolver = new RealIpHeaderResolver<HttpServletRequest>(proxyIpHeader, tomcatRealIpEmptyValue);
+            this.remoteAddressResolver = new RealIpHeaderResolver<HttpServletRequest>(proxyIpHeader, tomcatRealIpEmptyValue);
         }
+        this.isTraceRequestParam = profilerConfig.isTomcatTraceRequestParam();
+        this.excludeProfileMethodFilter = profilerConfig.getTomcatExcludeProfileMethodFilter();
+
+        traceContext.cacheApi(SERVLET_ASYNCHRONOUS_API_TAG);
+        traceContext.cacheApi(SERVLET_SYNCHRONOUS_API_TAG);
     }
 
     @Override
@@ -115,7 +116,7 @@ public String resolve(T servletRequest) {
     public static class RealIpHeaderResolver<T extends HttpServletRequest> implements RemoteAddressResolver<T> {
 
         public static final String X_FORWARDED_FOR = "x-forwarded-for";
-        public static final String X_REAL_IP =  "x-real-ip";
+        public static final String X_REAL_IP = "x-real-ip";
         public static final String UNKNOWN = "unknown";
 
         private final String realIpHeaderName;
@@ -200,11 +201,11 @@ private Trace createTrace(Object target, Object[] args) {
                 recordRootSpan(recorder, request);
                 setTraceMetadata(request, trace);
                 if (isDebug) {
-                    logger.debug("TraceID exist. continue trace. traceId:{}, requestUrl:{}, remoteAddr:{}", new Object[] { traceId, request.getRequestURI(), request.getRemoteAddr() });
+                    logger.debug("TraceID exist. continue trace. traceId:{}, requestUrl:{}, remoteAddr:{}", new Object[]{traceId, request.getRequestURI(), request.getRemoteAddr()});
                 }
             } else {
                 if (isDebug) {
-                    logger.debug("TraceID exist. camSampled is false. skip trace. traceId:{}, requestUrl:{}, remoteAddr:{}", new Object[] { traceId, request.getRequestURI(), request.getRemoteAddr() });
+                    logger.debug("TraceID exist. camSampled is false. skip trace. traceId:{}, requestUrl:{}, remoteAddr:{}", new Object[]{traceId, request.getRequestURI(), request.getRemoteAddr()});
                 }
             }
             return trace;
@@ -228,7 +229,7 @@ private Trace createTrace(Object target, Object[] args) {
 
     private void setTraceMetadata(final Request request, final Trace trace) {
         if (request instanceof TraceAccessor) {
-            ((TraceAccessor)request)._$PINPOINT$_setTrace(trace);
+            ((TraceAccessor) request)._$PINPOINT$_setTrace(trace);
         }
     }
 
@@ -237,7 +238,7 @@ private Trace getTraceMetadata(final Request request) {
             return null;
         }
 
-        return ((TraceAccessor)request)._$PINPOINT$_getTrace();
+        return ((TraceAccessor) request)._$PINPOINT$_getTrace();
     }
 
     private boolean getAsyncMetadata(final Request request) {
@@ -245,7 +246,7 @@ private boolean getAsyncMetadata(final Request request) {
             return false;
         }
 
-        return ((AsyncAccessor)request)._$PINPOINT$_isAsync();
+        return ((AsyncAccessor) request)._$PINPOINT$_isAsync();
     }
 
     private boolean isAsynchronousProcess(final Request request) {
@@ -267,7 +268,7 @@ private void recordRootSpan(final SpanRecorder recorder, final HttpServletReques
         final String endPoint = request.getServerName() + ":" + port;
         recorder.recordEndPoint(endPoint);
 
-        final String remoteAddr =  remoteAddressResolver.resolve(request);
+        final String remoteAddr = remoteAddressResolver.resolve(request);
         recorder.recordRemoteAddress(remoteAddr);
 
         if (!recorder.isRoot()) {
@@ -310,12 +311,14 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
         // ------------------------------------------------------
         try {
             SpanEventRecorder recorder = trace.currentSpanEventRecorder();
-            final HttpServletRequest request = (HttpServletRequest) args[0];
 
-            if (!excludeProfileMethodFilter.filter(request.getMethod())) {
-                final String parameters = getRequestParameter(request, 64, 512);
-                if (parameters != null && parameters.length() > 0) {
-                    recorder.recordAttribute(AnnotationKey.HTTP_PARAM, parameters);
+            if (this.isTraceRequestParam) {
+                final HttpServletRequest request = (HttpServletRequest) args[0];
+                if (!excludeProfileMethodFilter.filter(request.getMethod())) {
+                    final String parameters = getRequestParameter(request, 64, 512);
+                    if (parameters != null && parameters.length() > 0) {
+                        recorder.recordAttribute(AnnotationKey.HTTP_PARAM, parameters);
+                    }
                 }
             }
 


Test patch: