From b2a0d2ca62ce004789b7b032974646ca6146bc9f Mon Sep 17 00:00:00 2001 From: jaehong-kim Date: Tue, 15 Dec 2020 15:31:48 +0900 Subject: [PATCH] [#7463] Fix Reactor-Netty HTTP client plugin --- .../ReactorNettyPluginTestController.java | 4 +- .../reactor/netty/ReactorNettyPlugin.java | 10 ++ ...tpClientHandlerConstructorInterceptor.java | 14 -- ...ientHandlerRequestWithBodyInterceptor.java | 111 ++++------------ .../HttpClientOperationsSendInterceptor.java | 120 ++++++++++++++++++ 5 files changed, 156 insertions(+), 103 deletions(-) create mode 100644 plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/interceptor/HttpClientOperationsSendInterceptor.java diff --git a/agent-testweb/reactor-netty-plugin-testweb/src/main/java/com/pinpoint/test/plugin/ReactorNettyPluginTestController.java b/agent-testweb/reactor-netty-plugin-testweb/src/main/java/com/pinpoint/test/plugin/ReactorNettyPluginTestController.java index b472cf742bd3..49c93051ebb4 100644 --- a/agent-testweb/reactor-netty-plugin-testweb/src/main/java/com/pinpoint/test/plugin/ReactorNettyPluginTestController.java +++ b/agent-testweb/reactor-netty-plugin-testweb/src/main/java/com/pinpoint/test/plugin/ReactorNettyPluginTestController.java @@ -47,7 +47,7 @@ public String clientGet() { return response; } - @RequestMapping(value = "/client/get_local", method = RequestMethod.GET) + @RequestMapping(value = "/client/local", method = RequestMethod.GET) @ResponseBody public String clientError(HttpServletRequest request) { HttpClient client = HttpClient.create().port(request.getLocalPort()); @@ -63,7 +63,7 @@ public String clientPost() { return response.toString(); } - @RequestMapping(value = "/client/unknown_host", method = RequestMethod.GET) + @RequestMapping(value = "/client/unknown", method = RequestMethod.GET) @ResponseBody public String clientError() { HttpClient client = HttpClient.create().port(80); diff --git a/plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/ReactorNettyPlugin.java b/plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/ReactorNettyPlugin.java index bba3189cfcf5..aa340743a2de 100644 --- a/plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/ReactorNettyPlugin.java +++ b/plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/ReactorNettyPlugin.java @@ -43,6 +43,7 @@ import com.navercorp.pinpoint.plugin.reactor.netty.interceptor.HttpClientOperationsOnInboundNextInterceptor; import com.navercorp.pinpoint.plugin.reactor.netty.interceptor.HttpClientOperationsOnOutboundCompleteInterceptor; import com.navercorp.pinpoint.plugin.reactor.netty.interceptor.HttpClientOperationsOnOutboundErrorInterceptor; +import com.navercorp.pinpoint.plugin.reactor.netty.interceptor.HttpClientOperationsSendInterceptor; import com.navercorp.pinpoint.plugin.reactor.netty.interceptor.HttpServerHandleHttpServerStateInterceptor; import com.navercorp.pinpoint.plugin.reactor.netty.interceptor.HttpServerHandleStateInterceptor; import com.navercorp.pinpoint.plugin.reactor.netty.interceptor.HttpTcpClientConnectInterceptor; @@ -189,6 +190,15 @@ public static class HttpClientOperationsTransform implements TransformCallback { public byte[] doInTransform(Instrumentor instrumentor, ClassLoader loader, String className, Class classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) throws InstrumentException { InstrumentClass target = instrumentor.getInstrumentClass(loader, className, classfileBuffer); target.addField(AsyncContextAccessor.class); + + final InstrumentMethod sendMethod = target.getDeclaredMethod("send"); + if (sendMethod != null) { + sendMethod.addInterceptor(HttpClientOperationsSendInterceptor.class); + } + final InstrumentMethod sendArgMethod = target.getDeclaredMethod("send", "org.reactivestreams.Publisher"); + if (sendArgMethod != null) { + sendArgMethod.addInterceptor(HttpClientOperationsSendInterceptor.class); + } final InstrumentMethod onOutboundCompleteMethod = target.getDeclaredMethod("onOutboundComplete"); if (onOutboundCompleteMethod != null) { onOutboundCompleteMethod.addInterceptor(HttpClientOperationsOnOutboundCompleteInterceptor.class); diff --git a/plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/interceptor/HttpClientHandlerConstructorInterceptor.java b/plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/interceptor/HttpClientHandlerConstructorInterceptor.java index 669002bbdac2..ab512c6a5f64 100644 --- a/plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/interceptor/HttpClientHandlerConstructorInterceptor.java +++ b/plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/interceptor/HttpClientHandlerConstructorInterceptor.java @@ -22,13 +22,8 @@ import com.navercorp.pinpoint.bootstrap.context.SpanEventRecorder; import com.navercorp.pinpoint.bootstrap.context.TraceContext; import com.navercorp.pinpoint.bootstrap.interceptor.SpanEventSimpleAroundInterceptorForPlugin; -import com.navercorp.pinpoint.bootstrap.plugin.util.SocketAddressUtils; -import com.navercorp.pinpoint.common.plugin.util.HostAndPort; -import com.navercorp.pinpoint.common.trace.AnnotationKey; import com.navercorp.pinpoint.plugin.reactor.netty.ReactorNettyConstants; -import java.net.InetSocketAddress; - /** * @author jaehong.kim */ @@ -40,15 +35,6 @@ public HttpClientHandlerConstructorInterceptor(TraceContext traceContext, Method @Override public void doInBeforeTrace(SpanEventRecorder recorder, Object target, Object[] args) throws Exception { - if (args != null && args.length >= 2 && args[1] instanceof InetSocketAddress) { - final InetSocketAddress inetSocketAddress = (InetSocketAddress) args[1]; - if (inetSocketAddress != null) { - final String hostName = SocketAddressUtils.getHostNameFirst(inetSocketAddress); - if (hostName != null) { - recorder.recordAttribute(AnnotationKey.HTTP_INTERNAL_DISPLAY, HostAndPort.toHostAndPortString(hostName, inetSocketAddress.getPort())); - } - } - } } @Override diff --git a/plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/interceptor/HttpClientHandlerRequestWithBodyInterceptor.java b/plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/interceptor/HttpClientHandlerRequestWithBodyInterceptor.java index 550fed039411..d24869e73230 100644 --- a/plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/interceptor/HttpClientHandlerRequestWithBodyInterceptor.java +++ b/plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/interceptor/HttpClientHandlerRequestWithBodyInterceptor.java @@ -17,122 +17,59 @@ package com.navercorp.pinpoint.plugin.reactor.netty.interceptor; import com.navercorp.pinpoint.bootstrap.async.AsyncContextAccessor; -import com.navercorp.pinpoint.bootstrap.async.AsyncContextAccessorUtils; import com.navercorp.pinpoint.bootstrap.context.AsyncContext; import com.navercorp.pinpoint.bootstrap.context.MethodDescriptor; import com.navercorp.pinpoint.bootstrap.context.SpanEventRecorder; -import com.navercorp.pinpoint.bootstrap.context.Trace; import com.navercorp.pinpoint.bootstrap.context.TraceContext; -import com.navercorp.pinpoint.bootstrap.context.TraceId; import com.navercorp.pinpoint.bootstrap.interceptor.AsyncContextSpanEventSimpleAroundInterceptor; -import com.navercorp.pinpoint.bootstrap.logging.PLogger; -import com.navercorp.pinpoint.bootstrap.logging.PLoggerFactory; -import com.navercorp.pinpoint.bootstrap.plugin.request.ClientRequestAdaptor; -import com.navercorp.pinpoint.bootstrap.plugin.request.ClientRequestRecorder; -import com.navercorp.pinpoint.bootstrap.plugin.request.ClientRequestWrapper; -import com.navercorp.pinpoint.bootstrap.plugin.request.ClientRequestWrapperAdaptor; -import com.navercorp.pinpoint.bootstrap.plugin.request.DefaultRequestTraceWriter; -import com.navercorp.pinpoint.bootstrap.plugin.request.RequestTraceWriter; +import com.navercorp.pinpoint.bootstrap.plugin.util.SocketAddressUtils; +import com.navercorp.pinpoint.common.plugin.util.HostAndPort; +import com.navercorp.pinpoint.common.trace.AnnotationKey; import com.navercorp.pinpoint.plugin.reactor.netty.ReactorNettyConstants; -import com.navercorp.pinpoint.plugin.reactor.netty.ReactorNettyPluginConfig; -import reactor.netty.http.client.HttpClientRequest; +import reactor.netty.channel.ChannelOperations; + +import java.net.InetSocketAddress; /** * @author jaehong.kim */ public class HttpClientHandlerRequestWithBodyInterceptor extends AsyncContextSpanEventSimpleAroundInterceptor { - private final PLogger logger = PLoggerFactory.getLogger(this.getClass()); - private final boolean isDebug = logger.isDebugEnabled(); - - private final ClientRequestRecorder clientRequestRecorder; - private final RequestTraceWriter requestTraceWriter; public HttpClientHandlerRequestWithBodyInterceptor(TraceContext traceContext, MethodDescriptor methodDescriptor) { super(traceContext, methodDescriptor); - - final ReactorNettyPluginConfig config = new ReactorNettyPluginConfig(traceContext.getProfilerConfig()); - final boolean param = config.isParam(); - final ClientRequestAdaptor clientRequestAdaptor = ClientRequestWrapperAdaptor.INSTANCE; - this.clientRequestRecorder = new ClientRequestRecorder(param, clientRequestAdaptor); - final HttpClientRequestHeaderAdaptor clientHeaderAdaptor = new HttpClientRequestHeaderAdaptor(); - this.requestTraceWriter = new DefaultRequestTraceWriter(clientHeaderAdaptor, traceContext); - } - - // BEFORE - @Override - public AsyncContext getAsyncContext(Object target, Object[] args) { - if (Boolean.FALSE == validate(args)) { - return null; - } - - final HttpClientRequest request = (HttpClientRequest) args[0]; - final AsyncContext asyncContext = AsyncContextAccessorUtils.getAsyncContext(target); - if (asyncContext == null) { - // Set sampling rate to false - this.requestTraceWriter.write(request); - return null; - } - return asyncContext; } @Override public void doInBeforeTrace(SpanEventRecorder recorder, AsyncContext asyncContext, Object target, Object[] args) { - final Trace trace = asyncContext.currentAsyncTraceObject(); - if (trace == null) { - if (logger.isWarnEnabled()) { - logger.warn("Unexpected error, Current async trace is null"); - } + if (args == null || args.length < 1) { + // Skip return; } - final TraceId nextId = trace.getTraceId().getNextTraceId(); - recorder.recordNextSpanId(nextId.getSpanId()); - recorder.recordServiceType(ReactorNettyConstants.REACTOR_NETTY_CLIENT); - - final HttpClientRequest request = (HttpClientRequest) args[0]; - final ClientRequestWrapper clientRequestWrapper = new HttpClientRequestWrapper(request); - this.requestTraceWriter.write(request, nextId, clientRequestWrapper.getDestinationId()); // Set HttpClientOptions - if (request instanceof AsyncContextAccessor) { - ((AsyncContextAccessor) request)._$PINPOINT$_setAsyncContext(asyncContext); + if (args[0] instanceof AsyncContextAccessor) { + ((AsyncContextAccessor) args[0])._$PINPOINT$_setAsyncContext(asyncContext); } - } - - // AFTER - @Override - public AsyncContext getAsyncContext(Object target, Object[] args, Object result, Throwable throwable) { - if (Boolean.FALSE == validate(args)) { - return null; + // Set hostname + if (args[0] instanceof ChannelOperations) { + try { + final ChannelOperations channelOperations = (ChannelOperations) args[0]; + final InetSocketAddress inetSocketAddress = (InetSocketAddress) channelOperations.channel().remoteAddress(); + if (inetSocketAddress != null) { + final String hostName = SocketAddressUtils.getHostNameFirst(inetSocketAddress); + if (hostName != null) { + recorder.recordAttribute(AnnotationKey.HTTP_INTERNAL_DISPLAY, HostAndPort.toHostAndPortString(hostName, inetSocketAddress.getPort())); + } + } + } catch (Exception ignored) { + } } - - return AsyncContextAccessorUtils.getAsyncContext(target); } @Override public void doInAfterTrace(SpanEventRecorder recorder, Object target, Object[] args, Object result, Throwable throwable) { recorder.recordApi(methodDescriptor); recorder.recordException(throwable); - - final HttpClientRequest request = (HttpClientRequest) args[0]; - final ClientRequestWrapper clientRequestWrapper = new HttpClientRequestWrapper(request); - this.clientRequestRecorder.record(recorder, clientRequestWrapper, throwable); - } - - private boolean validate(final Object[] args) { - if (args == null || args.length < 1) { - if (isDebug) { - logger.debug("Invalid args object. args={}.", args); - } - return false; - } - - if (!(args[0] instanceof HttpClientRequest)) { - if (isDebug) { - logger.debug("Invalid args[0] object. Need ClientHttpRequest, args[0]={}.", args[0]); - } - return false; - } - - return true; + recorder.recordServiceType(ReactorNettyConstants.REACTOR_NETTY_CLIENT_INTERNAL); } } diff --git a/plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/interceptor/HttpClientOperationsSendInterceptor.java b/plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/interceptor/HttpClientOperationsSendInterceptor.java new file mode 100644 index 000000000000..083c9c14e155 --- /dev/null +++ b/plugins/reactor-netty/src/main/java/com/navercorp/pinpoint/plugin/reactor/netty/interceptor/HttpClientOperationsSendInterceptor.java @@ -0,0 +1,120 @@ +/* + * Copyright 2020 NAVER Corp. + * + * 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.navercorp.pinpoint.plugin.reactor.netty.interceptor; + +import com.navercorp.pinpoint.bootstrap.async.AsyncContextAccessorUtils; +import com.navercorp.pinpoint.bootstrap.context.AsyncContext; +import com.navercorp.pinpoint.bootstrap.context.MethodDescriptor; +import com.navercorp.pinpoint.bootstrap.context.SpanEventRecorder; +import com.navercorp.pinpoint.bootstrap.context.Trace; +import com.navercorp.pinpoint.bootstrap.context.TraceContext; +import com.navercorp.pinpoint.bootstrap.context.TraceId; +import com.navercorp.pinpoint.bootstrap.interceptor.AsyncContextSpanEventSimpleAroundInterceptor; +import com.navercorp.pinpoint.bootstrap.plugin.request.ClientRequestAdaptor; +import com.navercorp.pinpoint.bootstrap.plugin.request.ClientRequestRecorder; +import com.navercorp.pinpoint.bootstrap.plugin.request.ClientRequestWrapper; +import com.navercorp.pinpoint.bootstrap.plugin.request.ClientRequestWrapperAdaptor; +import com.navercorp.pinpoint.bootstrap.plugin.request.DefaultRequestTraceWriter; +import com.navercorp.pinpoint.bootstrap.plugin.request.RequestTraceWriter; +import com.navercorp.pinpoint.plugin.reactor.netty.ReactorNettyConstants; +import com.navercorp.pinpoint.plugin.reactor.netty.ReactorNettyPluginConfig; +import reactor.netty.http.client.HttpClientRequest; + +/** + * @author jaehong.kim + */ +public class HttpClientOperationsSendInterceptor extends AsyncContextSpanEventSimpleAroundInterceptor { + private final ClientRequestRecorder clientRequestRecorder; + private final RequestTraceWriter requestTraceWriter; + + public HttpClientOperationsSendInterceptor(TraceContext traceContext, MethodDescriptor methodDescriptor) { + super(traceContext, methodDescriptor); + + final ReactorNettyPluginConfig config = new ReactorNettyPluginConfig(traceContext.getProfilerConfig()); + final boolean param = config.isParam(); + final ClientRequestAdaptor clientRequestAdaptor = ClientRequestWrapperAdaptor.INSTANCE; + this.clientRequestRecorder = new ClientRequestRecorder(param, clientRequestAdaptor); + final HttpClientRequestHeaderAdaptor clientHeaderAdaptor = new HttpClientRequestHeaderAdaptor(); + this.requestTraceWriter = new DefaultRequestTraceWriter(clientHeaderAdaptor, traceContext); + } + + // BEFORE + @Override + public AsyncContext getAsyncContext(Object target, Object[] args) { + if (Boolean.FALSE == validate(target)) { + return null; + } + + final HttpClientRequest request = (HttpClientRequest) target; + final AsyncContext asyncContext = AsyncContextAccessorUtils.getAsyncContext(target); + if (asyncContext == null) { + // Set sampling rate to false + this.requestTraceWriter.write(request); + return null; + } + return asyncContext; + } + + @Override + public void doInBeforeTrace(SpanEventRecorder recorder, AsyncContext asyncContext, Object target, Object[] args) { + final Trace trace = asyncContext.currentAsyncTraceObject(); + if (trace == null) { + if (logger.isWarnEnabled()) { + logger.warn("Unexpected error, Current async trace is null"); + } + return; + } + final TraceId nextId = trace.getTraceId().getNextTraceId(); + recorder.recordNextSpanId(nextId.getSpanId()); + recorder.recordServiceType(ReactorNettyConstants.REACTOR_NETTY_CLIENT); + + final HttpClientRequest request = (HttpClientRequest) target; + final ClientRequestWrapper clientRequestWrapper = new HttpClientRequestWrapper(request); + this.requestTraceWriter.write(request, nextId, clientRequestWrapper.getDestinationId()); + } + + // AFTER + @Override + public AsyncContext getAsyncContext(Object target, Object[] args, Object result, Throwable throwable) { + if (Boolean.FALSE == validate(target)) { + return null; + } + + return AsyncContextAccessorUtils.getAsyncContext(target); + } + + @Override + public void doInAfterTrace(SpanEventRecorder recorder, Object target, Object[] args, Object result, Throwable throwable) { + recorder.recordApi(methodDescriptor); + recorder.recordException(throwable); + + final HttpClientRequest request = (HttpClientRequest) target; + final ClientRequestWrapper clientRequestWrapper = new HttpClientRequestWrapper(request); + this.clientRequestRecorder.record(recorder, clientRequestWrapper, throwable); + } + + private boolean validate(final Object target) { + if (Boolean.FALSE == (target instanceof HttpClientRequest)) { + if (isDebug) { + logger.debug("Invalid target object. Need ClientHttpRequest, target={}.", target); + } + return false; + } + + return true; + } +}