Skip to content

Commit ad645cf

Browse files
vechekavecheka
committed
Address typo and improve publishErrorTypeMetricCounter function
Signed-off-by: Vecheka Chhourn <vecheka@amazon.com>
1 parent 4b4febc commit ad645cf

4 files changed

Lines changed: 32 additions & 37 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
import java.util.concurrent.TimeoutException;
4343

4444
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.NOISY;
45-
import static org.opensearch.dataprepper.plugins.source.source_crawler.utils.MetricsHelper.REQEUEST_ERRORS;
45+
import static org.opensearch.dataprepper.plugins.source.source_crawler.utils.MetricsHelper.REQUEST_ERRORS;
4646

4747
/**
4848
* Implementation of CrawlerClient for Office 365 audit logs.
@@ -86,7 +86,7 @@ public Office365CrawlerClient(final Office365Service service,
8686
this.bufferWriteRetrySuccessCounter = pluginMetrics.counter(BUFFER_WRITE_RETRY_SUCCESS);
8787
this.bufferWriteRetryAttemptsCounter = pluginMetrics.counter(BUFFER_WRITE_RETRY_ATTEMPTS);
8888
this.bufferWriteFailuresCounter = pluginMetrics.counter(BUFFER_WRITE_FAILURES);
89-
this.requestErrorsCounter = pluginMetrics.counter(REQEUEST_ERRORS);
89+
this.requestErrorsCounter = pluginMetrics.counter(REQUEST_ERRORS);
9090
}
9191

9292
@VisibleForTesting

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

Lines changed: 3 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -142,17 +142,8 @@ public void startSubscriptions() {
142142
}
143143
}, authConfig::renewCredentials);
144144
}
145-
} catch (HttpClientErrorException | HttpServerErrorException e) {
146-
HttpStatus statusCode = e.getStatusCode();
147-
publishErrorTypeMetricCounter(statusCode.getReasonPhrase(), this.errorTypeMetricCounterMap);
148-
log.error(NOISY, "Failed to initialize subscriptions with status code {}: {}",
149-
statusCode, e.getMessage());
150-
throw new RuntimeException("Failed to initialize subscriptions: " + e.getMessage(), e);
151145
} catch (Exception e) {
152-
// FORBIDDEN throws SecurityException in RetryHandler
153-
if (e instanceof SecurityException) {
154-
publishErrorTypeMetricCounter(HttpStatus.FORBIDDEN.getReasonPhrase(), this.errorTypeMetricCounterMap);
155-
}
146+
publishErrorTypeMetricCounter(e, this.errorTypeMetricCounterMap);
156147
log.error(NOISY, "Failed to initialize subscriptions", e);
157148
throw new RuntimeException("Failed to initialize subscriptions: " + e.getMessage(), e);
158149
}
@@ -214,16 +205,8 @@ public AuditLogsResponse searchAuditLogs(final String contentType,
214205
authConfig::renewCredentials,
215206
searchRequestsFailedCounter
216207
);
217-
} catch (HttpClientErrorException | HttpServerErrorException e) {
218-
HttpStatus statusCode = e.getStatusCode();
219-
publishErrorTypeMetricCounter(statusCode.getReasonPhrase(), this.errorTypeMetricCounterMap);
220-
log.error(NOISY, "Error while fetching audit logs for content type {}", contentType, e);
221-
throw new RuntimeException("Failed to fetch audit logs", e);
222208
} catch (Exception e) {
223-
// FORBIDDEN throws SecurityException in RetryHandler
224-
if (e instanceof SecurityException) {
225-
publishErrorTypeMetricCounter(HttpStatus.FORBIDDEN.getReasonPhrase(), this.errorTypeMetricCounterMap);
226-
}
209+
publishErrorTypeMetricCounter(e, this.errorTypeMetricCounterMap);
227210
log.error(NOISY, "Error while fetching audit logs for content type {}", contentType, e);
228211
throw new RuntimeException("Failed to fetch audit logs", e);
229212
}
@@ -265,16 +248,8 @@ public String getAuditLog(String contentUri) {
265248
}, authConfig::renewCredentials, auditLogRequestsFailedCounter);
266249
auditLogRequestsSuccessCounter.increment();
267250
return response;
268-
} catch (HttpClientErrorException | HttpServerErrorException e) {
269-
HttpStatus statusCode = e.getStatusCode();
270-
publishErrorTypeMetricCounter(statusCode.getReasonPhrase(), this.errorTypeMetricCounterMap);
271-
log.error(NOISY, "Error while fetching audit log content from URI: {}", contentUri, e);
272-
throw new RuntimeException("Failed to fetch audit log", e);
273251
} catch (Exception e) {
274-
// FORBIDDEN throws SecurityException in RetryHandler
275-
if (e instanceof SecurityException) {
276-
publishErrorTypeMetricCounter(HttpStatus.FORBIDDEN.getReasonPhrase(), this.errorTypeMetricCounterMap);
277-
}
252+
publishErrorTypeMetricCounter(e, this.errorTypeMetricCounterMap);
278253
log.error(NOISY, "Error while fetching audit log content from URI: {}", contentUri, e);
279254
throw new RuntimeException("Failed to fetch audit log", e);
280255
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@
5757
import static org.mockito.Mockito.when;
5858
import static org.mockito.Mockito.never;
5959

60+
import static org.opensearch.dataprepper.plugins.source.source_crawler.utils.MetricsHelper.REQUEST_ERRORS;
61+
6062
@ExtendWith(MockitoExtension.class)
6163
@MockitoSettings(strictness = Strictness.LENIENT)
6264
class Office365CrawlerClientTest {
@@ -154,7 +156,7 @@ void testExecutePartitionWithJsonProcessingError() throws Exception {
154156

155157
// Mock the total failures counter
156158
Counter mockRequestErrorsCounter = mock(Counter.class);
157-
when(pluginMetrics.counter("requestErrors")).thenReturn(mockRequestErrorsCounter);
159+
when(pluginMetrics.counter(REQUEST_ERRORS)).thenReturn(mockRequestErrorsCounter);
158160

159161
AuditLogsResponse response = new AuditLogsResponse(
160162
Arrays.asList(Map.of(
@@ -318,7 +320,7 @@ void testMissingWorkloadField() throws Exception {
318320
void testExecutePartitionWithSearchAuditLogsError() throws Exception {
319321
// Mock the total failures counter before creating the client
320322
Counter mockRequestErrorsCounter = mock(Counter.class);
321-
when(pluginMetrics.counter("requestErrors")).thenReturn(mockRequestErrorsCounter);
323+
when(pluginMetrics.counter(REQUEST_ERRORS)).thenReturn(mockRequestErrorsCounter);
322324

323325
Office365CrawlerClient client = new Office365CrawlerClient(service, sourceConfig, pluginMetrics);
324326

‎data-prepper-plugins/saas-source-plugins/source-crawler/src/main/java/org/opensearch/dataprepper/plugins/source/source_crawler/utils/MetricsHelper.java‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,11 @@
1313
import io.micrometer.core.instrument.Counter;
1414
import org.springframework.http.HttpStatus;
1515
import org.opensearch.dataprepper.metrics.PluginMetrics;
16+
import org.springframework.web.client.HttpClientErrorException;
17+
import org.springframework.web.client.HttpServerErrorException;
1618

1719
import java.util.Map;
20+
import java.util.Optional;
1821
import java.util.HashMap;
1922
/**
2023
* The MetricsHelper class.
@@ -27,7 +30,7 @@ public class MetricsHelper {
2730
private static final String RESOURCE_NOT_FOUND = "resourceNotFound";
2831

2932
// other errors in crawlerClient
30-
public static final String REQEUEST_ERRORS = "requestErrors";
33+
public static final String REQUEST_ERRORS = "requestErrors";
3134

3235
/**
3336
* Get the metric counter map for specific errorType
@@ -54,12 +57,27 @@ public static Map<String, Counter> getErrorTypeMetricCounterMap(PluginMetrics pl
5457
* TOO_MANY_REQUESTS = requestThrottled
5558
* NOT_FOUND = resourceNotFound
5659
*
57-
* @param errorType - the httpStatusCode string represenation
60+
* @param ex - exception from RestClient
5861
* @param errorTypeMetricCounterMap - the map of errorType to metric counter
5962
*/
60-
public static void publishErrorTypeMetricCounter(String errorType, Map<String, Counter> errorTypeMetricCounterMap) {
61-
if (errorTypeMetricCounterMap != null && errorTypeMetricCounterMap.containsKey(errorType)) {
62-
errorTypeMetricCounterMap.get(errorType).increment();
63+
public static void publishErrorTypeMetricCounter(Exception ex, Map<String, Counter> errorTypeMetricCounterMap) {
64+
Optional<String> statusCode = Optional.empty();
65+
if (ex instanceof HttpClientErrorException) {
66+
HttpClientErrorException httpE = (HttpClientErrorException) ex;
67+
statusCode = Optional.ofNullable(httpE.getStatusCode().getReasonPhrase());
68+
} else if (ex instanceof HttpServerErrorException) {
69+
HttpServerErrorException httpE = (HttpServerErrorException) ex;
70+
statusCode = Optional.ofNullable(httpE.getStatusCode().getReasonPhrase());
71+
} else if (ex instanceof SecurityException) { // FORBIDDEN throws SecurityException in RetryHandler
72+
statusCode = Optional.ofNullable(HttpStatus.FORBIDDEN.getReasonPhrase());
73+
} // ignore for others
74+
75+
if (statusCode.isPresent()) {
76+
String errorType = statusCode.get();
77+
if (errorTypeMetricCounterMap != null && errorTypeMetricCounterMap.containsKey(errorType)) {
78+
errorTypeMetricCounterMap.get(errorType).increment();
79+
}
6380
}
81+
6482
}
6583
}

0 commit comments

Comments
 (0)