Skip to content

Commit 5143d47

Browse files
authored
fix: Dedup failure metrics (opensearch-project#6501)
Signed-off-by: Alexander Christensen <alchrisk@amazon.com>
1 parent 7b3e987 commit 5143d47

File tree

2 files changed

+79
-2
lines changed
  • data-prepper-plugins/saas-source-plugins/microsoft-office365-source/src

2 files changed

+79
-2
lines changed

data-prepper-plugins/saas-source-plugins/microsoft-office365-source/src/main/java/org/opensearch/dataprepper/plugins/source/microsoft_office365/Office365RestClient.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,6 @@ public AuditLogsResponse searchAuditLogs(final String contentType,
269269
);
270270
} catch (Exception e) {
271271
metricsRecorder.recordError(e);
272-
metricsRecorder.recordSearchFailure();
273272
log.error(NOISY, "Error while fetching audit logs for content type {} from URL: {}",
274273
contentType, url, e);
275274
throw new SaaSCrawlerException("Failed to fetch audit logs", e, true);
@@ -314,7 +313,6 @@ public String getAuditLog(String contentUri) {
314313
return response;
315314
} catch (Exception e) {
316315
metricsRecorder.recordError(e);
317-
metricsRecorder.recordGetFailure();
318316
log.error(NOISY, "Error while fetching audit log content from URI: {}", contentUri, e);
319317
throw new SaaSCrawlerException("Failed to fetch audit log", e, true);
320318
}

data-prepper-plugins/saas-source-plugins/microsoft-office365-source/src/test/java/org/opensearch/dataprepper/plugins/source/microsoft_office365/Office365RestClientTest.java

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -764,4 +764,83 @@ void testStartSubscriptionsAF20024RecordsSuccessMetrics() {
764764
verify(metricsRecorder, times(1)).recordSubscriptionSuccess(); // Overall operation
765765
verify(metricsRecorder, times(CONTENT_TYPES.length)).recordSubscriptionCall(); // Start API calls
766766
}
767+
768+
/**
769+
* Tests that searchAuditLogs records failure metrics only once when an exception is thrown.
770+
*
771+
* This test will FAIL if the double recording bug is present, indicating that
772+
* recordSearchFailure() is being called multiple times instead of once.
773+
*
774+
* Expected behavior: recordSearchFailure should be called exactly once per logical failure.
775+
*/
776+
@Test
777+
void testSearchAuditLogsRecordsFailureMetricsOnlyOnce() {
778+
Instant startTime = Instant.now().minus(1, ChronoUnit.HOURS);
779+
Instant endTime = Instant.now();
780+
781+
when(authConfig.getTenantId()).thenReturn("test-tenant");
782+
when(authConfig.getAccessToken()).thenReturn("test-token");
783+
784+
// Mock REST template to throw an exception that persists through all retry attempts
785+
HttpClientErrorException exception = new HttpClientErrorException(HttpStatus.INTERNAL_SERVER_ERROR);
786+
when(restTemplate.exchange(
787+
anyString(),
788+
eq(HttpMethod.GET),
789+
any(),
790+
any(ParameterizedTypeReference.class)
791+
)).thenThrow(exception);
792+
793+
// Verify that the exception is propagated
794+
SaaSCrawlerException crawlerException = assertThrows(SaaSCrawlerException.class,
795+
() -> office365RestClient.searchAuditLogs(
796+
"Audit.AzureActiveDirectory",
797+
startTime,
798+
endTime,
799+
null
800+
));
801+
802+
// Verify the exception details
803+
assertEquals("Failed to fetch audit logs", crawlerException.getMessage());
804+
assertTrue(crawlerException.isRetryable());
805+
806+
// CRITICAL TEST: Verify that recordSearchFailure is called exactly once
807+
// This test will FAIL if the double recording bug exists
808+
verify(metricsRecorder, times(1)).recordSearchFailure();
809+
}
810+
811+
/**
812+
* Tests that getAuditLog records failure metrics only once when an exception is thrown.
813+
*
814+
* This test will FAIL if the double recording bug is present, indicating that
815+
* recordGetFailure() is being called multiple times instead of once.
816+
*
817+
* Expected behavior: recordGetFailure should be called exactly once per logical failure.
818+
*/
819+
@Test
820+
void testGetAuditLogRecordsFailureMetricsOnlyOnce() {
821+
String contentUri = "https://manage.office.com/api/v1.0/test-tenant/activity/feed/audit/123";
822+
823+
when(authConfig.getAccessToken()).thenReturn("test-token");
824+
825+
// Mock REST template to throw an exception that persists through all retry attempts
826+
HttpClientErrorException exception = new HttpClientErrorException(HttpStatus.INTERNAL_SERVER_ERROR);
827+
when(restTemplate.exchange(
828+
eq(contentUri),
829+
eq(HttpMethod.GET),
830+
any(),
831+
eq(String.class)
832+
)).thenThrow(exception);
833+
834+
// Verify that the exception is propagated
835+
SaaSCrawlerException crawlerException = assertThrows(SaaSCrawlerException.class,
836+
() -> office365RestClient.getAuditLog(contentUri));
837+
838+
// Verify the exception details
839+
assertEquals("Failed to fetch audit log", crawlerException.getMessage());
840+
assertTrue(crawlerException.isRetryable());
841+
842+
// CRITICAL TEST: Verify that recordGetFailure is called exactly once
843+
// This test will FAIL if the double recording bug exists
844+
verify(metricsRecorder, times(1)).recordGetFailure();
845+
}
767846
}

0 commit comments

Comments
 (0)