From 7f3a2add8658e3f2f7e40957ff8a113a3a6ecb6a Mon Sep 17 00:00:00 2001 From: Yaliang Wu Date: Thu, 29 Jul 2021 23:42:43 +0000 Subject: [PATCH] put general error message into constants --- .../org/opensearch/ad/EntityProfileRunner.java | 3 ++- .../ad/constant/CommonErrorMessages.java | 14 ++++++++++++++ .../AnomalyDetectorJobTransportAction.java | 6 ++++-- .../DeleteAnomalyDetectorTransportAction.java | 3 ++- .../DeleteAnomalyResultsTransportAction.java | 3 ++- .../GetAnomalyDetectorTransportAction.java | 3 ++- .../IndexAnomalyDetectorTransportAction.java | 6 ++++-- .../PreviewAnomalyDetectorTransportAction.java | 3 ++- .../SearchAnomalyDetectorInfoTransportAction.java | 3 ++- .../StatsAnomalyDetectorTransportAction.java | 3 ++- .../ad/transport/StopDetectorTransportAction.java | 7 ++++--- .../ad/transport/handler/ADSearchHandler.java | 3 ++- .../java/org/opensearch/ad/util/ParseUtils.java | 9 ++++++--- 13 files changed, 48 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/opensearch/ad/EntityProfileRunner.java b/src/main/java/org/opensearch/ad/EntityProfileRunner.java index 8d557863f..1cdf3e489 100644 --- a/src/main/java/org/opensearch/ad/EntityProfileRunner.java +++ b/src/main/java/org/opensearch/ad/EntityProfileRunner.java @@ -76,6 +76,7 @@ public class EntityProfileRunner extends AbstractProfileRunner { private final Logger logger = LogManager.getLogger(EntityProfileRunner.class); static final String NOT_HC_DETECTOR_ERR_MSG = "This is not a high cardinality detector"; + static final String EMPTY_ENTITY_ATTRIBUTES = "Empty entity attributes"; static final String NO_ENTITY = "Cannot find entity"; private Client client; private NamedXContentRegistry xContentRegistry; @@ -159,7 +160,7 @@ private void validateEntity( ) { Map attributes = entity.getAttributes(); if (attributes == null || attributes.size() != categoryFields.size()) { - listener.onFailure(new IllegalArgumentException("Empty entity attributes")); + listener.onFailure(new IllegalArgumentException(EMPTY_ENTITY_ATTRIBUTES)); return; } for (String field : categoryFields) { diff --git a/src/main/java/org/opensearch/ad/constant/CommonErrorMessages.java b/src/main/java/org/opensearch/ad/constant/CommonErrorMessages.java index ef0375a0e..3bfd220aa 100644 --- a/src/main/java/org/opensearch/ad/constant/CommonErrorMessages.java +++ b/src/main/java/org/opensearch/ad/constant/CommonErrorMessages.java @@ -48,6 +48,8 @@ public class CommonErrorMessages { public static String FAIL_TO_FIND_DETECTOR_MSG = "Fail to find detector with id: "; public static String FAIL_TO_GET_PROFILE_MSG = "Fail to get profile for detector "; public static String FAIL_TO_GET_TOTAL_ENTITIES = "Failed to get total entities for detector "; + public static String FAIL_TO_GET_USER_INFO = "Unable to get user information from detector "; + public static String NO_PERMISSION_TO_ACCESS_DETECTOR = "User does not have permissions to access detector: "; public static String CATEGORICAL_FIELD_NUMBER_SURPASSED = "We don't support categorical fields more than "; public static String EMPTY_PROFILES_COLLECT = "profiles to collect are missing or invalid"; public static String FAIL_FETCH_ERR_MSG = "Fail to fetch profile for "; @@ -72,4 +74,16 @@ public static String getTooManyCategoricalFieldErr(int limit) { public static String EXCEED_HISTORICAL_ANALYSIS_LIMIT = "Exceed max historical analysis limit per node"; public static String NO_ELIGIBLE_NODE_TO_RUN_DETECTOR = "No eligible node to run detector "; public static String EMPTY_STALE_RUNNING_ENTITIES = "Empty stale running entities"; + + public static String FAIL_TO_GET_DETECTOR = "Fail to get detector"; + public static String FAIL_TO_GET_DETECTOR_INFO = "Fail to get detector info"; + public static String FAIL_TO_CREATE_DETECTOR = "Fail to create detector"; + public static String FAIL_TO_UPDATE_DETECTOR = "Fail to update detector"; + public static String FAIL_TO_PREVIEW_DETECTOR = "Fail to preview detector"; + public static String FAIL_TO_START_DETECTOR = "Fail to start detector"; + public static String FAIL_TO_STOP_DETECTOR = "Fail to stop detector"; + public static String FAIL_TO_DELETE_DETECTOR = "Fail to delete detector"; + public static String FAIL_TO_DELETE_AD_RESULT = "Fail to delete anomaly result"; + public static String FAIL_TO_GET_STATS = "Fail to get stats"; + public static String FAIL_TO_SEARCH = "Fail to search"; } diff --git a/src/main/java/org/opensearch/ad/transport/AnomalyDetectorJobTransportAction.java b/src/main/java/org/opensearch/ad/transport/AnomalyDetectorJobTransportAction.java index 2c05b4f04..6e59fd23d 100644 --- a/src/main/java/org/opensearch/ad/transport/AnomalyDetectorJobTransportAction.java +++ b/src/main/java/org/opensearch/ad/transport/AnomalyDetectorJobTransportAction.java @@ -26,6 +26,8 @@ package org.opensearch.ad.transport; +import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_START_DETECTOR; +import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_STOP_DETECTOR; import static org.opensearch.ad.settings.AnomalyDetectorSettings.FILTER_BY_BACKEND_ROLES; import static org.opensearch.ad.settings.AnomalyDetectorSettings.REQUEST_TIMEOUT; import static org.opensearch.ad.util.ParseUtils.getUserContext; @@ -97,8 +99,8 @@ protected void doExecute(Task task, AnomalyDetectorJobRequest request, ActionLis long primaryTerm = request.getPrimaryTerm(); String rawPath = request.getRawPath(); TimeValue requestTimeout = REQUEST_TIMEOUT.get(settings); - String action = rawPath.endsWith(RestHandlerUtils.START_JOB) ? "start" : "stop"; - ActionListener listener = wrapRestActionListener(actionListener, "Failed to " + action + " detector"); + String errorMessage = rawPath.endsWith(RestHandlerUtils.START_JOB) ? FAIL_TO_START_DETECTOR : FAIL_TO_STOP_DETECTOR; + ActionListener listener = wrapRestActionListener(actionListener, errorMessage); // By the time request reaches here, the user permissions are validated by Security plugin. User user = getUserContext(client); diff --git a/src/main/java/org/opensearch/ad/transport/DeleteAnomalyDetectorTransportAction.java b/src/main/java/org/opensearch/ad/transport/DeleteAnomalyDetectorTransportAction.java index dc69385b9..6b8de19b1 100644 --- a/src/main/java/org/opensearch/ad/transport/DeleteAnomalyDetectorTransportAction.java +++ b/src/main/java/org/opensearch/ad/transport/DeleteAnomalyDetectorTransportAction.java @@ -26,6 +26,7 @@ package org.opensearch.ad.transport; +import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_DELETE_DETECTOR; import static org.opensearch.ad.model.ADTaskType.HISTORICAL_DETECTOR_TASK_TYPES; import static org.opensearch.ad.model.AnomalyDetectorJob.ANOMALY_DETECTOR_JOB_INDEX; import static org.opensearch.ad.settings.AnomalyDetectorSettings.FILTER_BY_BACKEND_ROLES; @@ -103,7 +104,7 @@ protected void doExecute(Task task, DeleteAnomalyDetectorRequest request, Action String detectorId = request.getDetectorID(); LOG.info("Delete anomaly detector job {}", detectorId); User user = getUserContext(client); - ActionListener listener = wrapRestActionListener(actionListener, "Failed to delete detector"); + ActionListener listener = wrapRestActionListener(actionListener, FAIL_TO_DELETE_DETECTOR); // By the time request reaches here, the user permissions are validated by Security plugin. try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) { resolveUserAndExecute( diff --git a/src/main/java/org/opensearch/ad/transport/DeleteAnomalyResultsTransportAction.java b/src/main/java/org/opensearch/ad/transport/DeleteAnomalyResultsTransportAction.java index f759b80dc..1634170cb 100644 --- a/src/main/java/org/opensearch/ad/transport/DeleteAnomalyResultsTransportAction.java +++ b/src/main/java/org/opensearch/ad/transport/DeleteAnomalyResultsTransportAction.java @@ -11,6 +11,7 @@ package org.opensearch.ad.transport; +import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_DELETE_AD_RESULT; import static org.opensearch.ad.settings.AnomalyDetectorSettings.FILTER_BY_BACKEND_ROLES; import static org.opensearch.ad.util.ParseUtils.addUserBackendRolesFilter; import static org.opensearch.ad.util.ParseUtils.getUserContext; @@ -55,7 +56,7 @@ public DeleteAnomalyResultsTransportAction( @Override protected void doExecute(Task task, DeleteByQueryRequest request, ActionListener actionListener) { - ActionListener listener = wrapRestActionListener(actionListener, "Failed to delete anomaly result"); + ActionListener listener = wrapRestActionListener(actionListener, FAIL_TO_DELETE_AD_RESULT); delete(request, listener); } diff --git a/src/main/java/org/opensearch/ad/transport/GetAnomalyDetectorTransportAction.java b/src/main/java/org/opensearch/ad/transport/GetAnomalyDetectorTransportAction.java index 8e759fd21..1ee7a5e08 100644 --- a/src/main/java/org/opensearch/ad/transport/GetAnomalyDetectorTransportAction.java +++ b/src/main/java/org/opensearch/ad/transport/GetAnomalyDetectorTransportAction.java @@ -26,6 +26,7 @@ package org.opensearch.ad.transport; +import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_GET_DETECTOR; import static org.opensearch.ad.model.ADTaskType.ALL_DETECTOR_TASK_TYPES; import static org.opensearch.ad.model.AnomalyDetector.ANOMALY_DETECTORS_INDEX; import static org.opensearch.ad.model.AnomalyDetectorJob.ANOMALY_DETECTOR_JOB_INDEX; @@ -144,7 +145,7 @@ public GetAnomalyDetectorTransportAction( protected void doExecute(Task task, GetAnomalyDetectorRequest request, ActionListener actionListener) { String detectorID = request.getDetectorID(); User user = getUserContext(client); - ActionListener listener = wrapRestActionListener(actionListener, "Failed to get detector"); + ActionListener listener = wrapRestActionListener(actionListener, FAIL_TO_GET_DETECTOR); try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) { resolveUserAndExecute( user, diff --git a/src/main/java/org/opensearch/ad/transport/IndexAnomalyDetectorTransportAction.java b/src/main/java/org/opensearch/ad/transport/IndexAnomalyDetectorTransportAction.java index 2be704f31..db7227d96 100644 --- a/src/main/java/org/opensearch/ad/transport/IndexAnomalyDetectorTransportAction.java +++ b/src/main/java/org/opensearch/ad/transport/IndexAnomalyDetectorTransportAction.java @@ -26,6 +26,8 @@ package org.opensearch.ad.transport; +import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_CREATE_DETECTOR; +import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_UPDATE_DETECTOR; import static org.opensearch.ad.settings.AnomalyDetectorSettings.FILTER_BY_BACKEND_ROLES; import static org.opensearch.ad.util.ParseUtils.checkFilterByBackendRoles; import static org.opensearch.ad.util.ParseUtils.getDetector; @@ -100,8 +102,8 @@ protected void doExecute(Task task, IndexAnomalyDetectorRequest request, ActionL User user = getUserContext(client); String detectorId = request.getDetectorID(); RestRequest.Method method = request.getMethod(); - String action = method == RestRequest.Method.PUT ? "update" : "create"; - ActionListener listener = wrapRestActionListener(actionListener, "Failed to " + action + " detector"); + String errorMessage = method == RestRequest.Method.PUT ? FAIL_TO_UPDATE_DETECTOR : FAIL_TO_CREATE_DETECTOR; + ActionListener listener = wrapRestActionListener(actionListener, errorMessage); try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) { resolveUserAndExecute(user, detectorId, method, listener, (detector) -> adExecute(request, user, detector, context, listener)); } catch (Exception e) { diff --git a/src/main/java/org/opensearch/ad/transport/PreviewAnomalyDetectorTransportAction.java b/src/main/java/org/opensearch/ad/transport/PreviewAnomalyDetectorTransportAction.java index 38d13c390..40a0eed8a 100644 --- a/src/main/java/org/opensearch/ad/transport/PreviewAnomalyDetectorTransportAction.java +++ b/src/main/java/org/opensearch/ad/transport/PreviewAnomalyDetectorTransportAction.java @@ -26,6 +26,7 @@ package org.opensearch.ad.transport; +import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_PREVIEW_DETECTOR; import static org.opensearch.ad.settings.AnomalyDetectorSettings.FILTER_BY_BACKEND_ROLES; import static org.opensearch.ad.settings.AnomalyDetectorSettings.MAX_ANOMALY_FEATURES; import static org.opensearch.ad.settings.AnomalyDetectorSettings.MAX_CONCURRENT_PREVIEW; @@ -116,7 +117,7 @@ protected void doExecute( ) { String detectorId = request.getDetectorId(); User user = getUserContext(client); - ActionListener listener = wrapRestActionListener(actionListener, "Failed to preview detector"); + ActionListener listener = wrapRestActionListener(actionListener, FAIL_TO_PREVIEW_DETECTOR); try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) { resolveUserAndExecute( user, diff --git a/src/main/java/org/opensearch/ad/transport/SearchAnomalyDetectorInfoTransportAction.java b/src/main/java/org/opensearch/ad/transport/SearchAnomalyDetectorInfoTransportAction.java index 790367e7d..14d390ecb 100644 --- a/src/main/java/org/opensearch/ad/transport/SearchAnomalyDetectorInfoTransportAction.java +++ b/src/main/java/org/opensearch/ad/transport/SearchAnomalyDetectorInfoTransportAction.java @@ -26,6 +26,7 @@ package org.opensearch.ad.transport; +import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_GET_DETECTOR_INFO; import static org.opensearch.ad.model.AnomalyDetector.ANOMALY_DETECTORS_INDEX; import static org.opensearch.ad.util.RestHandlerUtils.wrapRestActionListener; @@ -74,7 +75,7 @@ protected void doExecute( ) { String name = request.getName(); String rawPath = request.getRawPath(); - ActionListener listener = wrapRestActionListener(actionListener, "Failed to get detector info"); + ActionListener listener = wrapRestActionListener(actionListener, FAIL_TO_GET_DETECTOR_INFO); try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) { SearchRequest searchRequest = new SearchRequest().indices(ANOMALY_DETECTORS_INDEX); if (rawPath.endsWith(RestHandlerUtils.COUNT)) { diff --git a/src/main/java/org/opensearch/ad/transport/StatsAnomalyDetectorTransportAction.java b/src/main/java/org/opensearch/ad/transport/StatsAnomalyDetectorTransportAction.java index fbf2c4773..4910e6aae 100644 --- a/src/main/java/org/opensearch/ad/transport/StatsAnomalyDetectorTransportAction.java +++ b/src/main/java/org/opensearch/ad/transport/StatsAnomalyDetectorTransportAction.java @@ -26,6 +26,7 @@ package org.opensearch.ad.transport; +import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_GET_STATS; import static org.opensearch.ad.util.RestHandlerUtils.wrapRestActionListener; import java.util.HashMap; @@ -83,7 +84,7 @@ public StatsAnomalyDetectorTransportAction( @Override protected void doExecute(Task task, ADStatsRequest request, ActionListener actionListener) { - ActionListener listener = wrapRestActionListener(actionListener, "Failed to get stats"); + ActionListener listener = wrapRestActionListener(actionListener, FAIL_TO_GET_STATS); try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) { getStats(client, listener, request); } catch (Exception e) { diff --git a/src/main/java/org/opensearch/ad/transport/StopDetectorTransportAction.java b/src/main/java/org/opensearch/ad/transport/StopDetectorTransportAction.java index e6d632640..593f24e71 100644 --- a/src/main/java/org/opensearch/ad/transport/StopDetectorTransportAction.java +++ b/src/main/java/org/opensearch/ad/transport/StopDetectorTransportAction.java @@ -26,6 +26,8 @@ package org.opensearch.ad.transport; +import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_STOP_DETECTOR; + import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; @@ -87,10 +89,9 @@ protected void doExecute(Task task, ActionRequest actionRequest, ActionListener< listener.onResponse(new StopDetectorResponse(false)); })); } catch (Exception e) { - String errorMessage = "Fail to stop detector " + adID; - LOG.error(errorMessage, e); + LOG.error(FAIL_TO_STOP_DETECTOR + " " + adID, e); Throwable cause = ExceptionsHelper.unwrapCause(e); - listener.onFailure(new InternalFailure(adID, errorMessage, cause)); + listener.onFailure(new InternalFailure(adID, FAIL_TO_STOP_DETECTOR, cause)); } } } diff --git a/src/main/java/org/opensearch/ad/transport/handler/ADSearchHandler.java b/src/main/java/org/opensearch/ad/transport/handler/ADSearchHandler.java index 43f72d779..7a96f5bde 100644 --- a/src/main/java/org/opensearch/ad/transport/handler/ADSearchHandler.java +++ b/src/main/java/org/opensearch/ad/transport/handler/ADSearchHandler.java @@ -26,6 +26,7 @@ package org.opensearch.ad.transport.handler; +import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_SEARCH; import static org.opensearch.ad.settings.AnomalyDetectorSettings.FILTER_BY_BACKEND_ROLES; import static org.opensearch.ad.util.ParseUtils.addUserBackendRolesFilter; import static org.opensearch.ad.util.ParseUtils.getUserContext; @@ -66,7 +67,7 @@ public ADSearchHandler(Settings settings, ClusterService clusterService, Client */ public void search(SearchRequest request, ActionListener actionListener) { User user = getUserContext(client); - ActionListener listener = wrapRestActionListener(actionListener, "Failed to get stats"); + ActionListener listener = wrapRestActionListener(actionListener, FAIL_TO_SEARCH); try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) { validateRole(request, user, listener); } catch (Exception e) { diff --git a/src/main/java/org/opensearch/ad/util/ParseUtils.java b/src/main/java/org/opensearch/ad/util/ParseUtils.java index 425033488..b973c25e7 100644 --- a/src/main/java/org/opensearch/ad/util/ParseUtils.java +++ b/src/main/java/org/opensearch/ad/util/ParseUtils.java @@ -26,6 +26,9 @@ package org.opensearch.ad.util; +import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_FIND_DETECTOR_MSG; +import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_GET_USER_INFO; +import static org.opensearch.ad.constant.CommonErrorMessages.NO_PERMISSION_TO_ACCESS_DETECTOR; import static org.opensearch.ad.constant.CommonName.DATE_HISTOGRAM; import static org.opensearch.ad.constant.CommonName.EPOCH_MILLIS_FORMAT; import static org.opensearch.ad.constant.CommonName.FEATURE_AGGS; @@ -562,13 +565,13 @@ public static void onGetAdResponse( function.accept(detector); } else { logger.debug("User: " + requestUser.getName() + " does not have permissions to access detector: " + detectorId); - listener.onFailure(new AnomalyDetectionException("User does not have permissions to access detector: " + detectorId)); + listener.onFailure(new AnomalyDetectionException(NO_PERMISSION_TO_ACCESS_DETECTOR + detectorId)); } } catch (Exception e) { - listener.onFailure(new AnomalyDetectionException("Unable to get user information from detector " + detectorId)); + listener.onFailure(new AnomalyDetectionException(FAIL_TO_GET_USER_INFO + detectorId)); } } else { - listener.onFailure(new ResourceNotFoundException(detectorId, "AnomalyDetector is not found with id: " + detectorId)); + listener.onFailure(new ResourceNotFoundException(detectorId, FAIL_TO_FIND_DETECTOR_MSG + detectorId)); } }