jianyuan/go-sentry

Metric alert has two types of response

Closed this issue · 7 comments

When we create project alerts with integrations (for example slack) we are expected response like MetricAlert
(

type MetricAlert struct {
)

Here is method:

func (s *MetricAlertsService) Update(ctx context.Context, organizationSlug string, projectSlug string, alertRuleID string, params *MetricAlert) (*MetricAlert, *Response, error) {

Here is endpoint from sentry repository.
https://github.com/getsentry/sentry/blob/41f9103679d2669ab5e9b32a8e2b1970578e4f4e/src/sentry/incidents/endpoints/project_alert_rule_index.py#L112

Uuid is using in next endpoint https://sentry.io/api/0/projects/<organizationName>/<alertsName>/alert-rule-task/<uuid>/:

https://github.com/getsentry/sentry/blob/41f9103679d2669ab5e9b32a8e2b1970578e4f4e/src/sentry/incidents/endpoints/project_alert_rule_task_details.py#L16

@jianyuan Do you have idea how better that should be implemented? My proposition I can implement in method additional calls inside of method create metric.

I would return nil as the *MetricAlert and a custom error that the user can then inspect. e.g.

type MetricAlertAsyncError struct {
    UUID string
}

func (e MetricAlertAsyncError) Error() {
    // this method is needed to make the struct conform to the error interface.
    return fmt.Sprintf("response will be available later: %v", e.UUID)
}

// modify Update() to return the error when HTTP status was 202
metricAlert, respErr, err := client.MetricAlerts.Update(ctx, "org-slug", "project-slug", "alert-rule-id", &MetricAlert{...})
if err != nil {
    if err, ok := err.(MetricAlertAsyncError); ok {
        fmt.Println("UUID was %v", err.UUID)
    }
}

But there is not error, http status is 202 (The request has been accepted for processing, but the processing has not been completed).

So it's should be response like:

type MetricAlertAsync struct {
    UUID string
}

But how and where it will handle?

@eugeniykurasov It seems that we've done something similar to what you've described in the issues alert method.

In hindsight, I prefer for the user to handle the retries instead of the library. I think returning an error containing the task UUID is a good way to tell the user to retry.

Yes, I mean like issues alert method.

What sense retries calls instead of the library? So method will be return 2 of success responses. I think that should be hide it. On client side you shouldn't care about additional calls. Action is create "alert" but instead you can get UUID, and than you need call other endpoint with logic of retry. So retries calls going to repeat on each client sides who use the lib.

Thanks for the patch, @eugeniykurasov! This is now available in v2.2.0 🎉