Merge pull request #1815 from newrelic/netty-http2-support

Add HTTP/2 Support For Netty 4.1.16.Final +
newrelic · Apr 8, 2024 · c72c054 · c72c054
2 parents c3a1f53 + a751f85
commit c72c054
Show file tree

Hide file tree

Showing 29 changed files with 975 additions and 40 deletions.
diff --git a/instrumentation/netty-3.4/src/main/java/org/jboss/netty/bootstrap/NettyDispatcher.java b/instrumentation/netty-3.4/src/main/java/org/jboss/netty/bootstrap/NettyDispatcher.java
@@ -7,8 +7,6 @@
 
 package org.jboss.netty.bootstrap;
 
-import java.util.logging.Level;
-
 import com.agent.instrumentation.netty34.RequestWrapper;
 import com.newrelic.agent.bridge.AgentBridge;
 import com.newrelic.agent.bridge.Transaction;
@@ -20,10 +18,12 @@
 import org.jboss.netty.channel.ChannelHandlerContext_Instrumentation;
 import org.jboss.netty.handler.codec.http.DefaultHttpRequest;
 
+import java.util.logging.Level;
+
 /**
  * This isn't a netty class. This is an agent ChannelUpstreamHandler which will start a transaction on i/o read. It's
  * best to put this handler in front of "interesting" (e.g. ServerBootstrap) pipelines.
- *
+ * <p>
  * Since this class creates a tracer, its class+method name will show in the TT, hence the class name.
  */
 public class NettyDispatcher {
@@ -61,15 +61,15 @@ public static void upstreamDispatcher(ChannelHandlerContext_Instrumentation ctx,
             }
 
             Transaction tx = AgentBridge.getAgent().getTransaction(false);
+
             if (tx != null) {
                 tx.setWebRequest(new RequestWrapper((DefaultHttpRequest) msg));
             }
 
-        } catch(Throwable t) {
+        } catch (Throwable t) {
             AgentBridge.instrumentation.noticeInstrumentationError(t, Weaver.getImplementationTitle());
         } finally {
             AgentBridge.currentApiSource.remove();
         }
     }
-
 }
diff --git a/...tation/netty-3.4/src/main/java/org/jboss/netty/handler/codec/http/HttpMessageDecoder.java b/...tation/netty-3.4/src/main/java/org/jboss/netty/handler/codec/http/HttpMessageDecoder.java
@@ -7,17 +7,14 @@
 
 package org.jboss.netty.handler.codec.http;
 
-import org.jboss.netty.bootstrap.NettyDispatcher;
-import org.jboss.netty.buffer.ChannelBuffer;
-import org.jboss.netty.channel.Channel;
-import org.jboss.netty.channel.ChannelHandlerContext;
-import org.jboss.netty.channel.ChannelHandlerContext_Instrumentation;
-
-import com.agent.instrumentation.netty34.NettyUtil;
 import com.newrelic.api.agent.Trace;
 import com.newrelic.api.agent.weaver.MatchType;
 import com.newrelic.api.agent.weaver.Weave;
 import com.newrelic.api.agent.weaver.Weaver;
+import org.jboss.netty.bootstrap.NettyDispatcher;
+import org.jboss.netty.buffer.ChannelBuffer;
+import org.jboss.netty.channel.Channel;
+import org.jboss.netty.channel.ChannelHandlerContext_Instrumentation;
 
 @Weave(type = MatchType.BaseClass)
 public class HttpMessageDecoder {

diff --git a/instrumentation/netty-3.8/src/main/java/org/jboss/netty/bootstrap/NettyDispatcher.java b/instrumentation/netty-3.8/src/main/java/org/jboss/netty/bootstrap/NettyDispatcher.java
@@ -7,8 +7,6 @@
 
 package org.jboss.netty.bootstrap;
 
-import java.util.logging.Level;
-
 import com.agent.instrumentation.netty38.RequestWrapper;
 import com.newrelic.agent.bridge.AgentBridge;
 import com.newrelic.agent.bridge.Transaction;
@@ -20,10 +18,12 @@
 import org.jboss.netty.channel.ChannelHandlerContext_Instrumentation;
 import org.jboss.netty.handler.codec.http.DefaultHttpRequest;
 
+import java.util.logging.Level;
+
 /**
  * This isn't a netty class. This is an agent ChannelUpstreamHandler which will start a transaction on i/o read. It's
  * best to put this handler in front of "interesting" (e.g. ServerBootstrap) pipelines.
- * 
+ * <p>
  * Since this class creates a tracer, its class+method name will show in the TT, hence the class name.
  */
 public class NettyDispatcher {
@@ -61,15 +61,15 @@ public static void upstreamDispatcher(ChannelHandlerContext_Instrumentation ctx,
             }
 
             Transaction tx = AgentBridge.getAgent().getTransaction(false);
+
             if (tx != null) {
                 tx.setWebRequest(new RequestWrapper((DefaultHttpRequest) msg));
             }
-            
-        } catch(Throwable t) {
+
+        } catch (Throwable t) {
             AgentBridge.instrumentation.noticeInstrumentationError(t, Weaver.getImplementationTitle());
         } finally {
             AgentBridge.currentApiSource.remove();
         }
     }
-
 }
diff --git a/...tation/netty-3.8/src/main/java/org/jboss/netty/handler/codec/http/HttpMessageDecoder.java b/...tation/netty-3.8/src/main/java/org/jboss/netty/handler/codec/http/HttpMessageDecoder.java
@@ -7,16 +7,13 @@
 
 package org.jboss.netty.handler.codec.http;
 
-import org.jboss.netty.bootstrap.NettyDispatcher;
-import org.jboss.netty.buffer.ChannelBuffer;
-import org.jboss.netty.channel.Channel;
-import org.jboss.netty.channel.ChannelHandlerContext;
-
-import com.agent.instrumentation.netty38.NettyUtil;
 import com.newrelic.api.agent.Trace;
 import com.newrelic.api.agent.weaver.MatchType;
 import com.newrelic.api.agent.weaver.Weave;
 import com.newrelic.api.agent.weaver.Weaver;
+import org.jboss.netty.bootstrap.NettyDispatcher;
+import org.jboss.netty.buffer.ChannelBuffer;
+import org.jboss.netty.channel.Channel;
 import org.jboss.netty.channel.ChannelHandlerContext_Instrumentation;
 
 @Weave(type = MatchType.BaseClass)

diff --git a/instrumentation/netty-4.0.0/src/main/java/io/netty/bootstrap/NettyDispatcher.java b/instrumentation/netty-4.0.0/src/main/java/io/netty/bootstrap/NettyDispatcher.java
@@ -20,7 +20,7 @@
 
 /**
  * This isn't a netty class. This is an agent class which will start a transaction on i/o read.
- *
+ * <p>
  * Since this class creates a tracer, its class+method name will show in the TT, hence the class name.
  */
 public class NettyDispatcher {
@@ -56,9 +56,9 @@ public static void channelRead(ChannelHandlerContext_Instrumentation ctx, Object
         }
 
         Transaction tx = AgentBridge.getAgent().getTransaction(false);
+
         if (tx != null) {
             tx.setWebRequest(new RequestWrapper((DefaultHttpRequest) msg));
         }
     }
-
 }
diff --git a/instrumentation/netty-4.0.0/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java b/instrumentation/netty-4.0.0/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
@@ -7,15 +7,15 @@
 
 package io.netty.handler.codec.http;
 
-import java.util.List;
-
 import com.newrelic.api.agent.weaver.MatchType;
 import com.newrelic.api.agent.weaver.Weave;
 import com.newrelic.api.agent.weaver.Weaver;
 import io.netty.bootstrap.NettyDispatcher;
 import io.netty.buffer.ByteBuf;
 import io.netty.channel.ChannelHandlerContext_Instrumentation;
 
+import java.util.List;
+
 @Weave(type = MatchType.BaseClass)
 public class HttpObjectDecoder {
 
@@ -28,5 +28,4 @@ protected void decode(ChannelHandlerContext_Instrumentation ctx, ByteBuf buffer,
             }
         }
     }
-
 }
diff --git a/instrumentation/netty-4.0.8/build.gradle b/instrumentation/netty-4.0.8/build.gradle
@@ -1,15 +1,18 @@
 dependencies {
-  implementation(project(":agent-bridge"))
-  implementation("io.netty:netty-all:4.0.8.Final")
+    implementation(project(":agent-bridge"))
+    implementation("io.netty:netty-all:4.1.15.Final")
 }
 
 jar {
-  manifest { attributes 'Implementation-Title': 'com.newrelic.instrumentation.netty-4.0.8',
-          'Implementation-Title-Alias': 'netty_instrumentation' }
+    manifest {
+        attributes 'Implementation-Title': 'com.newrelic.instrumentation.netty-4.0.8',
+                'Implementation-Title-Alias': 'netty_instrumentation'
+    }
 }
 
+// This module doesn't support HTTP/2
 verifyInstrumentation {
-  passesOnly 'io.netty:netty-all:[4.0.8.Final,5.0.0.Alpha1)'
+    passesOnly 'io.netty:netty-all:[4.0.8.Final,4.1.16.Final)'
 }
 
 site {

diff --git a/instrumentation/netty-4.0.8/src/main/java/io/netty/bootstrap/NettyDispatcher.java b/instrumentation/netty-4.0.8/src/main/java/io/netty/bootstrap/NettyDispatcher.java
@@ -21,7 +21,7 @@
 
 /**
  * This isn't a netty class. This is an agent class which will start a transaction on i/o read.
- *
+ * <p>
  * Since this class creates a tracer, its class+method name will show in the TT, hence the class name.
  */
 public class NettyDispatcher {
@@ -59,9 +59,9 @@ public static void channelRead(ChannelHandlerContext_Instrumentation ctx, Object
         }
 
         Transaction tx = AgentBridge.getAgent().getTransaction(false);
+
         if (tx != null) {
             tx.setWebRequest(new RequestWrapper((HttpRequest) msg));
         }
     }
-
 }
diff --git a/instrumentation/netty-4.0.8/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java b/instrumentation/netty-4.0.8/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
@@ -7,15 +7,15 @@
 
 package io.netty.handler.codec.http;
 
-import java.util.List;
-
 import com.newrelic.api.agent.weaver.MatchType;
 import com.newrelic.api.agent.weaver.Weave;
 import com.newrelic.api.agent.weaver.Weaver;
 import io.netty.bootstrap.NettyDispatcher;
 import io.netty.buffer.ByteBuf;
 import io.netty.channel.ChannelHandlerContext_Instrumentation;
 
+import java.util.List;
+
 @Weave(type = MatchType.BaseClass)
 public class HttpObjectDecoder {
 
@@ -34,5 +34,4 @@ protected void decode(ChannelHandlerContext_Instrumentation ctx, ByteBuf buffer,
             }
         }
     }
-
 }
diff --git a/....8/src/main/java/io/netty/handler/codec/http2/Http2StreamFrameToHttpObjectCodec_Skip.java b/....8/src/main/java/io/netty/handler/codec/http2/Http2StreamFrameToHttpObjectCodec_Skip.java
@@ -0,0 +1,17 @@
+/*
+ *
+ *  * Copyright 2024 New Relic Corporation. All rights reserved.
+ *  * SPDX-License-Identifier: Apache-2.0
+ *
+ */
+
+package io.netty.handler.codec.http2;
+
+import com.newrelic.api.agent.weaver.SkipIfPresent;
+
+/*
+ * Stops this instrumentation from applying when the netty-4.1.16 module applies
+ */
+@SkipIfPresent(originalName = "io.netty.handler.codec.http2.Http2StreamFrameToHttpObjectCodec")
+public class Http2StreamFrameToHttpObjectCodec_Skip {
+}
diff --git a/instrumentation/netty-4.0.8/src/test/java/netty408/RequestWrapperTest.java b/instrumentation/netty-4.0.8/src/test/java/netty408/RequestWrapperTest.java
@@ -1,3 +1,10 @@
+/*
+ *
+ *  * Copyright 2023 New Relic Corporation. All rights reserved.
+ *  * SPDX-License-Identifier: Apache-2.0
+ *
+ */
+
 package netty408;
 
 import com.agent.instrumentation.netty40.RequestWrapper;
@@ -32,4 +39,4 @@ public void testPercentageEscaping() {
             Assert.assertEquals(expected, requestWrapper.getParameterValues("asdf")[0]);
         }
     }
-}
+}
diff --git a/instrumentation/netty-4.1.16/README.md b/instrumentation/netty-4.1.16/README.md
@@ -0,0 +1,47 @@
+# Netty
+
+In addition to supporting HTTP/1, this instrumentation module adds support for
+HTTP/2 starting with Netty version `io.netty:netty-all:4.1.16.Final`.
+
+At the time this instrumentation was added the most recent version of Netty was
+`io.netty:netty-all:4.1.107.Final` and virtually all the HTTP/2 APIs in the
+project are annotated as unstable.
+
+## HTTP/2
+
+HTTP/2 works in a fundamentally different manner than HTTP/1.
+
+> At the core of all performance enhancements of HTTP/2 is the new binary framing layer, which dictates how the HTTP messages are encapsulated and transferred
+> between the client and server.
+>
+>The "layer" refers to a design choice to introduce a new optimized encoding mechanism between the socket interface and the higher HTTP API exposed to our
+> applications: the HTTP semantics, such as verbs, methods, and headers, are unaffected, but the way they are encoded while in transit is different.
+>
+>In short, HTTP/2 breaks down the HTTP protocol communication into an exchange of binary-encoded frames, which are then mapped to messages that belong to a
+> particular stream, all of which are multiplexed within a single TCP connection.
+>
+>* Stream: A bidirectional flow of bytes within an established connection, which may carry one or more messages.
+>* Message: A complete sequence of frames that map to a logical request or response message.
+>* Frame: The smallest unit of communication in HTTP/2, each containing a frame header, which at a minimum identifies the stream to which the frame belongs.
+>
+>The relation of these terms can be summarized as follows:
+>* All communication is performed over a single TCP connection that can carry any number of bidirectional streams.
+>* Each stream has a unique identifier and optional priority information that is used to carry bidirectional messages.
+>* Each message is a logical HTTP message, such as a request, or response, which consists of one or more frames.
+>* The frame is the smallest unit of communication that carries a specific type of data—e.g., HTTP headers, message payload, and so on. Frames from different
+   streams may be interleaved and then reassembled via the embedded stream identifier in the header of each frame.
+
+## HTTP/2 Instrumentation
+
+To instrument HTTP/2 the `io.netty.handler.codec.http2.Http2FrameCodec` class is weaved and new wrappers are used to handle working
+with HTTP/2 frames instead of request/response objects.
+
+### Request
+
+When `Http2FrameCodec.onHttp2Frame` is invoked on a `Http2HeadersFrame` representing the request headers, `NettyDispatcher.channelRead` is called which stores
+a `token` in the context pipeline, starts a transaction, and sets/wraps the request on the transaction.
+
+### Response
+
+When `Http2FrameCodec.write` is invoked on a `Http2HeadersFrame` representing the response headers, `NettyUtil.processResponse` is called which uses the
+`token` in the context pipeline to get the transaction, sets/wraps the response on the transaction, and expires the `token`.
diff --git a/instrumentation/netty-4.1.16/build.gradle b/instrumentation/netty-4.1.16/build.gradle
@@ -0,0 +1,21 @@
+dependencies {
+    implementation(project(":agent-bridge"))
+    implementation 'io.netty:netty-all:4.1.107.Final'
+}
+
+jar {
+    manifest {
+        attributes 'Implementation-Title': 'com.newrelic.instrumentation.netty-4.1.16',
+                'Implementation-Title-Alias': 'netty_instrumentation'
+    }
+}
+
+verifyInstrumentation {
+    // This module supports HTTP/2 starting from netty 4.1.16.Final
+    passesOnly 'io.netty:netty-all:[4.1.16.Final,5.0.0.Alpha1)'
+}
+
+site {
+    title 'Netty'
+    type 'Appserver'
+}