Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NEWRELIC-658 check expected error config for noticeError api #1014

Merged
merged 2 commits into from
Oct 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 All @@ -274,7 +306,7 @@ public void expectedErrorNoTransaction() throws Exception {
assertEquals(0, statsEngine.getStats("Errors/all").getCallCount());
assertEquals(1, statsEngine.getStats("ErrorsExpected/all").getCallCount());

verifyExpectedErrorSupportabilityApiCalls(statsEngine, 0, 0, 1, 0);
verifyExpectedErrorSupportabilityApiCalls(statsEngine, 0, 1, 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
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
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