From 3acebd3ac5330c7fda00b213986ba8f82d3e6446 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Tue, 17 May 2022 16:45:15 -0700 Subject: [PATCH 01/19] Update HttpURLConnection to use reportAsExternal API --- .../httpurlconnection/MetricState.java | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java b/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java index 08330214f1..e2388d8214 100644 --- a/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java +++ b/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java @@ -10,13 +10,16 @@ import com.newrelic.agent.bridge.AgentBridge; import com.newrelic.agent.bridge.TracedMethod; import com.newrelic.agent.bridge.Transaction; -import com.newrelic.agent.bridge.external.ExternalMetrics; import com.newrelic.agent.bridge.external.URISupport; +import com.newrelic.api.agent.HttpParameters; +import java.io.IOException; import java.net.HttpURLConnection; +import java.net.URI; +import java.net.URISyntaxException; public class MetricState { - + private static final String LIBRARY = "HttpURLConnection"; private boolean metricsRecorded; private boolean recordedANetworkCall; @@ -64,13 +67,20 @@ public void getInboundPostamble(HttpURLConnection connection, TracedMethod metho } private void makeMetric(HttpURLConnection connection, TracedMethod method, String operation) { - String uri = URISupport.getURI(connection.getURL()); - ExternalMetrics.makeExternalComponentMetric( - method, - connection.getURL().getHost(), - "HttpURLConnection", - false, - uri, - operation); + try { + URI uri = connection.getURL().toURI(); + int responseCode = connection.getResponseCode(); + String responseMessage = connection.getResponseMessage(); + + method.reportAsExternal(HttpParameters + .library(LIBRARY) + .uri(uri) + .procedure(operation) + .inboundHeaders(new InboundWrapper(connection)) + .status(responseCode, responseMessage) + .build()); + } catch (URISyntaxException | IOException e) { + throw new RuntimeException(e); + } } } From f512aa02150fe28d73113ecde6b3e94bccf3faef Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Thu, 16 Jun 2022 15:13:26 -0700 Subject: [PATCH 02/19] Update httpurlconnection instrumentation to use reportAsExternal API --- .../bridge/external/ExternalMetrics.java | 4 +- .../httpurlconnection/MetricState.java | 68 +++++++++++-------- .../main/java/java/net/HttpURLConnection.java | 13 ++-- .../MetricStateResponseCodeTest.java | 22 +++--- 4 files changed, 54 insertions(+), 53 deletions(-) diff --git a/agent-bridge/src/main/java/com/newrelic/agent/bridge/external/ExternalMetrics.java b/agent-bridge/src/main/java/com/newrelic/agent/bridge/external/ExternalMetrics.java index a1d60832a8..a2207c4efd 100644 --- a/agent-bridge/src/main/java/com/newrelic/agent/bridge/external/ExternalMetrics.java +++ b/agent-bridge/src/main/java/com/newrelic/agent/bridge/external/ExternalMetrics.java @@ -54,7 +54,7 @@ public static void makeExternalComponentMetric(TracedMethod method, String host, // transaction segment name always contains operations; metric name may or may not String operationsPath = fixOperations(operations); - if(operationsPath == null) { + if (operationsPath == null) { String metricName = MessageFormat.format(METRIC_NAME, host, library); method.setMetricNameFormatInfo(metricName, metricName, uri); } else { @@ -89,7 +89,7 @@ public static void makeExternalComponentTrace(boolean isWebTransaction, TracedMe makeExternalComponentMetric(method, hostName, library, includeOperationInMetric, uri, operations); - if(UNKNOWN_HOST.equals(hostName)) { + if (UNKNOWN_HOST.equals(hostName)) { return; // NR doesn't add rollup metrics for "UnknownHost" } diff --git a/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java b/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java index e2388d8214..7b2caa5b29 100644 --- a/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java +++ b/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java @@ -9,14 +9,16 @@ import com.newrelic.agent.bridge.AgentBridge; import com.newrelic.agent.bridge.TracedMethod; -import com.newrelic.agent.bridge.Transaction; +import com.newrelic.agent.bridge.external.ExternalMetrics; import com.newrelic.agent.bridge.external.URISupport; +import com.newrelic.api.agent.ConcurrentHashMapHeaders; +import com.newrelic.api.agent.HeaderType; import com.newrelic.api.agent.HttpParameters; +import com.newrelic.api.agent.NewRelic; +import com.newrelic.api.agent.Transaction; -import java.io.IOException; import java.net.HttpURLConnection; import java.net.URI; -import java.net.URISyntaxException; public class MetricState { private static final String LIBRARY = "HttpURLConnection"; @@ -25,62 +27,72 @@ public class MetricState { public void nonNetworkPreamble(boolean isConnected, HttpURLConnection connection, String operation) { TracedMethod method = AgentBridge.getAgent().getTracedMethod(); - Transaction tx = AgentBridge.getAgent().getTransaction(false); + Transaction tx = NewRelic.getAgent().getTransaction(); if (!isConnected && method.isMetricProducer() && tx != null) { // This method doesn't have any network I/O so we are explicitly not recording external rollup metrics - makeMetric(connection, method, operation); - tx.getCrossProcessState().processOutboundRequestHeaders(new OutboundWrapper(connection), method); + makeNonRollupExternalMetric(connection, method, operation); + ConcurrentHashMapHeaders headers = ConcurrentHashMapHeaders.build(HeaderType.HTTP); + tx.insertDistributedTraceHeaders(headers); } } public void getInputStreamPreamble(boolean isConnected, HttpURLConnection connection, TracedMethod method) { - Transaction tx = AgentBridge.getAgent().getTransaction(false); + Transaction tx = NewRelic.getAgent().getTransaction(); if (method.isMetricProducer() && tx != null) { if (!recordedANetworkCall) { this.recordedANetworkCall = true; - makeMetric(connection, method, "getInputStream"); + makeNonRollupExternalMetric(connection, method, "getInputStream"); } if (!isConnected) { - tx.getCrossProcessState().processOutboundRequestHeaders(new OutboundWrapper(connection), method); + ConcurrentHashMapHeaders headers = ConcurrentHashMapHeaders.build(HeaderType.HTTP); + tx.insertDistributedTraceHeaders(headers); } } } public void getResponseCodePreamble(HttpURLConnection connection, TracedMethod method) { - Transaction tx = AgentBridge.getAgent().getTransaction(false); + Transaction tx = NewRelic.getAgent().getTransaction(); if (method.isMetricProducer() && tx != null && !recordedANetworkCall) { this.recordedANetworkCall = true; - makeMetric(connection, method, "getResponseCode"); + makeNonRollupExternalMetric(connection, method, "getResponseCode"); } } - public void getInboundPostamble(HttpURLConnection connection, TracedMethod method) { - Transaction tx = AgentBridge.getAgent().getTransaction(false); + public void getInboundPostamble(HttpURLConnection connection, int responseCode, String responseMessage, String requestMethod, TracedMethod method) { + Transaction tx = NewRelic.getAgent().getTransaction(); if (method.isMetricProducer() && !metricsRecorded && tx != null) { this.metricsRecorded = true; + // This conversion is necessary as it strips query parameters from the URI String uri = URISupport.getURI(connection.getURL()); - InboundWrapper inboundWrapper = new InboundWrapper(connection); - tx.getCrossProcessState() - .processInboundResponseHeaders(inboundWrapper, method, connection.getURL().getHost(), uri, true); - } - } - - private void makeMetric(HttpURLConnection connection, TracedMethod method, String operation) { - try { - URI uri = connection.getURL().toURI(); - int responseCode = connection.getResponseCode(); - String responseMessage = connection.getResponseMessage(); + // This will result in External rollup metrics being generated method.reportAsExternal(HttpParameters .library(LIBRARY) - .uri(uri) - .procedure(operation) + .uri(URI.create(uri)) + .procedure(requestMethod) .inboundHeaders(new InboundWrapper(connection)) .status(responseCode, responseMessage) .build()); - } catch (URISyntaxException | IOException e) { - throw new RuntimeException(e); } } + + /** + * Sets external metric name (i.e. External/{HOST}/HttpURLConnection). + * This does not create rollup metrics such as External/all, External/allWeb, External/allOther, External/{HOST}/all + * + * @param connection HttpURLConnection instance + * @param method TracedMethod instance + * @param operation String representation of operation + */ + private void makeNonRollupExternalMetric(HttpURLConnection connection, TracedMethod method, String operation) { + String uri = URISupport.getURI(connection.getURL()); + ExternalMetrics.makeExternalComponentMetric( + method, + connection.getURL().getHost(), + LIBRARY, + false, + uri, + operation); + } } diff --git a/instrumentation/httpurlconnection/src/main/java/java/net/HttpURLConnection.java b/instrumentation/httpurlconnection/src/main/java/java/net/HttpURLConnection.java index 53dd026e8f..b7dedf9db5 100644 --- a/instrumentation/httpurlconnection/src/main/java/java/net/HttpURLConnection.java +++ b/instrumentation/httpurlconnection/src/main/java/java/net/HttpURLConnection.java @@ -8,7 +8,6 @@ package java.net; import com.newrelic.agent.bridge.AgentBridge; -import com.newrelic.agent.bridge.TracedMethod; import com.newrelic.api.agent.Trace; import com.newrelic.api.agent.weaver.MatchType; import com.newrelic.api.agent.weaver.NewField; @@ -42,21 +41,19 @@ protected HttpURLConnection(URL url) { @Trace(leaf = true) public void connect() throws IOException { lazyGetMetricState().nonNetworkPreamble(connected, this, "connect"); - Weaver.callOriginal(); } @Trace(leaf = true) public synchronized OutputStream getOutputStream() throws IOException { lazyGetMetricState().nonNetworkPreamble(connected, this, "getOutputStream"); - return Weaver.callOriginal(); } @Trace(leaf = true) public synchronized InputStream getInputStream() throws IOException { MetricState metricState = lazyGetMetricState(); - TracedMethod method = AgentBridge.getAgent().getTracedMethod(); + com.newrelic.agent.bridge.TracedMethod method = AgentBridge.getAgent().getTracedMethod(); metricState.getInputStreamPreamble(connected, this, method); InputStream inputStream; @@ -71,15 +68,14 @@ public synchronized InputStream getInputStream() throws IOException { throw e; } - metricState.getInboundPostamble(this, method); - + metricState.getInboundPostamble(this, 0, null, "getInputStream", method); return inputStream; } @Trace(leaf = true) public int getResponseCode() throws Exception { MetricState metricState = lazyGetMetricState(); - TracedMethod method = AgentBridge.getAgent().getTracedMethod(); + com.newrelic.agent.bridge.TracedMethod method = AgentBridge.getAgent().getTracedMethod(); metricState.getResponseCodePreamble(this, method); int responseCodeValue; @@ -94,8 +90,7 @@ public int getResponseCode() throws Exception { throw e; } - metricState.getInboundPostamble(this, method); - + metricState.getInboundPostamble(this, responseCodeValue, null, "getResponseCode", method); return responseCodeValue; } diff --git a/instrumentation/httpurlconnection/src/test/java/com/nr/agent/instrumentation/httpurlconnection/MetricStateResponseCodeTest.java b/instrumentation/httpurlconnection/src/test/java/com/nr/agent/instrumentation/httpurlconnection/MetricStateResponseCodeTest.java index 0c73a03036..1939c342e2 100644 --- a/instrumentation/httpurlconnection/src/test/java/com/nr/agent/instrumentation/httpurlconnection/MetricStateResponseCodeTest.java +++ b/instrumentation/httpurlconnection/src/test/java/com/nr/agent/instrumentation/httpurlconnection/MetricStateResponseCodeTest.java @@ -8,18 +8,14 @@ package com.nr.agent.instrumentation.httpurlconnection; import com.newrelic.agent.bridge.AgentBridge; -import com.newrelic.agent.introspec.HttpTestServer; import com.newrelic.agent.introspec.InstrumentationTestConfig; import com.newrelic.agent.introspec.InstrumentationTestRunner; import com.newrelic.agent.introspec.Introspector; import com.newrelic.agent.introspec.TraceSegment; import com.newrelic.agent.introspec.TransactionEvent; import com.newrelic.agent.introspec.TransactionTrace; -import com.newrelic.agent.introspec.internal.HttpServerLocator; import com.newrelic.agent.introspec.internal.HttpServerRule; import com.newrelic.api.agent.Trace; -import org.junit.After; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -67,8 +63,8 @@ private TraceSegment runAndVerifyFirstCall(MetricState target, int nCalls) throw String transactionName = fetchTransactionName(introspector, "callGetResponseCode"); // We only have one call to this external metric. - assertTrue(introspector.getMetricsForTransaction(transactionName).containsKey("External/localhost/HttpURLConnection")); - assertEquals(1, introspector.getMetricsForTransaction(transactionName).get("External/localhost/HttpURLConnection").getCallCount()); + assertTrue(introspector.getMetricsForTransaction(transactionName).containsKey("External/localhost/HttpURLConnection/getResponseCode")); + assertEquals(1, introspector.getMetricsForTransaction(transactionName).get("External/localhost/HttpURLConnection/getResponseCode").getCallCount()); // The number of segments is equal to the number of calls to getResponseCode. TransactionTrace trace = introspector.getTransactionTracesForTransaction(transactionName).iterator().next(); @@ -94,8 +90,8 @@ public void shouldBumpMetricOnceIfAlreadyHaveResponseCode() throws Exception { // Only one external call. String transactionName = fetchTransactionName(introspector, "callGetResponseCodeThenGetInputStream"); - assertTrue(introspector.getMetricsForTransaction(transactionName).containsKey("External/localhost/HttpURLConnection")); - assertEquals(1, introspector.getMetricsForTransaction(transactionName).get("External/localhost/HttpURLConnection").getCallCount()); + assertTrue(introspector.getMetricsForTransaction(transactionName).containsKey("External/localhost/HttpURLConnection/getResponseCode")); + assertEquals(1, introspector.getMetricsForTransaction(transactionName).get("External/localhost/HttpURLConnection/getResponseCode").getCallCount()); // Two child segments within the transaction. TransactionTrace trace = introspector.getTransactionTracesForTransaction(transactionName).iterator().next(); @@ -125,8 +121,8 @@ public void shouldBumpMetricOnceIfAlreadyHaveInputStream() throws Exception { // Only one external call. String transactionName = fetchTransactionName(introspector, "callGetInputStreamThenResponseCode"); - assertTrue(introspector.getMetricsForTransaction(transactionName).containsKey("External/localhost/HttpURLConnection")); - assertEquals(1, introspector.getMetricsForTransaction(transactionName).get("External/localhost/HttpURLConnection").getCallCount()); + assertTrue(introspector.getMetricsForTransaction(transactionName).containsKey("External/localhost/HttpURLConnection/getInputStream")); + assertEquals(1, introspector.getMetricsForTransaction(transactionName).get("External/localhost/HttpURLConnection/getInputStream").getCallCount()); // Two child segments within the transaction. TransactionTrace trace = introspector.getTransactionTracesForTransaction(transactionName).iterator().next(); @@ -204,14 +200,12 @@ public void callGetInputStreamThenResponseCode(MetricState target) throws Except @Trace(leaf = true) private void simulatedInstrumentedGetResponseCodeMethod(HttpURLConnection conn, MetricState target) { target.getResponseCodePreamble(conn, AgentBridge.getAgent().getTracedMethod()); - - target.getInboundPostamble(conn, AgentBridge.getAgent().getTracedMethod()); + target.getInboundPostamble(conn, 0, null, "getResponseCode", AgentBridge.getAgent().getTracedMethod()); } @Trace(leaf = true) private void simulatedInstrumentedGetInputStreamMethod(boolean isConnected, HttpURLConnection conn, MetricState target) { target.getInputStreamPreamble(isConnected, conn, AgentBridge.getAgent().getTracedMethod()); - - target.getInboundPostamble(conn, AgentBridge.getAgent().getTracedMethod()); + target.getInboundPostamble(conn, 0, null, "getInputStream", AgentBridge.getAgent().getTracedMethod()); } } From a78c5b3905c504642c686630bb76fb35ed5519ab Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Thu, 16 Jun 2022 15:42:56 -0700 Subject: [PATCH 03/19] Add qualified import --- .../src/main/java/java/net/HttpURLConnection.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/instrumentation/httpurlconnection/src/main/java/java/net/HttpURLConnection.java b/instrumentation/httpurlconnection/src/main/java/java/net/HttpURLConnection.java index b7dedf9db5..72a1a34ebc 100644 --- a/instrumentation/httpurlconnection/src/main/java/java/net/HttpURLConnection.java +++ b/instrumentation/httpurlconnection/src/main/java/java/net/HttpURLConnection.java @@ -8,6 +8,7 @@ package java.net; import com.newrelic.agent.bridge.AgentBridge; +import com.newrelic.agent.bridge.TracedMethod; import com.newrelic.api.agent.Trace; import com.newrelic.api.agent.weaver.MatchType; import com.newrelic.api.agent.weaver.NewField; @@ -53,7 +54,7 @@ public synchronized OutputStream getOutputStream() throws IOException { @Trace(leaf = true) public synchronized InputStream getInputStream() throws IOException { MetricState metricState = lazyGetMetricState(); - com.newrelic.agent.bridge.TracedMethod method = AgentBridge.getAgent().getTracedMethod(); + TracedMethod method = AgentBridge.getAgent().getTracedMethod(); metricState.getInputStreamPreamble(connected, this, method); InputStream inputStream; @@ -75,7 +76,7 @@ public synchronized InputStream getInputStream() throws IOException { @Trace(leaf = true) public int getResponseCode() throws Exception { MetricState metricState = lazyGetMetricState(); - com.newrelic.agent.bridge.TracedMethod method = AgentBridge.getAgent().getTracedMethod(); + TracedMethod method = AgentBridge.getAgent().getTracedMethod(); metricState.getResponseCodePreamble(this, method); int responseCodeValue; From 9dc22bc66fc34848c1380c46ede9f2b805c1aa0a Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Thu, 16 Jun 2022 19:19:25 -0700 Subject: [PATCH 04/19] Fix HttpURLConnectionTest --- .../pointcuts/net/HttpURLConnectionTest.java | 101 +++++++++++------- 1 file changed, 63 insertions(+), 38 deletions(-) diff --git a/functional_test/src/test/java/com/newrelic/agent/instrumentation/pointcuts/net/HttpURLConnectionTest.java b/functional_test/src/test/java/com/newrelic/agent/instrumentation/pointcuts/net/HttpURLConnectionTest.java index 4f1827810e..8b1ad9593f 100644 --- a/functional_test/src/test/java/com/newrelic/agent/instrumentation/pointcuts/net/HttpURLConnectionTest.java +++ b/functional_test/src/test/java/com/newrelic/agent/instrumentation/pointcuts/net/HttpURLConnectionTest.java @@ -7,8 +7,16 @@ package com.newrelic.agent.instrumentation.pointcuts.net; -import static java.text.MessageFormat.format; -import static org.junit.Assert.assertEquals; +import com.google.common.collect.Sets; +import com.newrelic.agent.AgentHelper; +import com.newrelic.agent.TransactionDataList; +import com.newrelic.agent.metric.MetricName; +import com.newrelic.agent.service.ServiceFactory; +import com.newrelic.api.agent.Trace; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; import java.io.BufferedReader; import java.io.BufferedWriter; @@ -23,16 +31,8 @@ import java.util.Map; import java.util.Set; -import com.google.common.collect.Sets; -import com.newrelic.agent.AgentHelper; -import com.newrelic.agent.TransactionDataList; -import com.newrelic.agent.metric.MetricName; -import com.newrelic.agent.service.ServiceFactory; -import com.newrelic.api.agent.Trace; -import org.junit.Assert; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.Test; +import static java.text.MessageFormat.format; +import static org.junit.Assert.assertEquals; public class HttpURLConnectionTest { @@ -489,43 +489,68 @@ private void verifyMetrics(String url, String scope, int scopedHttpUrlCount, int int scopedNetworkIOCount, int unscopedNetworkIOCount) { Set metrics = AgentHelper.getMetrics(); + String httpURLConnectionMetric = "External/{0}/HttpURLConnection"; + String httpURLConnectionGetInputStreamMetric = "External/{0}/HttpURLConnection/getInputStream"; + String httpURLConnectionGetResponseCodeMetric = "External/{0}/HttpURLConnection/getResponseCode"; + String externalHostAllMetric = "External/{0}/all"; + String externalAllMetric = "External/all"; + String externalAllOtherMetric = "External/allOther"; + if (scopedHttpUrlCount > 0 || unscopedHttpUrlCount > 0) { - Assert.assertTrue(metrics.toString(), metrics.contains(format("External/{0}/HttpURLConnection", url))); + Assert.assertTrue(metrics.toString(), + metrics.contains(format(httpURLConnectionMetric, url)) || + metrics.contains(format(httpURLConnectionGetInputStreamMetric, url)) || + metrics.contains(format(httpURLConnectionGetResponseCodeMetric, url))); } else { - Assert.assertFalse(metrics.toString(), metrics.contains(format("External/{0}/HttpURLConnection", url))); + Assert.assertFalse(metrics.toString(), + metrics.contains(format(httpURLConnectionMetric, url)) || + metrics.contains(format(httpURLConnectionGetInputStreamMetric, url)) || + metrics.contains(format(httpURLConnectionGetResponseCodeMetric, url))); } if (scopedNetworkIOCount > 0 || unscopedNetworkIOCount > 0) { - Assert.assertTrue(metrics.toString(), metrics.contains(format("External/{0}/all", url))); - Assert.assertTrue(metrics.toString(), metrics.contains("External/all")); - Assert.assertTrue(metrics.toString(), metrics.contains("External/allOther")); + Assert.assertTrue(metrics.toString(), metrics.contains(format(externalHostAllMetric, url))); + Assert.assertTrue(metrics.toString(), metrics.contains(externalAllMetric)); + Assert.assertTrue(metrics.toString(), metrics.contains(externalAllOtherMetric)); } else { - Assert.assertFalse(metrics.toString(), metrics.contains(format("External/{0}/all", url))); - Assert.assertFalse(metrics.toString(), metrics.contains("External/all")); - Assert.assertFalse(metrics.toString(), metrics.contains("External/allOther")); + Assert.assertFalse(metrics.toString(), metrics.contains(format(externalHostAllMetric, url))); + Assert.assertFalse(metrics.toString(), metrics.contains(externalAllMetric)); + Assert.assertFalse(metrics.toString(), metrics.contains(externalAllOtherMetric)); } Map scopedMetrics = getMetricCounts( - MetricName.create(format("External/{0}/HttpURLConnection", url), scope), - MetricName.create(format("External/{0}/all", url), scope), - MetricName.create("External/all", scope), - MetricName.create("External/allOther", scope)); + MetricName.create(format(httpURLConnectionMetric, url), scope), + MetricName.create(format(httpURLConnectionGetInputStreamMetric, url), scope), + MetricName.create(format(httpURLConnectionGetResponseCodeMetric, url), scope), + MetricName.create(format(externalHostAllMetric, url), scope), + MetricName.create(externalAllMetric, scope), + MetricName.create(externalAllOtherMetric, scope)); Map unscopedMetrics = getMetricCounts( - MetricName.create(format("External/{0}/HttpURLConnection", url)), - MetricName.create(format("External/{0}/all", url)), - MetricName.create("External/all"), - MetricName.create("External/allOther")); - - assertEquals(scopedHttpUrlCount, (int) scopedMetrics.get(format("External/{0}/HttpURLConnection", url))); - assertEquals(scopedNetworkIOCount, (int) scopedMetrics.get(format("External/{0}/all", url))); - assertEquals(scopedNetworkIOCount, (int) scopedMetrics.get("External/all")); - assertEquals(scopedNetworkIOCount, (int) scopedMetrics.get("External/allOther")); - - assertEquals(unscopedHttpUrlCount, (int) unscopedMetrics.get(format("External/{0}/HttpURLConnection", url))); - assertEquals(unscopedNetworkIOCount, (int) unscopedMetrics.get(format("External/{0}/all", url))); - assertEquals(unscopedNetworkIOCount, (int) unscopedMetrics.get("External/all")); - assertEquals(unscopedNetworkIOCount, (int) unscopedMetrics.get("External/allOther")); + MetricName.create(format(httpURLConnectionMetric, url)), + MetricName.create(format(httpURLConnectionGetInputStreamMetric, url)), + MetricName.create(format(httpURLConnectionGetResponseCodeMetric, url)), + MetricName.create(format(externalHostAllMetric, url)), + MetricName.create(externalAllMetric), + MetricName.create(externalAllOtherMetric)); + + int actualHttpURLConnectionScopedMetricCount = scopedMetrics.get(format(httpURLConnectionMetric, url)) + + scopedMetrics.get(format(httpURLConnectionGetInputStreamMetric, url)) + + scopedMetrics.get(format(httpURLConnectionGetResponseCodeMetric, url)); + + int actualHttpURLConnectionUnscopedMetricCount = unscopedMetrics.get(format(httpURLConnectionMetric, url)) + + unscopedMetrics.get(format(httpURLConnectionGetInputStreamMetric, url)) + + unscopedMetrics.get(format(httpURLConnectionGetResponseCodeMetric, url)); + + assertEquals(scopedHttpUrlCount, actualHttpURLConnectionScopedMetricCount); + assertEquals(scopedNetworkIOCount, (int) scopedMetrics.get(format(externalHostAllMetric, url))); + assertEquals(scopedNetworkIOCount, (int) scopedMetrics.get(externalAllMetric)); + assertEquals(scopedNetworkIOCount, (int) scopedMetrics.get(externalAllOtherMetric)); + + assertEquals(unscopedHttpUrlCount, actualHttpURLConnectionUnscopedMetricCount); + assertEquals(unscopedNetworkIOCount, (int) unscopedMetrics.get(format(externalHostAllMetric, url))); + assertEquals(unscopedNetworkIOCount, (int) unscopedMetrics.get(externalAllMetric)); + assertEquals(unscopedNetworkIOCount, (int) unscopedMetrics.get(externalAllOtherMetric)); } private Map getMetricCounts(MetricName... responseTimeMetricNames) { From 64401b9f6646eb1fc65172472cc0bc54c3c1b7f1 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Thu, 16 Jun 2022 19:57:34 -0700 Subject: [PATCH 05/19] Fix HttpCommonsTest --- .../test/java/test/newrelic/test/agent/HttpCommonsTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/functional_test/src/test/java/test/newrelic/test/agent/HttpCommonsTest.java b/functional_test/src/test/java/test/newrelic/test/agent/HttpCommonsTest.java index f63fed0348..b60341294b 100644 --- a/functional_test/src/test/java/test/newrelic/test/agent/HttpCommonsTest.java +++ b/functional_test/src/test/java/test/newrelic/test/agent/HttpCommonsTest.java @@ -65,13 +65,13 @@ public void httpURLConnection() throws Exception { httpURLConnectionTx(); Set metrics = AgentHelper.getMetrics(); - assertTrue(metrics.toString(), metrics.contains("External/" + HOST + "/HttpURLConnection")); + assertTrue(metrics.toString(), metrics.contains("External/" + HOST + "/HttpURLConnection/getResponseCode")); Map metricCounts = getMetricCounts( - MetricName.create("External/" + HOST + "/HttpURLConnection", + MetricName.create("External/" + HOST + "/HttpURLConnection/getResponseCode", "OtherTransaction/Custom/test.newrelic.test.agent.HttpCommonsTest/httpURLConnectionTx")); - assertEquals(1, (int) metricCounts.get("External/" + HOST + "/HttpURLConnection")); + assertEquals(1, (int) metricCounts.get("External/" + HOST + "/HttpURLConnection/getResponseCode")); } @Trace(dispatcher = true) From 877847abf382d66a0c985f4f40fb4995c8927346 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Thu, 16 Jun 2022 20:53:43 -0700 Subject: [PATCH 06/19] Add outbound CAT headers --- .../httpurlconnection/MetricState.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java b/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java index 7b2caa5b29..25259e2e01 100644 --- a/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java +++ b/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java @@ -9,13 +9,12 @@ import com.newrelic.agent.bridge.AgentBridge; import com.newrelic.agent.bridge.TracedMethod; +import com.newrelic.agent.bridge.Transaction; import com.newrelic.agent.bridge.external.ExternalMetrics; import com.newrelic.agent.bridge.external.URISupport; import com.newrelic.api.agent.ConcurrentHashMapHeaders; import com.newrelic.api.agent.HeaderType; import com.newrelic.api.agent.HttpParameters; -import com.newrelic.api.agent.NewRelic; -import com.newrelic.api.agent.Transaction; import java.net.HttpURLConnection; import java.net.URI; @@ -27,8 +26,10 @@ public class MetricState { public void nonNetworkPreamble(boolean isConnected, HttpURLConnection connection, String operation) { TracedMethod method = AgentBridge.getAgent().getTracedMethod(); - Transaction tx = NewRelic.getAgent().getTransaction(); + Transaction tx = AgentBridge.getAgent().getTransaction(false); if (!isConnected && method.isMetricProducer() && tx != null) { + // Add outbound CAT headers + tx.getCrossProcessState().processOutboundRequestHeaders(new OutboundWrapper(connection), method); // This method doesn't have any network I/O so we are explicitly not recording external rollup metrics makeNonRollupExternalMetric(connection, method, operation); ConcurrentHashMapHeaders headers = ConcurrentHashMapHeaders.build(HeaderType.HTTP); @@ -37,7 +38,7 @@ public void nonNetworkPreamble(boolean isConnected, HttpURLConnection connection } public void getInputStreamPreamble(boolean isConnected, HttpURLConnection connection, TracedMethod method) { - Transaction tx = NewRelic.getAgent().getTransaction(); + Transaction tx = AgentBridge.getAgent().getTransaction(false); if (method.isMetricProducer() && tx != null) { if (!recordedANetworkCall) { this.recordedANetworkCall = true; @@ -45,6 +46,8 @@ public void getInputStreamPreamble(boolean isConnected, HttpURLConnection connec } if (!isConnected) { + // Add outbound CAT headers + tx.getCrossProcessState().processOutboundRequestHeaders(new OutboundWrapper(connection), method); ConcurrentHashMapHeaders headers = ConcurrentHashMapHeaders.build(HeaderType.HTTP); tx.insertDistributedTraceHeaders(headers); } @@ -52,7 +55,7 @@ public void getInputStreamPreamble(boolean isConnected, HttpURLConnection connec } public void getResponseCodePreamble(HttpURLConnection connection, TracedMethod method) { - Transaction tx = NewRelic.getAgent().getTransaction(); + Transaction tx = AgentBridge.getAgent().getTransaction(false); if (method.isMetricProducer() && tx != null && !recordedANetworkCall) { this.recordedANetworkCall = true; makeNonRollupExternalMetric(connection, method, "getResponseCode"); @@ -60,7 +63,7 @@ public void getResponseCodePreamble(HttpURLConnection connection, TracedMethod m } public void getInboundPostamble(HttpURLConnection connection, int responseCode, String responseMessage, String requestMethod, TracedMethod method) { - Transaction tx = NewRelic.getAgent().getTransaction(); + Transaction tx = AgentBridge.getAgent().getTransaction(false); if (method.isMetricProducer() && !metricsRecorded && tx != null) { this.metricsRecorded = true; // This conversion is necessary as it strips query parameters from the URI From fbdb11d126fba0859586bf72b4b2630a4fa8a9f1 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Tue, 21 Jun 2022 16:49:46 -0700 Subject: [PATCH 07/19] Correctly add CAT/Distributed tracing headers to outbound request --- .../httpurlconnection/MetricState.java | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java b/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java index 25259e2e01..bc68e23535 100644 --- a/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java +++ b/instrumentation/httpurlconnection/src/main/java/com/nr/agent/instrumentation/httpurlconnection/MetricState.java @@ -12,8 +12,6 @@ import com.newrelic.agent.bridge.Transaction; import com.newrelic.agent.bridge.external.ExternalMetrics; import com.newrelic.agent.bridge.external.URISupport; -import com.newrelic.api.agent.ConcurrentHashMapHeaders; -import com.newrelic.api.agent.HeaderType; import com.newrelic.api.agent.HttpParameters; import java.net.HttpURLConnection; @@ -28,12 +26,10 @@ public void nonNetworkPreamble(boolean isConnected, HttpURLConnection connection TracedMethod method = AgentBridge.getAgent().getTracedMethod(); Transaction tx = AgentBridge.getAgent().getTransaction(false); if (!isConnected && method.isMetricProducer() && tx != null) { - // Add outbound CAT headers - tx.getCrossProcessState().processOutboundRequestHeaders(new OutboundWrapper(connection), method); // This method doesn't have any network I/O so we are explicitly not recording external rollup metrics makeNonRollupExternalMetric(connection, method, operation); - ConcurrentHashMapHeaders headers = ConcurrentHashMapHeaders.build(HeaderType.HTTP); - tx.insertDistributedTraceHeaders(headers); + // Add CAT/Distributed tracing headers to this outbound request + method.addOutboundRequestHeaders(new OutboundWrapper(connection)); } } @@ -46,10 +42,8 @@ public void getInputStreamPreamble(boolean isConnected, HttpURLConnection connec } if (!isConnected) { - // Add outbound CAT headers - tx.getCrossProcessState().processOutboundRequestHeaders(new OutboundWrapper(connection), method); - ConcurrentHashMapHeaders headers = ConcurrentHashMapHeaders.build(HeaderType.HTTP); - tx.insertDistributedTraceHeaders(headers); + // Add CAT/Distributed tracing headers to this outbound request + method.addOutboundRequestHeaders(new OutboundWrapper(connection)); } } } @@ -68,13 +62,17 @@ public void getInboundPostamble(HttpURLConnection connection, int responseCode, this.metricsRecorded = true; // This conversion is necessary as it strips query parameters from the URI String uri = URISupport.getURI(connection.getURL()); + InboundWrapper inboundWrapper = new InboundWrapper(connection); + + // Add CAT/Distributed tracing headers to this outbound request + method.addOutboundRequestHeaders(new OutboundWrapper(connection)); // This will result in External rollup metrics being generated method.reportAsExternal(HttpParameters .library(LIBRARY) .uri(URI.create(uri)) .procedure(requestMethod) - .inboundHeaders(new InboundWrapper(connection)) + .inboundHeaders(inboundWrapper) .status(responseCode, responseMessage) .build()); } From 81c521b12b942779140991dd3ce65381896c6227 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Tue, 21 Jun 2022 17:01:49 -0700 Subject: [PATCH 08/19] Clarify Javadoc comment on deprecated API --- .../src/main/java/com/newrelic/api/agent/TracedMethod.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/newrelic-api/src/main/java/com/newrelic/api/agent/TracedMethod.java b/newrelic-api/src/main/java/com/newrelic/api/agent/TracedMethod.java index d5207ee0e0..a9006a61c7 100644 --- a/newrelic-api/src/main/java/com/newrelic/api/agent/TracedMethod.java +++ b/newrelic-api/src/main/java/com/newrelic/api/agent/TracedMethod.java @@ -70,7 +70,8 @@ public interface TracedMethod extends AttributeHolder { * determines if the external call is HTTP or JMS. * @since 3.36.0 * @deprecated Instead, use the Distributed Tracing API {@link Transaction#insertDistributedTraceHeaders(Headers)} to create a - * distributed tracing payload + * distributed tracing payload. However, note that the insertDistributedTraceHeaders API only adds distributed tracing + * headers and does not add legacy CAT headers. If CAT must be supported then instead use this deprecated API. */ @Deprecated void addOutboundRequestHeaders(OutboundHeaders outboundHeaders); From d879e6b3e19f41929cdfd7ca58e78f72d1c159da Mon Sep 17 00:00:00 2001 From: Gerard Downes Date: Mon, 27 Jun 2022 15:21:04 +0100 Subject: [PATCH 09/19] Add additional client and framework AIT tests to workflows --- .github/workflows/AITs-Clients.yml | 1 + .github/workflows/AITs-Frameworks.yml | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/.github/workflows/AITs-Clients.yml b/.github/workflows/AITs-Clients.yml index 968efdc33b..36dda6dfce 100644 --- a/.github/workflows/AITs-Clients.yml +++ b/.github/workflows/AITs-Clients.yml @@ -33,6 +33,7 @@ jobs: - sttp_2.py - sttp_3.py - http4s.py + - play_ws_2_6 steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/AITs-Frameworks.yml b/.github/workflows/AITs-Frameworks.yml index 2befb2e55b..a7d92b4774 100644 --- a/.github/workflows/AITs-Frameworks.yml +++ b/.github/workflows/AITs-Frameworks.yml @@ -35,10 +35,13 @@ jobs: func_tests: - akka/akka.py - akka/akkahttp.py + - cats/cats2.py + - cats/cats3.py # - ejb/ejb.py - jaxrs/jaxrs.py # - jms/jms.py - lettuce/lettuce.py + - monix/monix.py # - netty/netty.py # - play/play2.py - quartz/quartz.py @@ -46,6 +49,7 @@ jobs: - spring/spring_5_0.py - spring/spring_webflux.py - thrift/thrift.py + - zio/zio.py steps: - uses: actions/checkout@v2 From 1cfaf03647ed2035910ebced469bf8c4fd7b8d1a Mon Sep 17 00:00:00 2001 From: Gerard Downes Date: Mon, 27 Jun 2022 17:25:01 +0100 Subject: [PATCH 10/19] Add MSSQL R2DBC to workflow --- .github/workflows/AITs-R2DBC.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/AITs-R2DBC.yml b/.github/workflows/AITs-R2DBC.yml index cf6a06fe12..81a40931d6 100644 --- a/.github/workflows/AITs-R2DBC.yml +++ b/.github/workflows/AITs-R2DBC.yml @@ -34,6 +34,7 @@ jobs: - oracle.py - postgres.py - mysql.py + - mssql.py - h2.py steps: - uses: actions/checkout@v2 From dd9366793eb6e2ab9d254c19bf31b271a666f585 Mon Sep 17 00:00:00 2001 From: Gerard Downes Date: Mon, 27 Jun 2022 17:41:15 +0100 Subject: [PATCH 11/19] Fix Play WS client test name in workflow --- .github/workflows/AITs-Clients.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/AITs-Clients.yml b/.github/workflows/AITs-Clients.yml index 36dda6dfce..f83e6c7879 100644 --- a/.github/workflows/AITs-Clients.yml +++ b/.github/workflows/AITs-Clients.yml @@ -33,7 +33,7 @@ jobs: - sttp_2.py - sttp_3.py - http4s.py - - play_ws_2_6 + - play_ws_2_6.py steps: - uses: actions/checkout@v2 From 439df1a05b37fca37e544d32c124c8bd25480454 Mon Sep 17 00:00:00 2001 From: Gerard Downes Date: Tue, 28 Jun 2022 14:39:21 +0100 Subject: [PATCH 12/19] Add logging flag for testing --- .github/workflows/AITs-R2DBC.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/AITs-R2DBC.yml b/.github/workflows/AITs-R2DBC.yml index 81a40931d6..70ea9907d2 100644 --- a/.github/workflows/AITs-R2DBC.yml +++ b/.github/workflows/AITs-R2DBC.yml @@ -24,7 +24,7 @@ jobs: env: java_func_path: "tests/java/functionality" java_func_type: "r2dbc" - default-branch: "main" + default-branch: "ait-logging" strategy: ##max-parallel: 1 ## used to force sequential fail-fast: false From ee852632304cb72405da66e0bb21f85548d33606 Mon Sep 17 00:00:00 2001 From: Gerard Downes Date: Wed, 29 Jun 2022 10:16:58 +0100 Subject: [PATCH 13/19] Remove mssql test --- .github/workflows/AITs-R2DBC.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/AITs-R2DBC.yml b/.github/workflows/AITs-R2DBC.yml index 70ea9907d2..6636c2431c 100644 --- a/.github/workflows/AITs-R2DBC.yml +++ b/.github/workflows/AITs-R2DBC.yml @@ -24,7 +24,7 @@ jobs: env: java_func_path: "tests/java/functionality" java_func_type: "r2dbc" - default-branch: "ait-logging" + default-branch: "main" strategy: ##max-parallel: 1 ## used to force sequential fail-fast: false @@ -34,7 +34,7 @@ jobs: - oracle.py - postgres.py - mysql.py - - mssql.py +# - mssql.py - h2.py steps: - uses: actions/checkout@v2 From 898cb87279bf6aaba8b27b7f8c5b011f8fd6627b Mon Sep 17 00:00:00 2001 From: tbradellis Date: Wed, 22 Jun 2022 14:49:14 -0700 Subject: [PATCH 14/19] include debug as env clean code. no null guard needed clean configfilehelper and settings.gradle fix comment --- newrelic-agent/src/main/java/com/newrelic/agent/Agent.java | 7 ++++++- .../java/com/newrelic/agent/config/AgentConfigImpl.java | 2 +- .../java/com/newrelic/agent/config/AgentJarHelper.java | 4 ++-- .../java/com/newrelic/agent/config/ConfigFileHelper.java | 4 +++- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/Agent.java b/newrelic-agent/src/main/java/com/newrelic/agent/Agent.java index 2fdb444c08..53cd224caa 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/Agent.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/Agent.java @@ -51,7 +51,7 @@ public final class Agent { private static final String NEWRELIC_BOOTSTRAP = "newrelic-bootstrap"; private static final String AGENT_ENABLED_PROPERTY = "newrelic.config.agent_enabled"; - private static final boolean DEBUG = Boolean.getBoolean("newrelic.debug"); + private static final boolean DEBUG = Agent.setDebug(); private static final String VERSION = Agent.initVersion(); private static long agentPremainTime; @@ -76,6 +76,11 @@ public static boolean isDebugEnabled() { return DEBUG; } + private static boolean setDebug(){ + return Boolean.getBoolean("newrelic.debug") || Boolean.parseBoolean(System.getenv("NEWRELIC_DEBUG")); + } + + private static volatile boolean canFastPath = true; /** diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java index 7c36b2341a..37e351fabe 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java @@ -293,7 +293,7 @@ private AgentConfigImpl(Map props) { putForDataSend = getProperty(PUT_FOR_DATA_SEND_PROPERTY, DEFAULT_PUT_FOR_DATA_SEND_ENABLED); isApdexTSet = getProperty(APDEX_T) != null; apdexTInMillis = (long) (getDoubleProperty(APDEX_T, DEFAULT_APDEX_T) * 1000L); - debug = Boolean.getBoolean(DEBUG); + debug = Boolean.getBoolean(DEBUG) || Boolean.parseBoolean(System.getenv("NEWRELIC_DEBUG")); metricDebug = initMetricDebugConfig(); enabled = getProperty(ENABLED, DEFAULT_ENABLED) && getProperty(AGENT_ENABLED, DEFAULT_ENABLED); experimentalRuntime = allowExperimentalRuntimeVersions(); diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentJarHelper.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentJarHelper.java index d9072e3f74..2dd480d374 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentJarHelper.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentJarHelper.java @@ -223,12 +223,12 @@ public static String getAgentJarAttribute(String name) { } } - // The "newrelic.debug" flag redirects all Agent logging to the standard output. Unfortunately, + // The "newrelic.debug" flag redirects all Agent logging (prior to log init) to the standard output -after log init, log as usual. Unfortunately, // we haven't initialized the Agent yet, so we cannot check it in the usual low-cost way by // calling Agent.isDebugEnabled(). So we duplicate the functionality here for use in a few cases. private static final boolean isNewRelicDebug() { final String newrelicDebug = "newrelic.debug"; - return System.getProperty(newrelicDebug) != null && Boolean.getBoolean(newrelicDebug); + return Boolean.getBoolean(newrelicDebug) || Boolean.parseBoolean(System.getenv("NEWRELIC_DEBUG")); } // Use of this method should be limited to serious error cases that would cause the Agent to diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigFileHelper.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigFileHelper.java index 4dd27e3fde..30e638c0a4 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigFileHelper.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigFileHelper.java @@ -21,6 +21,8 @@ public class ConfigFileHelper { private static final String NEW_RELIC_HOME_DIRECTORY_PROPERTY = "newrelic.home"; private static final String NEW_RELIC_HOME_DIRECTORY_ENVIRONMENT_VARIABLE = "NEWRELIC_HOME"; private static final String NEW_RELIC_DEBUG_PROPERTY = "newrelic.debug"; + + private static final String NEW_RELIC_DEBUG_ENV = "NEWRELIC_DEBUG"; private static final String[] SEARCH_DIRECTORIES = { ".", "conf", "config", "etc" }; /** @@ -36,7 +38,7 @@ public static File findConfigFile() { File parentDir = getNewRelicDirectory(); if (parentDir != null) { - if (Boolean.getBoolean(NEW_RELIC_DEBUG_PROPERTY)) { + if (Boolean.getBoolean(NEW_RELIC_DEBUG_PROPERTY) || Boolean.parseBoolean(System.getenv(NEW_RELIC_DEBUG_ENV)) ) { System.err.println(MessageFormat.format("New Relic home directory: {0}", parentDir)); } } From 104212d8d5ec50703d3bae970368a744ce16d902 Mon Sep 17 00:00:00 2001 From: tbradellis Date: Mon, 27 Jun 2022 12:57:03 -0700 Subject: [PATCH 15/19] refactor DebugFlag --- .../src/main/java/com/newrelic/agent/Agent.java | 9 +++------ .../src/main/java/com/newrelic/agent/DebugFlag.java | 8 ++++++++ .../com/newrelic/agent/config/AgentConfigImpl.java | 4 ++-- .../java/com/newrelic/agent/config/AgentJarHelper.java | 10 +++++----- .../com/newrelic/agent/config/ConfigFileHelper.java | 10 +++++----- settings.gradle | 2 +- 6 files changed, 24 insertions(+), 19 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/Agent.java b/newrelic-agent/src/main/java/com/newrelic/agent/Agent.java index 53cd224caa..0535480de9 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/Agent.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/Agent.java @@ -22,6 +22,7 @@ import com.newrelic.agent.util.asm.ClassStructure; import com.newrelic.bootstrap.BootstrapLoader; import com.newrelic.weave.utils.Streams; +import com.sun.org.apache.xpath.internal.operations.Bool; import org.objectweb.asm.ClassReader; import java.io.ByteArrayOutputStream; @@ -32,6 +33,7 @@ import java.net.URL; import java.text.MessageFormat; import java.util.*; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.jar.*; import java.util.logging.Level; import java.util.regex.Pattern; @@ -51,7 +53,6 @@ public final class Agent { private static final String NEWRELIC_BOOTSTRAP = "newrelic-bootstrap"; private static final String AGENT_ENABLED_PROPERTY = "newrelic.config.agent_enabled"; - private static final boolean DEBUG = Agent.setDebug(); private static final String VERSION = Agent.initVersion(); private static long agentPremainTime; @@ -73,11 +74,7 @@ private static String initVersion() { } public static boolean isDebugEnabled() { - return DEBUG; - } - - private static boolean setDebug(){ - return Boolean.getBoolean("newrelic.debug") || Boolean.parseBoolean(System.getenv("NEWRELIC_DEBUG")); + return DebugFlag.DEBUG; } diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/DebugFlag.java b/newrelic-agent/src/main/java/com/newrelic/agent/DebugFlag.java index 7c9775cdad..67b02c86ab 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/DebugFlag.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/DebugFlag.java @@ -11,4 +11,12 @@ public class DebugFlag { public static final AtomicBoolean tokenEnabled = new AtomicBoolean(); + + // This replaces a previous check that duplicated code in several places. We need to check the debug flag in AgentJarHelper before the Agent class has been loaded. + // This flag cannot be set via newrelic.yml (AgentConfigImpl) because a ServiceManager and ConfigService have not been initialized for the earliest checks + // for the debug setting. + public static final boolean DEBUG = Boolean.getBoolean("newrelic.debug") || Boolean.parseBoolean(System.getenv("NEWRELIC_DEBUG")); + ; + + } diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java index 37e351fabe..a955360667 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java @@ -9,6 +9,7 @@ import com.google.common.base.Joiner; import com.newrelic.agent.Agent; +import com.newrelic.agent.DebugFlag; import com.newrelic.agent.transaction.TransactionNamingScheme; import com.newrelic.agent.transport.DataSenderImpl; @@ -69,7 +70,6 @@ public class AgentConfigImpl extends BaseConfig implements AgentConfig { public static final String MAX_STACK_TRACE_LINES = "max_stack_trace_lines"; public static final String METRIC_INGEST_URI = "metric_ingest_uri"; public static final String EVENT_INGEST_URI = "event_ingest_uri"; - public static final String DEBUG = "newrelic.debug"; public static final String METRIC_DEBUG = "metric_debug"; public static final String PLATFORM_INFORMATION_ENABLED = "platform_information_enabled"; public static final String PORT = "port"; @@ -293,7 +293,7 @@ private AgentConfigImpl(Map props) { putForDataSend = getProperty(PUT_FOR_DATA_SEND_PROPERTY, DEFAULT_PUT_FOR_DATA_SEND_ENABLED); isApdexTSet = getProperty(APDEX_T) != null; apdexTInMillis = (long) (getDoubleProperty(APDEX_T, DEFAULT_APDEX_T) * 1000L); - debug = Boolean.getBoolean(DEBUG) || Boolean.parseBoolean(System.getenv("NEWRELIC_DEBUG")); + debug = DebugFlag.DEBUG; metricDebug = initMetricDebugConfig(); enabled = getProperty(ENABLED, DEFAULT_ENABLED) && getProperty(AGENT_ENABLED, DEFAULT_ENABLED); experimentalRuntime = allowExperimentalRuntimeVersions(); diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentJarHelper.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentJarHelper.java index 2dd480d374..dc67f844dc 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentJarHelper.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentJarHelper.java @@ -7,6 +7,9 @@ package com.newrelic.agent.config; +import com.newrelic.agent.Agent; +import com.newrelic.agent.DebugFlag; + import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -223,12 +226,9 @@ public static String getAgentJarAttribute(String name) { } } - // The "newrelic.debug" flag redirects all Agent logging (prior to log init) to the standard output -after log init, log as usual. Unfortunately, - // we haven't initialized the Agent yet, so we cannot check it in the usual low-cost way by - // calling Agent.isDebugEnabled(). So we duplicate the functionality here for use in a few cases. private static final boolean isNewRelicDebug() { - final String newrelicDebug = "newrelic.debug"; - return Boolean.getBoolean(newrelicDebug) || Boolean.parseBoolean(System.getenv("NEWRELIC_DEBUG")); + + return DebugFlag.DEBUG; } // Use of this method should be limited to serious error cases that would cause the Agent to diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigFileHelper.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigFileHelper.java index 30e638c0a4..4921553898 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigFileHelper.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigFileHelper.java @@ -7,6 +7,9 @@ package com.newrelic.agent.config; +import com.newrelic.agent.Agent; +import com.newrelic.agent.DebugFlag; + import java.io.File; import java.text.MessageFormat; @@ -20,9 +23,6 @@ public class ConfigFileHelper { private static final String CONFIG_FILE_PROPERTY = "newrelic.config.file"; private static final String NEW_RELIC_HOME_DIRECTORY_PROPERTY = "newrelic.home"; private static final String NEW_RELIC_HOME_DIRECTORY_ENVIRONMENT_VARIABLE = "NEWRELIC_HOME"; - private static final String NEW_RELIC_DEBUG_PROPERTY = "newrelic.debug"; - - private static final String NEW_RELIC_DEBUG_ENV = "NEWRELIC_DEBUG"; private static final String[] SEARCH_DIRECTORIES = { ".", "conf", "config", "etc" }; /** @@ -38,7 +38,7 @@ public static File findConfigFile() { File parentDir = getNewRelicDirectory(); if (parentDir != null) { - if (Boolean.getBoolean(NEW_RELIC_DEBUG_PROPERTY) || Boolean.parseBoolean(System.getenv(NEW_RELIC_DEBUG_ENV)) ) { + if (DebugFlag.DEBUG) { System.err.println(MessageFormat.format("New Relic home directory: {0}", parentDir)); } } @@ -151,7 +151,7 @@ private static File findHomeDirectoryFromEnvironmentVariable() { private static File findConfigFile(File parentDirectory) { for (String searchDir : SEARCH_DIRECTORIES) { File configDir = new File(parentDirectory, searchDir); - if (Boolean.getBoolean(NEW_RELIC_DEBUG_PROPERTY)) { + if (DebugFlag.DEBUG) { System.err.println(MessageFormat.format("Searching for New Relic configuration in directory {0}", configDir)); } if (configDir.exists()) { diff --git a/settings.gradle b/settings.gradle index db8568bc74..6f6286007f 100644 --- a/settings.gradle +++ b/settings.gradle @@ -50,7 +50,7 @@ include 'functional_test' include 'functional_test:weave_test' include 'functional_test:weave_test_impl' -// JDK 9+ module system tests + JDK 9+ module system tests if (JavaVersion.current().isJava9Compatible()) { include 'module_test_9' } From 2711d816811fd53944fcfcf9eeed9319ee0e77ac Mon Sep 17 00:00:00 2001 From: tbradellis Date: Mon, 27 Jun 2022 13:00:34 -0700 Subject: [PATCH 16/19] clean code --- .../main/java/com/newrelic/agent/Agent.java | 26 +++++++++++++------ .../newrelic/agent/config/AgentJarHelper.java | 1 - .../agent/config/ConfigFileHelper.java | 1 - settings.gradle | 2 +- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/Agent.java b/newrelic-agent/src/main/java/com/newrelic/agent/Agent.java index 0535480de9..f6568db66c 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/Agent.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/Agent.java @@ -9,7 +9,11 @@ import com.google.common.collect.ImmutableMap; import com.newrelic.agent.bridge.AgentBridge; -import com.newrelic.agent.config.*; +import com.newrelic.agent.config.AgentJarHelper; +import com.newrelic.agent.config.ConfigService; +import com.newrelic.agent.config.ConfigServiceFactory; +import com.newrelic.agent.config.JarResource; +import com.newrelic.agent.config.JavaVersionUtils; import com.newrelic.agent.core.CoreService; import com.newrelic.agent.core.CoreServiceImpl; import com.newrelic.agent.logging.AgentLogManager; @@ -22,7 +26,6 @@ import com.newrelic.agent.util.asm.ClassStructure; import com.newrelic.bootstrap.BootstrapLoader; import com.newrelic.weave.utils.Streams; -import com.sun.org.apache.xpath.internal.operations.Bool; import org.objectweb.asm.ClassReader; import java.io.ByteArrayOutputStream; @@ -32,9 +35,16 @@ import java.lang.instrument.Instrumentation; import java.net.URL; import java.text.MessageFormat; -import java.util.*; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.jar.*; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.ResourceBundle; +import java.util.jar.Attributes; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; +import java.util.jar.JarOutputStream; +import java.util.jar.Manifest; import java.util.logging.Level; import java.util.regex.Pattern; @@ -77,7 +87,6 @@ public static boolean isDebugEnabled() { return DebugFlag.DEBUG; } - private static volatile boolean canFastPath = true; /** @@ -312,7 +321,8 @@ public static long getAgentPremainTimeInMillis() { private static void recordPremainTime(StatsService statsService, long startTime) { agentPremainTime = System.currentTimeMillis() - startTime; LOG.log(Level.INFO, "Premain startup complete in {0}ms", agentPremainTime); - statsService.doStatsWork(StatsWorks.getRecordResponseTimeWork(MetricNames.SUPPORTABILITY_TIMING_PREMAIN, agentPremainTime), MetricNames.SUPPORTABILITY_TIMING_PREMAIN); + statsService.doStatsWork(StatsWorks.getRecordResponseTimeWork(MetricNames.SUPPORTABILITY_TIMING_PREMAIN, agentPremainTime), + MetricNames.SUPPORTABILITY_TIMING_PREMAIN); Map environmentInfo = ImmutableMap.builder() .put("Duration", agentPremainTime) @@ -338,7 +348,7 @@ private static void recordPremainTime(StatsService statsService, long startTime) private static void recordAgentVersion(StatsService statsService) { statsService.doStatsWork( StatsWorks.getIncrementCounterWork(MessageFormat.format(MetricNames.SUPPORTABILITY_JAVA_AGENTVERSION, getVersion()), 1), - MetricNames.SUPPORTABILITY_JAVA_AGENTVERSION ); + MetricNames.SUPPORTABILITY_JAVA_AGENTVERSION); } /** diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentJarHelper.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentJarHelper.java index dc67f844dc..14acf31aab 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentJarHelper.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentJarHelper.java @@ -7,7 +7,6 @@ package com.newrelic.agent.config; -import com.newrelic.agent.Agent; import com.newrelic.agent.DebugFlag; import java.io.File; diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigFileHelper.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigFileHelper.java index 4921553898..963e660afb 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigFileHelper.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigFileHelper.java @@ -7,7 +7,6 @@ package com.newrelic.agent.config; -import com.newrelic.agent.Agent; import com.newrelic.agent.DebugFlag; import java.io.File; diff --git a/settings.gradle b/settings.gradle index 6f6286007f..db8568bc74 100644 --- a/settings.gradle +++ b/settings.gradle @@ -50,7 +50,7 @@ include 'functional_test' include 'functional_test:weave_test' include 'functional_test:weave_test_impl' - JDK 9+ module system tests +// JDK 9+ module system tests if (JavaVersion.current().isJava9Compatible()) { include 'module_test_9' } From bde9267af0e329689515bf05e11c1f97a032d15b Mon Sep 17 00:00:00 2001 From: tbradellis Date: Mon, 27 Jun 2022 19:45:07 -0700 Subject: [PATCH 17/19] delete semicolon --- newrelic-agent/src/main/java/com/newrelic/agent/DebugFlag.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/DebugFlag.java b/newrelic-agent/src/main/java/com/newrelic/agent/DebugFlag.java index 67b02c86ab..2b83c05717 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/DebugFlag.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/DebugFlag.java @@ -16,7 +16,5 @@ public class DebugFlag { // This flag cannot be set via newrelic.yml (AgentConfigImpl) because a ServiceManager and ConfigService have not been initialized for the earliest checks // for the debug setting. public static final boolean DEBUG = Boolean.getBoolean("newrelic.debug") || Boolean.parseBoolean(System.getenv("NEWRELIC_DEBUG")); - ; - } From b6e078f93b51cdd6baf50d6905dbbfe8aa922064 Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Wed, 29 Jun 2022 10:34:23 -0400 Subject: [PATCH 18/19] Capping r2dbc-mariadb --- instrumentation/r2dbc-mariadb/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/r2dbc-mariadb/build.gradle b/instrumentation/r2dbc-mariadb/build.gradle index 454a8044a4..3a54daf1ce 100644 --- a/instrumentation/r2dbc-mariadb/build.gradle +++ b/instrumentation/r2dbc-mariadb/build.gradle @@ -10,7 +10,7 @@ jar { } verifyInstrumentation { - passesOnly 'org.mariadb:r2dbc-mariadb:[1.0.2,)' + passesOnly 'org.mariadb:r2dbc-mariadb:[1.0.2,1.1.2)' excludeRegex(".*(alpha|beta|rc).*") } From 6e31d9653235b08aa2f2ed0ce45cbebdb68bde75 Mon Sep 17 00:00:00 2001 From: richard-gibson Date: Wed, 13 Jul 2022 00:32:14 +0100 Subject: [PATCH 19/19] issue 620 scala illegal access error (#876) move scala final field transformer after weaver --- .../internal/InstrumentingClassLoader.java | 15 +++++-- .../classmatchers/ScalaTraitMatcher.java | 9 ++++ .../InstrumentationClassTransformer.java | 11 +++++ .../context/InstrumentationContext.java | 11 +++++ .../ScalaTraitFinalFieldTransformer.java | 44 +++++++++++++++++++ .../custom/ScalaTraitFinalFieldVisitor.java | 25 +++++++++++ .../language/scala/ScalaAdapterTest.scala | 20 ++++++++- 7 files changed, 131 insertions(+), 4 deletions(-) create mode 100644 newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/custom/ScalaTraitFinalFieldTransformer.java create mode 100644 newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/custom/ScalaTraitFinalFieldVisitor.java diff --git a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/InstrumentingClassLoader.java b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/InstrumentingClassLoader.java index cab89d259c..5b6652f80a 100644 --- a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/InstrumentingClassLoader.java +++ b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/InstrumentingClassLoader.java @@ -20,12 +20,12 @@ import com.newrelic.agent.instrumentation.InstrumentationType; import com.newrelic.agent.instrumentation.classmatchers.ScalaTraitMatcher; import com.newrelic.agent.instrumentation.context.InstrumentationContext; +import com.newrelic.agent.instrumentation.custom.ScalaTraitFinalFieldVisitor; import com.newrelic.agent.instrumentation.tracing.Annotation; import com.newrelic.agent.instrumentation.tracing.NoticeSqlVisitor; import com.newrelic.agent.instrumentation.tracing.TraceClassVisitor; import com.newrelic.agent.instrumentation.tracing.TraceDetailsBuilder; import com.newrelic.agent.instrumentation.weaver.ClassWeaverService; -import com.newrelic.agent.instrumentation.weaver.errorhandler.LogAndReturnOriginal; import com.newrelic.agent.instrumentation.weaver.preprocessors.AgentPostprocessors; import com.newrelic.agent.instrumentation.weaver.preprocessors.AgentPreprocessors; import com.newrelic.agent.instrumentation.weaver.preprocessors.TracedWeaveInstrumentationTracker; @@ -137,6 +137,15 @@ public void classWeaved(PackageWeaveResult weaveResult, ClassLoader classloader, classBytes = weaved; } + if (classBytes != null && context.isModified() && !context.getScalaFinalFields().isEmpty()) { + ClassReader reader = new ClassReader(classBytes); + ClassWriter writer = new ClassWriter(ClassWriter.COMPUTE_FRAMES); + ClassVisitor cv = writer; + cv = new ScalaTraitFinalFieldVisitor(cv, context.getScalaFinalFields()); + reader.accept(cv, ClassReader.SKIP_FRAMES); + classBytes = writer.toByteArray(); + } + ClassReader reader = new ClassReader(classBytes); if (weaved == null) { // process trace annotations for non-weaved code @@ -145,8 +154,8 @@ public void classWeaved(PackageWeaveResult weaveResult, ClassLoader classloader, if (!context.isTracerMatch()) { if (weaved != null) { - //printClass(className, weaved); - return weaved; + printClass(className, classBytes); + return classBytes; } return null; } diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/classmatchers/ScalaTraitMatcher.java b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/classmatchers/ScalaTraitMatcher.java index 1bbe03bb1f..c06a9a12db 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/classmatchers/ScalaTraitMatcher.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/classmatchers/ScalaTraitMatcher.java @@ -6,6 +6,7 @@ import com.newrelic.weave.utils.WeaveUtils; import org.objectweb.asm.ClassReader; import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.FieldVisitor; import org.objectweb.asm.MethodVisitor; import org.objectweb.asm.Opcodes; import org.objectweb.asm.commons.Method; @@ -66,6 +67,14 @@ public void visitSource(String source, String debug) { this.isScalaSource = source.endsWith(".scala"); } + @Override + public FieldVisitor visitField(int access, String name, String descriptor, String signature, Object value) { + if(isScalaSource && this.interfaces.size() > 0 && ((access & Opcodes.ACC_FINAL) == Opcodes.ACC_FINAL)) { + context.addScalaFinalField(name); + } + return super.visitField(access, name, descriptor, signature, value); + } + @Override public MethodVisitor visitMethod(int access, String methodName, String methodDesc, String signature, String[] exceptions) { MethodVisitor mv = super.visitMethod(access, methodName, methodDesc, signature, exceptions); diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java index efca0f1013..886ff980c6 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java @@ -12,6 +12,7 @@ import com.newrelic.agent.MetricNames; import com.newrelic.agent.instrumentation.InstrumentationUtils; import com.newrelic.agent.instrumentation.classmatchers.OptimizedClassMatcher; +import com.newrelic.agent.instrumentation.custom.ScalaTraitFinalFieldTransformer; import com.newrelic.agent.instrumentation.tracing.TraceClassTransformer; import com.newrelic.agent.service.ServiceFactory; import com.newrelic.agent.stats.StatsWorks; @@ -43,6 +44,8 @@ public class InstrumentationClassTransformer implements ClassFileTransformer { private final boolean defaultMethodTracingEnabled; private final AtomicBoolean initialized = new AtomicBoolean(false); private final FinalClassTransformer finalClassTransformer = new FinalClassTransformer(); + private final ScalaTraitFinalFieldTransformer scalaTraitFinalFieldTransformer = + new ScalaTraitFinalFieldTransformer(); public InstrumentationClassTransformer(InstrumentationContextManager manager, TraceClassTransformer traceTransformer, boolean bootstrapClassloaderEnabled, boolean defaultMethodTracingEnabled) { @@ -137,6 +140,14 @@ public byte[] transform(ClassLoader loader, String className, Class classBein } } + if(context.isModified() && !context.getScalaFinalFields().isEmpty()) { + byte[] bytes = scalaTraitFinalFieldTransformer.transform(loader, className, classBeingRedefined, + protectionDomain, classfileBuffer, context, null); + if(bytes != null) { + classfileBuffer = bytes; + } + } + if (context.isModified()) { byte[] transformation = finalClassTransformer.transform(loader, className, classBeingRedefined, protectionDomain, classfileBuffer, context, null); diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationContext.java b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationContext.java index 899e804fea..0565ec945e 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationContext.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationContext.java @@ -48,6 +48,7 @@ public class InstrumentationContext implements TraceDetailsList { private boolean modified; private Multimap weavedMethods; private Multimap skipMethods; + private Set scalaFinalFields; private Set timedMethods; private Map oldReflectionStyleInstrumentationMethods; private Map oldInvokerStyleInstrumentationMethods; @@ -122,6 +123,12 @@ public void addSkipMethod(Method method, String owningClass) { skipMethods.put(method, owningClass); } + public void addScalaFinalField(String fieldName) { + if (scalaFinalFields == null) { + scalaFinalFields = new HashSet<>(); + } + scalaFinalFields.add(fieldName); + } public PointCut getOldStylePointCut(Method method) { PointCut pc = getOldInvokerStyleInstrumentationMethods().get(method); @@ -149,6 +156,10 @@ public Map> getSkipMethods() { return skipMethods == null ? Collections.emptyMap() : skipMethods.asMap(); } + public Set getScalaFinalFields() { + return scalaFinalFields == null ? Collections.emptySet() : new HashSet<>(scalaFinalFields); + } + /** * Returns methods that are timed with instrumentation injected by the new {@link TraceClassVisitor} or the old * GenericClassAdapter. diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/custom/ScalaTraitFinalFieldTransformer.java b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/custom/ScalaTraitFinalFieldTransformer.java new file mode 100644 index 0000000000..9c766c8390 --- /dev/null +++ b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/custom/ScalaTraitFinalFieldTransformer.java @@ -0,0 +1,44 @@ +package com.newrelic.agent.instrumentation.custom; + +import com.newrelic.agent.instrumentation.classmatchers.OptimizedClassMatcher; +import com.newrelic.agent.instrumentation.context.ContextClassTransformer; +import com.newrelic.agent.instrumentation.context.InstrumentationContext; +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.ClassWriter; + +import java.lang.instrument.IllegalClassFormatException; +import java.security.ProtectionDomain; +import java.util.logging.Level; + +import static com.newrelic.agent.Agent.LOG; + +public class ScalaTraitFinalFieldTransformer implements ContextClassTransformer { + @Override + public byte[] transform(ClassLoader loader, String className, Class classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer, InstrumentationContext context, OptimizedClassMatcher.Match match) throws IllegalClassFormatException { + try { + return doTransform(loader, className, classBeingRedefined, protectionDomain, classfileBuffer, context); + } catch (Throwable t) { + LOG.log(Level.FINE, "Unable to transform class " + className, t); + return null; + } + } + + + private byte[] doTransform(ClassLoader loader, String className, Class classBeingRedefined, + ProtectionDomain protectionDomain, byte[] classfileBuffer, InstrumentationContext context) { + + if (context.getScalaFinalFields().isEmpty()) { + return null; + } + + LOG.debug("Instrumenting class " + className); + ClassReader reader = new ClassReader(classfileBuffer); + ClassWriter writer = new ClassWriter(ClassWriter.COMPUTE_FRAMES); + ClassVisitor cv = writer; + cv = new ScalaTraitFinalFieldVisitor(cv, context.getScalaFinalFields()); + reader.accept(cv, ClassReader.SKIP_FRAMES); + + return writer.toByteArray(); + } +} diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/custom/ScalaTraitFinalFieldVisitor.java b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/custom/ScalaTraitFinalFieldVisitor.java new file mode 100644 index 0000000000..bf595ac6b2 --- /dev/null +++ b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/custom/ScalaTraitFinalFieldVisitor.java @@ -0,0 +1,25 @@ +package com.newrelic.agent.instrumentation.custom; + +import com.newrelic.weave.utils.WeaveUtils; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.FieldVisitor; +import org.objectweb.asm.Opcodes; + +import java.util.Set; + +public class ScalaTraitFinalFieldVisitor extends ClassVisitor { + private final Set finalFieldNames; + + public ScalaTraitFinalFieldVisitor(ClassVisitor cv, Set finalFieldNames) { + super(WeaveUtils.ASM_API_LEVEL, cv); + this.finalFieldNames = finalFieldNames; + } + + @Override + public FieldVisitor visitField(int access, String name, String descriptor, String signature, Object value) { + if(finalFieldNames.contains(name)) { + access &= ~Opcodes.ACC_FINAL; + } + return super.visitField(access, name, descriptor, signature, value); + } +} diff --git a/newrelic-weaver-scala/src/test/scala/com/nr/weave/weavepackage/language/scala/ScalaAdapterTest.scala b/newrelic-weaver-scala/src/test/scala/com/nr/weave/weavepackage/language/scala/ScalaAdapterTest.scala index 9471ab23f2..280d4c633a 100644 --- a/newrelic-weaver-scala/src/test/scala/com/nr/weave/weavepackage/language/scala/ScalaAdapterTest.scala +++ b/newrelic-weaver-scala/src/test/scala/com/nr/weave/weavepackage/language/scala/ScalaAdapterTest.scala @@ -16,7 +16,7 @@ import com.newrelic.test.marker.{Java11IncompatibleTest, Java12IncompatibleTest, @RunWith(classOf[InstrumentationTestRunner]) @InstrumentationTestConfig(includePrefixes = Array("com.nr.weave.weavepackage.language.scala.weaveclasses.")) -@Category(Array(classOf[Java11IncompatibleTest],classOf[Java12IncompatibleTest],classOf[Java13IncompatibleTest], +@Category(Array(classOf[Java12IncompatibleTest],classOf[Java13IncompatibleTest], classOf[Java14IncompatibleTest],classOf[Java15IncompatibleTest],classOf[Java16IncompatibleTest], classOf[Java17IncompatibleTest], classOf[Java18IncompatibleTest])) class ScalaAdapterTest { @@ -50,6 +50,10 @@ class ScalaAdapterTest { Assert.assertEquals("weaved-ChildNonWeaveTrait", multipleOverridingNonWeaved.traitmethod()) } + @Test + def weaveChildObjectWithTraitField() { + Assert.assertEquals("weaved-other-method", new ChildFieldClass().otherMethod()) + } } object ScalaAdapterTest { @@ -94,6 +98,15 @@ package testclasses { class MultipleOverridingNonWeaved extends ChildWeaveTrait with ChildNonWeaveTrait class MultipleOverridingWeaved extends ChildNonWeaveTrait with ChildWeaveTrait + trait FieldTrait { + val finalField: String = "field-name" + var nonFinalField: String = "non-final-field" + def otherMethod(): String = "other-method" + } + + trait ChildFieldTrait extends FieldTrait + class ChildFieldClass extends ChildFieldTrait + } package weaveclasses { @@ -123,6 +136,11 @@ package weaveclasses { } } + @ScalaWeave(originalName="com.nr.weave.weavepackage.language.scala.testclasses.FieldTrait", `type`=ScalaMatchType.Trait) + class FieldTraitWeave { + def otherMethod(): String = "weaved-"+Weaver.callOriginal() + } + @ScalaWeave(originalName="com.nr.weave.weavepackage.language.scala.testclasses.ChildWeaveTrait", `type`=ScalaMatchType.Trait) class ChildTraitWeave { def traitmethod(): String = {