check expected error config for noticeError api

use placeholder for config format rm double check still check for null corrections for config check tweak to fix Supportability metrics account for null throwable rm stand-in test handle null throwable refactor no expected strings remove hack reformat file. drop unused var tests for config+expected+noticeErrorAPI checkout APITest from main must close environment holder call for the correct count optimize imports
newrelic · Sep 30, 2022 · 69f4488 · 69f4488
1 parent c491b18
commit 69f4488
Show file tree

Hide file tree

Showing 3 changed files with 92 additions and 11 deletions.
diff --git a/functional_test/src/test/java/test/newrelic/test/agent/ExpectedErrorsTest.java b/functional_test/src/test/java/test/newrelic/test/agent/ExpectedErrorsTest.java
@@ -10,13 +10,15 @@
 import com.newrelic.agent.MetricNames;
 import com.newrelic.agent.TransactionData;
 import com.newrelic.agent.TransactionDataList;
+import com.newrelic.agent.errors.ErrorServiceImpl;
 import com.newrelic.agent.metric.MetricName;
 import com.newrelic.agent.service.ServiceFactory;
 import com.newrelic.agent.stats.StatsEngine;
 import com.newrelic.api.agent.HeaderType;
 import com.newrelic.api.agent.NewRelic;
 import com.newrelic.api.agent.Response;
 import com.newrelic.api.agent.Trace;
+import org.junit.Assert;
 import org.junit.Test;
 import test.newrelic.EnvironmentHolderSettingsGenerator;
 
@@ -203,6 +205,40 @@ public void expectedStatusRange() throws Exception {
         }
     }
 
+    private static class UnExpectedException extends RuntimeException {
+        UnExpectedException(String msg) {
+            super(msg);
+        }
+    }
+
+    @Test
+    public void testNoticeErrorAPIExpectedViaConfig() throws Exception {
+        EnvironmentHolder holder = setupEnvironemntHolder("expected_error_config_api_test");
+
+        try {
+            NewRelic.noticeError(new UnExpectedException("I'm not expected")); //nothing configured
+            NewRelic.noticeError(new ExpectedError("You should have expected me")); //only exception class is configured
+            NewRelic.noticeError(new RuntimeException("I should be expected by my message")); //exception class plus message is configured
+
+            TransactionDataList transactionList = holder.getTransactionList();
+            ServiceFactory.getHarvestService().harvestNow();
+            StatsEngine statsEngine = holder.getStatsEngine();
+            assertEquals(0, transactionList.size());
+
+            ErrorServiceImpl errorService = (ErrorServiceImpl) ServiceFactory.getRPMService().getErrorService();
+
+            assertEquals(2, statsEngine.getStats("ErrorsExpected/all").getCallCount());
+            assertEquals(1, statsEngine.getStats("Errors/all").getCallCount());
+            Assert.assertEquals("incorrect error count", 3, errorService.getTracedErrorsCount());
+
+            verifyExpectedErrorSupportabilityApiCalls(statsEngine, 0, 0, 1, 1);
+        } finally {
+
+            holder.close();
+        }
+
+    }
+
     @Test
     public void expectedStatusSoClose() throws Exception {
         EnvironmentHolder holder = setupEnvironemntHolder("non_expected_status_code_test");
@@ -251,6 +287,7 @@ public void nonExpectedErrorNoTransaction() throws Exception {
             verifyExpectedErrorSupportabilityApiCalls(statsEngine, 0, 0, 1, 0);
             verifyIgnoreErrorSupportabilityApiCalls(statsEngine, 0, 0, 0);
         } finally {
+
             holder.close();
         }
     }

diff --git a/functional_test/src/test/resources/configs/expected_errors_test.yml b/functional_test/src/test/resources/configs/expected_errors_test.yml
@@ -15,6 +15,19 @@ expected_error_test:
       -
         class_name: "test.newrelic.test.agent.ExpectedError"
 
+expected_error_config_api_test:
+
+  error_collector:
+
+    expected_classes:
+      -
+        class_name: "test.newrelic.test.agent.ExpectedError"
+      -
+        class_name: "java.lang.RuntimeException"
+        message: "I should be expected by my message"
+
+
+
 
 expected_error_bad_message_test:
 

diff --git a/newrelic-agent/src/main/java/com/newrelic/api/agent/NewRelicApiImplementation.java b/newrelic-agent/src/main/java/com/newrelic/api/agent/NewRelicApiImplementation.java
@@ -15,6 +15,7 @@
 import com.newrelic.agent.bridge.AgentBridge;
 import com.newrelic.agent.bridge.PublicApi;
 import com.newrelic.agent.config.ConfigConstant;
+import com.newrelic.agent.config.ExpectedErrorConfig;
 import com.newrelic.agent.dispatchers.Dispatcher;
 import com.newrelic.agent.service.ServiceFactory;
 import com.newrelic.agent.transaction.TransactionNamingPolicy;
@@ -24,6 +25,7 @@
 import java.util.Collections;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 import java.util.TreeMap;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.logging.Level;
@@ -56,7 +58,9 @@ public NewRelicApiImplementation() {
      */
     @Override
     public void noticeError(Throwable throwable, Map<String, ?> params) {
-        noticeError(throwable, params, false);
+        noticeError(throwable, params, throwable != null &&
+                        isExpectedErrorConfigured(throwable.getClass().getName(), throwable.getMessage()),
+                false);
     }
 
     /**
@@ -68,14 +72,19 @@ public void noticeError(Throwable throwable, Map<String, ?> params) {
     @Override
     public void noticeError(Throwable throwable) {
         Map<String, String> params = Collections.emptyMap();
-        noticeError(throwable, params, false);
+        noticeError(throwable, params, throwable != null &&
+                        isExpectedErrorConfigured(throwable.getClass().getName(),
+                                throwable.getMessage()),
+                false);
+
     }
 
     /**
      * Notice an error and report it to New Relic. If this method is called within a transaction, the error message will
      * be reported with the transaction when it finishes. If it is invoked outside of a transaction, a traced error will
      * be created and reported to New Relic.
-     *
+     * Expecting Errors via configuration is not supported from the APIs that do not accept a Throwable as an attribute.
+     * To mark an Error that accepts a String as expected use the API that accepts a boolean.
      * @param message the error message
      * @param params  Custom parameters to include in the traced error. May be null. Map is copied to avoid side effects.
      */
@@ -86,7 +95,8 @@ public void noticeError(String message, Map<String, ?> params) {
 
     /**
      * Report an error to New Relic.
-     *
+     * Expecting Errors via configuration is not supported from the APIs that do not accept a Throwable as an attribute.
+     * To mark an Error that accepts a String as expected use the API that accepts a boolean.
      * @param message the error message
      * @see #noticeError(String, Map)
      */
@@ -98,11 +108,23 @@ public void noticeError(String message) {
 
     @Override
     public void noticeError(Throwable throwable, Map<String, ?> params, boolean expected) {
+        noticeError(throwable, params, expected, true);
+
+    }
+
+    @Override
+    public void noticeError(Throwable throwable, boolean expected) {
+        Map<String, String> params = Collections.emptyMap();
+        noticeError(throwable, params, expected, true);
+    }
+
+    private void noticeError(Throwable throwable, Map<String, ?> params, boolean expected, boolean expectedByApi) {
         try {
             ServiceFactory.getRPMService().getErrorService().reportException(throwable, filterErrorAtts(params, attributeSender), expected);
 
             MetricNames.recordApiSupportabilityMetric(MetricNames.SUPPORTABILITY_API_NOTICE_ERROR);
-            if (expected) {
+
+            if (expected && expectedByApi) {
                 MetricNames.recordApiSupportabilityMetric(MetricNames.SUPPORTABILITY_API_EXPECTED_ERROR_API_THROWABLE);
             }
 
@@ -116,12 +138,6 @@ public void noticeError(Throwable throwable, Map<String, ?> params, boolean expe
         }
     }
 
-    @Override
-    public void noticeError(Throwable throwable, boolean expected) {
-        Map<String, String> params = Collections.emptyMap();
-        noticeError(throwable, params, expected);
-    }
-
     @Override
     public void noticeError(String message, Map<String, ?> params, boolean expected) {
         try {
@@ -147,6 +163,21 @@ public void noticeError(String message, boolean expected) {
         noticeError(message, params, expected);
     }
 
+    private boolean isExpectedErrorConfigured(String expectedConfigName, String message) {
+        boolean expected = true;
+        String app_name = NewRelic.getAgent().getConfig().getValue("app_name");
+        Set<ExpectedErrorConfig> expectedErrors = ServiceFactory.getConfigService().getErrorCollectorConfig(app_name).getExpectedErrors();
+        for (ExpectedErrorConfig errorConfig : expectedErrors) {
+            if ((errorConfig.getErrorClass().equals(expectedConfigName) && errorConfig.getErrorMessage() == null) ||
+                    (errorConfig.getErrorMessage() != null) &&
+                            (errorConfig.getErrorClass().equals(expectedConfigName) && errorConfig.getErrorMessage().equals(message))) {
+                return expected;
+            }
+        }
+
+        return !expected;
+    }
+
     private static Map<String, ?> filterErrorAtts(Map<String, ?> params, AttributeSender attributeSender) {
         Map<String, Object> attributes = new TreeMap<>();
         if (params == null) {