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

chore(anomaly-detection) Return meaningful errors on failure of save endpoint #1149

Merged
merged 2 commits into from
Sep 12, 2024
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
26 changes: 19 additions & 7 deletions src/seer/anomaly_detection/anomaly_detection.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
TimeSeriesWithHistory,
)
from seer.dependency_injection import inject, injected
from seer.exceptions import ClientError, ServerError

anomaly_detection_module.enable()
logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -107,10 +108,23 @@ def _online_detect(
# Retrieve historic data
historic = alert_data_accessor.query(alert.id)
if historic is None:
raise Exception(f"Invalid alert id {alert.id}")

logger.error(
"no_stored_history_data",
extra={
"alert_id": alert.id,
},
)
raise ClientError("No timeseries data found for alert")

if not isinstance(historic.anomalies, MPTimeSeriesAnomalies):
raise Exception("Invalid state")
logger.error(
"invalid_state",
extra={
"note": f"Expecting object of type MPTimeSeriesAnomalies but found {type(historic.anomalies)}"
},
)
raise ServerError("Invalid state")
anomalies: MPTimeSeriesAnomalies = historic.anomalies

# TODO: Need to check the time gap between historic data and the new datapoint against the alert configuration
Expand Down Expand Up @@ -168,7 +182,7 @@ def _combo_detect(
"minimum_required": min_len,
},
)
raise Exception("Insufficient history data")
raise ClientError("Insufficient history data")

logger.info(
f"Detecting anomalies for time series with {len(ts_with_history.current)} datapoints and history of {len(ts_with_history.history)} datapoints"
Expand All @@ -194,8 +208,6 @@ def _combo_detect(
return ts_external, streamed_anomalies

def _update_anomalies(self, ts_external: List[TimeSeriesPoint], anomalies: TimeSeriesAnomalies):
if anomalies is None:
raise Exception("No anomalies available for the timeseries.")
for i, point in enumerate(ts_external):
point.anomaly = Anomaly(
anomaly_score=anomalies.scores[i],
Expand Down Expand Up @@ -227,7 +239,7 @@ def detect_anomalies(self, request: DetectAnomaliesRequest) -> DetectAnomaliesRe
else:
ts, anomalies = self._batch_detect(request.context, request.config)
self._update_anomalies(ts, anomalies)
return DetectAnomaliesResponse(timeseries=ts)
return DetectAnomaliesResponse(success=True, timeseries=ts)

@inject
def store_data(
Expand All @@ -253,7 +265,7 @@ def store_data(
"minimum_required": min_len,
},
)
raise Exception(f"Insufficient time series data for alert {request.alert.id}")
raise ClientError("Insufficient time series data for alert")

logger.info(
"store_alert_request",
Expand Down
3 changes: 2 additions & 1 deletion src/seer/anomaly_detection/detectors/anomaly_detectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
TimeSeriesAnomalies,
)
from seer.dependency_injection import inject, injected
from seer.exceptions import ServerError

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -85,7 +86,7 @@ def _compute_matrix_profile(
logger.debug(f"window_size: {window_size}")
if window_size <= 0:
# TODO: Add sentry logging of this error
raise Exception("Invalid window size")
raise ServerError("Invalid window size")
# Get the matrix profile for the time series
mp = stumpy.stump(
ts_values,
Expand Down
5 changes: 3 additions & 2 deletions src/seer/anomaly_detection/detectors/mp_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from seer.anomaly_detection.detectors.mp_config import MPConfig
from seer.anomaly_detection.detectors.normalizers import Normalizer
from seer.dependency_injection import inject, injected
from seer.exceptions import ServerError


class MPUtils(BaseModel):
Expand Down Expand Up @@ -38,12 +39,12 @@ def get_mp_dist_from_mp(
mp_dist = mp[:, 0]
if mp_config is not None and mp_config.normalize_mp:
if normalizer is None:
raise Exception("Need normalizer to normalize MP")
raise ServerError("Need normalizer to normalize MP")
mp_dist = normalizer.normalize(mp_dist)

if pad_to_len is not None:
if pad_to_len < len(mp_dist):
raise Exception(
raise ServerError(
"Requested length should be greater than or equal to current mp_dist"
)
nan_value_count = np.empty(pad_to_len - len(mp_dist))
Expand Down
5 changes: 4 additions & 1 deletion src/seer/anomaly_detection/models/external.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ class DetectAnomaliesRequest(BaseModel):


class DetectAnomaliesResponse(BaseModel):
timeseries: List[TimeSeriesPoint]
success: bool
message: Optional[str] = Field(None)
timeseries: Optional[List[TimeSeriesPoint]] = Field(None)


class StoreDataRequest(BaseModel):
Expand All @@ -85,3 +87,4 @@ class StoreDataRequest(BaseModel):

class StoreDataResponse(BaseModel):
success: bool
message: Optional[str] = Field(None)
13 changes: 11 additions & 2 deletions src/seer/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
from seer.bootup import bootup, module
from seer.configuration import AppConfig
from seer.dependency_injection import inject, injected, resolve
from seer.exceptions import ClientError
from seer.grouping.grouping import (
BulkCreateGroupingRecordsResponse,
CreateGroupingRecordsRequest,
Expand Down Expand Up @@ -252,15 +253,23 @@ def summarize_issue_endpoint(data: SummarizeIssueRequest) -> SummarizeIssueRespo
def detect_anomalies_endpoint(data: DetectAnomaliesRequest) -> DetectAnomaliesResponse:
sentry_sdk.set_tag("organization_id", data.organization_id)
sentry_sdk.set_tag("project_id", data.project_id)
return anomaly_detection().detect_anomalies(data)
try:
response = anomaly_detection().detect_anomalies(data)
except ClientError as e:
response = DetectAnomaliesResponse(success=False, message=str(e))
return response


@json_api(blueprint, "/v1/anomaly-detection/store")
@sentry_sdk.trace
def store_data_endpoint(data: StoreDataRequest) -> StoreDataResponse:
sentry_sdk.set_tag("organization_id", data.organization_id)
sentry_sdk.set_tag("project_id", data.project_id)
return anomaly_detection().store_data(data)
try:
response = anomaly_detection().store_data(data)
except ClientError as e:
response = StoreDataResponse(success=False, message=str(e))
return response


@blueprint.route("/health/live", methods=["GET"])
Expand Down
10 changes: 10 additions & 0 deletions src/seer/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class ClientError(Exception):
def __init__(self, message):
# Call the base class constructor with the parameters it needs
super().__init__(message)


class ServerError(Exception):
def __init__(self, message):
# Call the base class constructor with the parameters it needs
super().__init__(message)
Loading