Skip to content

Commit

Permalink
check expected error config for noticeError api
Browse files Browse the repository at this point in the history 
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

remove bad test

remove invalid test

review revisions. clean code

fix supportability check

refactor no expectedByAPI bool
  • Loading branch information
@tbradellis
tbradellis committed Oct 4, 2022
1 parent b9c925e commit 878afb0
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 12 deletions.
41 changes: 39 additions & 2 deletions functional_test/src/test/java/test/newrelic/test/agent/ExpectedErrorsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -251,6 +282,7 @@ public void nonExpectedErrorNoTransaction() throws Exception {
verifyExpectedErrorSupportabilityApiCalls(statsEngine, 0, 0, 1, 0);
verifyIgnoreErrorSupportabilityApiCalls(statsEngine, 0, 0, 0);
} finally {

holder.close();
}
}
Expand Down Expand Up @@ -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.
*
* <p>
* The apiThrowable metric is recorded in the noticeError API and does not suffer from the same need to double count
* in the assertion.
* <p>
* 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());
}
Expand Down
13 changes: 13 additions & 0 deletions functional_test/src/test/resources/configs/expected_errors_test.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
47 changes: 37 additions & 10 deletions newrelic-agent/src/main/java/com/newrelic/api/agent/NewRelicApiImplementation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -56,7 +58,7 @@ public NewRelicApiImplementation() {
*/
@Override
public void noticeError(Throwable throwable, Map<String, ?> params) {
noticeError(throwable, params, false);
noticeError(throwable, params, isExpectedErrorConfigured(throwable));
}

/**
Expand All @@ -68,14 +70,16 @@ 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, 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.
*/
Expand All @@ -86,7 +90,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)
*/
Expand All @@ -97,11 +102,19 @@ public void noticeError(String message) {
}

@Override
public void noticeError(Throwable throwable, boolean expected) {
Map<String, String> params = Collections.emptyMap();
noticeError(throwable, params, expected);
}

public void noticeError(Throwable throwable, Map<String, ?> 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);
}
Expand All @@ -116,16 +129,11 @@ 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 {
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);
Expand All @@ -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<ExpectedErrorConfig> 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<String, ?> filterErrorAtts(Map<String, ?> params, AttributeSender attributeSender) {
Map<String, Object> attributes = new TreeMap<>();
if (params == null) {
Expand Down

0 comments on commit 878afb0

Please sign in to comment.