Merge pull request #1685 from newrelic/NR709_AbsurdValues

Swap the ProcessPointCut over to a weaver instrumentation module to …

functional_test/src/test/java/com/newrelic/agent/instrumentation/ClassNameFilterFuncTest.java

@@ -21,13 +21,6 @@ public void phase() {
         Assert.assertTrue(classTransformer.getClassNameFilter().isIncluded("com/sun/faces/lifecycle/Phase"));
     }
 
-    @Test
-    public void process() {
-        PointCutClassTransformer classTransformer = ServiceFactory.getClassTransformerService().getClassTransformer();
-        Assert.assertTrue(classTransformer.getClassNameFilter().isIncluded("java/lang/UNIXProcess"));
-        Assert.assertTrue(classTransformer.getClassNameFilter().isIncluded("java/lang/ProcessImpl"));
-    }
-
     @Test
     public void lifecycleImpl() {
         PointCutClassTransformer classTransformer = ServiceFactory.getClassTransformerService().getClassTransformer();

instrumentation/java.process/build.gradle

@@ -0,0 +1,20 @@
+dependencies {
+    implementation(project(":agent-bridge"))
+}
+
+// This instrumentation module should not use the bootstrap classpath
+
+
+jar {
+    manifest { attributes 'Implementation-Title': 'com.newrelic.instrumentation.java.process' }
+}
+
+verifyInstrumentation {
+    verifyClasspath = false // We don't want to verify classpath since these are JDK classes
+}
+
+site {
+    title 'Java Process'
+    type 'Other'
+    versionOverride '[8,)'
+}

instrumentation/java.process/src/main/java/java/lang/Process_Instrumentation.java

@@ -0,0 +1,20 @@
+package java.lang;
+
+import com.newrelic.agent.bridge.AgentBridge;
+import com.newrelic.agent.bridge.Transaction;
+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;
+
+@Weave(type = MatchType.Interface, originalName = "java.lang.Process")
+public class Process_Instrumentation {
+    @Trace
+    public int waitFor() {
+        Transaction transaction = AgentBridge.getAgent().getTransaction(false);
+        if (transaction != null) {
+            transaction.getTracedMethod().setMetricName("Java", "Process", "waitFor");
+        }
+        return Weaver.callOriginal();
+    }
+}

instrumentation/spring-webclient-6.0/src/main/java/com/nr/agent/instrumentation/spring_webclient_60/Util.java

@@ -29,7 +29,6 @@ public class Util {
 
     public static Segment startSegment() {
         Transaction txn = AgentBridge.getAgent().getTransaction(false);
-        System.out.println(txn);
         return txn == null ? null : txn.startSegment("WebClient.exchange");
     }
 

newrelic-agent/src/main/java/com/newrelic/agent/TransactionActivity.java

@@ -378,6 +378,31 @@ public void tracerFinished(Tracer tracer, int opcode) {
     /**
      * A serious internal error occurred. All data associated with this activity will be lost.
      *
+     * Note: We started seeing this error while investigating absurd metric values
+     * (e.g. - negative durations, or call counts in the hundreds of millions)
+     * The fix was to remove the ProcessPointCut and replace it with
+     * a weaver instrumentation module.
+     * We've left this code in place just in case we see the problem in other places.
+     *
+     * The offending code that produced this error looked something like this:
+     * It may be worth noting that myServiceMethod() was being called by another service method
+     * that was also annotated with @Trace(dispatcher = true) and that method was also
+     * called the same way.  And that 3rd method was being called every 1 second
+     * by a ScheduledThreadPoolExecutor.  And the mapOfStringToListOfStrings should have at least 2 entries.
+     *
+     *  @Trace(dispatcher = true, metricName = "myMetricName")
+     *  public void myServiceMethod () {
+     *      ...
+     *      CompletableFuture.allOf(
+     *          mapOfStringToListOfStrings.entrySet().stream().map(entry -> CompletableFuture.runAsync(() -> {
+     *              ...
+     *              callSomeMethodThatUsesProcessWaitFor();
+     *              ...
+ *              })).toArray(CompletableFuture[]::new)
+     *      ).join();
+     *      ...
+     *  }
+     *
      * @param tracer the tracer that completed, leading to the internal error detection.
      * @param opcode
      */

settings.gradle

@@ -144,6 +144,7 @@ include 'instrumentation:jakarta.xml'
 include 'instrumentation:java.completable-future-jdk8'
 include 'instrumentation:java.completable-future-jdk8u40'
 include 'instrumentation:java.logging-jdk8'
+include 'instrumentation:java.process'
 include 'instrumentation:java-io'
 include 'instrumentation:javax.xml'
 include 'instrumentation:jax-rs-1.0'