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 index 3a10fe8e7e..a5018cd18e 100644 --- a/functional_test/src/test/java/test/newrelic/test/agent/ExpectedErrorsTest.java +++ b/functional_test/src/test/java/test/newrelic/test/agent/ExpectedErrorsTest.java @@ -203,6 +203,37 @@ 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()); + + assertEquals(2, statsEngine.getStats("ErrorsExpected/all").getCallCount()); + assertEquals(1, statsEngine.getStats("Errors/all").getCallCount()); + + verifyExpectedErrorSupportabilityApiCalls(statsEngine, 0, 2, 1, 1); + } finally { + + holder.close(); + } + + } + @Test public void expectedStatusSoClose() throws Exception { EnvironmentHolder holder = setupEnvironemntHolder("non_expected_status_code_test"); @@ -251,6 +282,7 @@ public void nonExpectedErrorNoTransaction() throws Exception { verifyExpectedErrorSupportabilityApiCalls(statsEngine, 0, 0, 1, 0); verifyIgnoreErrorSupportabilityApiCalls(statsEngine, 0, 0, 0); } finally { + holder.close(); } } @@ -504,12 +536,17 @@ public void expectedClassMessagesFallbackNotIgnored() throws Exception { * the config file. However, due to how the environment holder is setup, the config is read twice, so you will see * ErrorCollectorConfigImpl#initExpectedErrors called twice which doubles our metrics. Therefore the values need to * be doubled. - * + *

+ * The apiThrowable metric is recorded in the noticeError API and does not suffer from the same need to double count + * in the assertion. + *

* When calling this method, pass in the number of 'actual' calls you expect to occur under normal operation. */ private void verifyExpectedErrorSupportabilityApiCalls(StatsEngine statsEngine, int apiMessage, int apiThrowable, int configClass, int configClassMessage) { assertEquals(2 * apiMessage, statsEngine.getStats("Supportability/API/ExpectedError/Api/Message/API").getCallCount()); - assertEquals(2 * apiThrowable, statsEngine.getStats("Supportability/API/ExpectedError/Api/Throwable/API").getCallCount()); + //The apiThrowable metric is not dependent on the config double read. You should pass in the actual expected count and the assertion should + //not double count + assertEquals(1 * apiThrowable, statsEngine.getStats("Supportability/API/ExpectedError/Api/Throwable/API").getCallCount()); assertEquals(2 * configClass, statsEngine.getStats("Supportability/API/ExpectedError/Config/Class/API").getCallCount()); assertEquals(2 * configClassMessage, statsEngine.getStats("Supportability/API/ExpectedError/Config/ClassMessage/API").getCallCount()); } diff --git a/functional_test/src/test/resources/configs/expected_errors_test.yml b/functional_test/src/test/resources/configs/expected_errors_test.yml index b21ef5d4d0..7027a0de8b 100644 --- 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 index 42275d73fa..67c07600c0 100644 --- 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,7 @@ public NewRelicApiImplementation() { */ @Override public void noticeError(Throwable throwable, Map params) { - noticeError(throwable, params, false); + noticeError(throwable, params, isExpectedErrorConfigured(throwable)); } /** @@ -68,14 +70,16 @@ public void noticeError(Throwable throwable, Map params) { @Override public void noticeError(Throwable throwable) { Map params = Collections.emptyMap(); - noticeError(throwable, params, false); + noticeError(throwable, params, isExpectedErrorConfigured(throwable)); + } /** * 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 +90,8 @@ public void noticeError(String message, Map 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) */ @@ -97,11 +102,19 @@ public void noticeError(String message) { } @Override + public void noticeError(Throwable throwable, boolean expected) { + Map params = Collections.emptyMap(); + noticeError(throwable, params, expected); + } + public void noticeError(Throwable throwable, Map params, boolean expected) { try { ServiceFactory.getRPMService().getErrorService().reportException(throwable, filterErrorAtts(params, attributeSender), expected); MetricNames.recordApiSupportabilityMetric(MetricNames.SUPPORTABILITY_API_NOTICE_ERROR); + //SUPPORTABILITY_API_EXPECTED_ERROR_API_THROWABLE metric is intended to be recorded independent of whether + //the expected error was defined in config or via the actual api. Read simply as the call came through the noticeError API (SUPPORTABILITY_API_NOTICE_ERROR) + //and it was expected. if (expected) { MetricNames.recordApiSupportabilityMetric(MetricNames.SUPPORTABILITY_API_EXPECTED_ERROR_API_THROWABLE); } @@ -116,16 +129,11 @@ public void noticeError(Throwable throwable, Map params, boolean expe } } - @Override - public void noticeError(Throwable throwable, boolean expected) { - Map params = Collections.emptyMap(); - noticeError(throwable, params, expected); - } - @Override public void noticeError(String message, Map params, boolean expected) { try { ServiceFactory.getRPMService().getErrorService().reportError(message, filterErrorAtts(params, attributeSender), expected); + MetricNames.recordApiSupportabilityMetric(MetricNames.SUPPORTABILITY_API_NOTICE_ERROR); if (expected) { MetricNames.recordApiSupportabilityMetric(MetricNames.SUPPORTABILITY_API_EXPECTED_ERROR_API_MESSAGE); @@ -147,6 +155,25 @@ public void noticeError(String message, boolean expected) { noticeError(message, params, expected); } + private boolean isExpectedErrorConfigured(Throwable throwable) { + if (throwable != null) { + String expectedConfigName = throwable.getClass().getName(); + String message = throwable.getMessage(); + String app_name = NewRelic.getAgent().getConfig().getValue("app_name"); + Set expectedErrors = ServiceFactory.getConfigService().getErrorCollectorConfig(app_name).getExpectedErrors(); + for (ExpectedErrorConfig errorConfig : expectedErrors) { + String errorClass = errorConfig.getErrorClass(); + String errorMessage = errorConfig.getErrorMessage(); + if ((errorClass.equals(expectedConfigName) && errorMessage == null) || + (errorMessage != null) && + (errorClass.equals(expectedConfigName) && errorMessage.equals(message))) { + return true; + } + } + } + return false; + } + private static Map filterErrorAtts(Map params, AttributeSender attributeSender) { Map attributes = new TreeMap<>(); if (params == null) {