From 00ba024a6b37db51e6191482b8764d7dd07893c2 Mon Sep 17 00:00:00 2001 From: Anton Ovchinnikov Date: Tue, 25 Jul 2023 15:34:11 +0200 Subject: [PATCH 1/4] fix: Pass correct event type --- _examples/crons/main.go | 72 ++++++++++++++++++++++++++++++++++ check_in.go | 2 +- client.go | 38 +++++++++++------- testdata/json/checkin/001.json | 2 +- testdata/json/checkin/002.json | 2 +- transport.go | 4 +- 6 files changed, 100 insertions(+), 20 deletions(-) create mode 100644 _examples/crons/main.go diff --git a/_examples/crons/main.go b/_examples/crons/main.go new file mode 100644 index 000000000..9fbda32bc --- /dev/null +++ b/_examples/crons/main.go @@ -0,0 +1,72 @@ +package main + +import ( + "fmt" + "time" + + "github.com/getsentry/sentry-go" +) + +func runTask(monitorSlug string, duration time.Duration, shouldFail bool) { + checkinId := sentry.CaptureCheckIn( + &sentry.CheckIn{ + MonitorSlug: monitorSlug, + Status: sentry.CheckInStatusInProgress, + }, + &sentry.MonitorConfig{ + Schedule: sentry.CrontabSchedule("* * * * *"), + MaxRuntime: 2, + CheckInMargin: 1, + }, + ) + task := fmt.Sprintf("Task[monitor_slug=%s,id=%s]", monitorSlug, *checkinId) + fmt.Printf("Task started: %s\n", task) + + time.Sleep(duration) + + var status sentry.CheckInStatus + if shouldFail { + status = sentry.CheckInStatusError + } else { + status = sentry.CheckInStatusOK + } + + sentry.CaptureCheckIn( + &sentry.CheckIn{ + ID: *checkinId, + MonitorSlug: monitorSlug, + Status: status, + }, + nil, + ) + fmt.Printf("Task finished: %s; Status: %s\n", task, status) +} + +func main() { + _ = sentry.Init(sentry.ClientOptions{ + Dsn: "", + Debug: true, + }) + + // Start a task that runs every minute and always succeeds + go func() { + for { + go runTask("sentry-go-periodic-task-success", time.Second, false) + time.Sleep(time.Minute) + } + }() + + time.Sleep(3 * time.Second) + + // Start a task that runs every minute and fails every second time + go func() { + shouldFail := true + for { + go runTask("sentry-go-periodic-task-sometimes-fail", 2*time.Second, shouldFail) + time.Sleep(time.Minute) + shouldFail = !shouldFail + } + }() + + select {} +} diff --git a/check_in.go b/check_in.go index aca7e36b1..d41ba3d2a 100644 --- a/check_in.go +++ b/check_in.go @@ -80,7 +80,7 @@ type MonitorConfig struct { //nolint: maligned // prefer readability over optima Schedule MonitorSchedule `json:"schedule,omitempty"` // The allowed margin of minutes after the expected check-in time that // the monitor will not be considered missed for. - CheckInMargin int64 `json:"check_in_margin,omitempty"` + CheckInMargin int64 `json:"checkin_margin,omitempty"` // The allowed duration in minutes that the monitor may be `in_progress` // for before being considered failed due to timeout. MaxRuntime int64 `json:"max_runtime,omitempty"` diff --git a/client.go b/client.go index 5f844ce67..6136d9b58 100644 --- a/client.go +++ b/client.go @@ -416,7 +416,12 @@ func (client *Client) CaptureException(exception error, hint *EventHint, scope E // CaptureCheckIn captures a check in. func (client *Client) CaptureCheckIn(checkIn *CheckIn, monitorConfig *MonitorConfig, scope EventModifier) *EventID { event := client.EventFromCheckIn(checkIn, monitorConfig) - return client.CaptureEvent(event, nil, scope) + if event != nil && event.CheckIn != nil { + client.CaptureEvent(event, nil, scope) + return &event.CheckIn.ID + } else { + return nil + } } // CaptureEvent captures an event on the currently active client if any. @@ -532,24 +537,27 @@ func (client *Client) EventFromException(exception error, level Level) *Event { // EventFromCheckIn creates a new Sentry event from the given `check_in` instance. func (client *Client) EventFromCheckIn(checkIn *CheckIn, monitorConfig *MonitorConfig) *Event { + if checkIn == nil { + return nil + } + event := NewEvent() - checkInID := EventID(uuid()) - if checkIn != nil { - if checkIn.ID != "" { - checkInID = checkIn.ID - } + event.Type = checkInType - event.CheckIn = &CheckIn{ - ID: checkInID, - MonitorSlug: checkIn.MonitorSlug, - Status: checkIn.Status, - Duration: checkIn.Duration, - } + var checkInID EventID + if checkIn.ID == "" { + checkInID = EventID(uuid()) + } else { + checkInID = checkIn.ID } - event.MonitorConfig = monitorConfig - // EventID should be equal to CheckInID - event.EventID = checkInID + event.CheckIn = &CheckIn{ + ID: checkInID, + MonitorSlug: checkIn.MonitorSlug, + Status: checkIn.Status, + Duration: checkIn.Duration, + } + event.MonitorConfig = monitorConfig return event } diff --git a/testdata/json/checkin/001.json b/testdata/json/checkin/001.json index 64f133db7..d39990101 100644 --- a/testdata/json/checkin/001.json +++ b/testdata/json/checkin/001.json @@ -11,7 +11,7 @@ "value": 1, "unit": "day" }, - "check_in_margin": 5, + "checkin_margin": 5, "max_runtime": 30, "timezone": "America/Los_Angeles" } diff --git a/testdata/json/checkin/002.json b/testdata/json/checkin/002.json index f46a69ed9..a0220f5aa 100644 --- a/testdata/json/checkin/002.json +++ b/testdata/json/checkin/002.json @@ -10,7 +10,7 @@ "type": "crontab", "value": "0 * * * *" }, - "check_in_margin": 5, + "checkin_margin": 5, "max_runtime": 30, "timezone": "America/Los_Angeles" } diff --git a/transport.go b/transport.go index d8bdf2a74..1da0bfbaf 100644 --- a/transport.go +++ b/transport.go @@ -143,8 +143,8 @@ func envelopeFromBody(event *Event, dsn *Dsn, sentAt time.Time, body json.RawMes return nil, err } - if event.Type == transactionType { - err = encodeEnvelopeItem(enc, transactionType, body) + if event.Type == transactionType || event.Type == checkInType { + err = encodeEnvelopeItem(enc, event.Type, body) } else { err = encodeEnvelopeItem(enc, eventType, body) } From 6912a790ae255e592bef1e55a9ffedba60ee78c9 Mon Sep 17 00:00:00 2001 From: Anton Ovchinnikov Date: Tue, 25 Jul 2023 15:50:51 +0200 Subject: [PATCH 2/4] fix tests --- client_test.go | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/client_test.go b/client_test.go index 29450430a..85fd309a4 100644 --- a/client_test.go +++ b/client_test.go @@ -328,14 +328,16 @@ func TestCaptureEventNil(t *testing.T) { func TestCaptureCheckIn(t *testing.T) { tests := []struct { - name string - checkIn *CheckIn - monitorConfig *MonitorConfig + name string + checkIn *CheckIn + monitorConfig *MonitorConfig + expectNilEvent bool }{ { - name: "Nil CheckIn", - checkIn: nil, - monitorConfig: nil, + name: "Nil CheckIn", + checkIn: nil, + monitorConfig: nil, + expectNilEvent: true, }, { name: "Nil MonitorConfig", @@ -384,15 +386,26 @@ func TestCaptureCheckIn(t *testing.T) { t.Run(tt.name, func(t *testing.T) { client, _, transport := setupClientTest() client.CaptureCheckIn(tt.checkIn, tt.monitorConfig, nil) - if transport.lastEvent == nil { + capturedEvent := transport.lastEvent + + if tt.expectNilEvent && capturedEvent == nil { + // Event is nil as expected, nothing else to check + return + } + + if capturedEvent == nil { t.Fatal("missing event") } - if diff := cmp.Diff(transport.lastEvent.CheckIn, tt.checkIn); diff != "" { + if capturedEvent.Type != checkInType { + t.Errorf("Event type mismatch: want %s, got %s", checkInType, capturedEvent.Type) + } + + if diff := cmp.Diff(capturedEvent.CheckIn, tt.checkIn); diff != "" { t.Errorf("CheckIn mismatch (-want +got):\n%s", diff) } - if diff := cmp.Diff(transport.lastEvent.MonitorConfig, tt.monitorConfig); diff != "" { + if diff := cmp.Diff(capturedEvent.MonitorConfig, tt.monitorConfig); diff != "" { t.Errorf("CheckIn mismatch (-want +got):\n%s", diff) } }) From 6310d5ef78f8c79b6b0f3ba1b17c154d0a2ca1a1 Mon Sep 17 00:00:00 2001 From: Anton Ovchinnikov Date: Tue, 25 Jul 2023 16:04:21 +0200 Subject: [PATCH 3/4] Add transport test --- transport_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/transport_test.go b/transport_test.go index bdddc6a18..71efc28bd 100644 --- a/transport_test.go +++ b/transport_test.go @@ -280,6 +280,36 @@ func TestEnvelopeFromTransactionWithProfile(t *testing.T) { } } +func TestEnvelopeFromCheckInEvent(t *testing.T) { + event := newTestEvent(checkInType) + event.CheckIn = &CheckIn{ + MonitorSlug: "test-slug", + ID: "123c5be4d31e48959103a1f878a1efcb", + Status: CheckInStatusOK, + } + event.MonitorConfig = &MonitorConfig{ + Schedule: IntervalSchedule(1, MonitorScheduleUnitHour), + CheckInMargin: 10, + MaxRuntime: 5000, + Timezone: "Asia/Singapore", + } + sentAt := time.Unix(0, 0).UTC() + + body := getRequestBodyFromEvent(event) + b, err := envelopeFromBody(event, newTestDSN(t), sentAt, body) + if err != nil { + t.Fatal(err) + } + got := b.String() + want := `{"event_id":"b81c5be4d31e48959103a1f878a1efcb","sent_at":"1970-01-01T00:00:00Z","dsn":"http://public@example.com/sentry/1","sdk":{"name":"sentry.go","version":"0.0.1"}} +{"type":"check_in","length":232} +{"check_in_id":"123c5be4d31e48959103a1f878a1efcb","monitor_slug":"test-slug","status":"ok","monitor_config":{"schedule":{"type":"interval","value":1,"unit":"hour"},"checkin_margin":10,"max_runtime":5000,"timezone":"Asia/Singapore"}} +` + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Envelope mismatch (-want +got):\n%s", diff) + } +} + func TestGetRequestFromEvent(t *testing.T) { testCases := []struct { testName string From 7c1d53cf8cec702d3b6a71d18587a26d3cc3d3aa Mon Sep 17 00:00:00 2001 From: Anton Ovchinnikov Date: Wed, 26 Jul 2023 10:42:05 +0200 Subject: [PATCH 4/4] fix linter, do not record check-in IDs as last events --- client.go | 3 +-- hub.go | 11 ++--------- hub_test.go | 13 ------------- 3 files changed, 3 insertions(+), 24 deletions(-) diff --git a/client.go b/client.go index 6136d9b58..6a93fc149 100644 --- a/client.go +++ b/client.go @@ -419,9 +419,8 @@ func (client *Client) CaptureCheckIn(checkIn *CheckIn, monitorConfig *MonitorCon if event != nil && event.CheckIn != nil { client.CaptureEvent(event, nil, scope) return &event.CheckIn.ID - } else { - return nil } + return nil } // CaptureEvent captures an event on the currently active client if any. diff --git a/hub.go b/hub.go index 0930cf80d..6af1d5afa 100644 --- a/hub.go +++ b/hub.go @@ -269,21 +269,14 @@ func (hub *Hub) CaptureException(exception error) *EventID { // CaptureCheckIn calls the method of the same name on currently bound Client instance // passing it a top-level Scope. -// Returns EventID if the event was captured successfully, or nil otherwise. +// Returns CheckInID if the check-in was captured successfully, or nil otherwise. func (hub *Hub) CaptureCheckIn(checkIn *CheckIn, monitorConfig *MonitorConfig) *EventID { client, scope := hub.Client(), hub.Scope() if client == nil { return nil } - eventID := client.CaptureCheckIn(checkIn, monitorConfig, scope) - if eventID != nil { - hub.mu.Lock() - hub.lastEventID = *eventID - hub.mu.Unlock() - } - - return eventID + return client.CaptureCheckIn(checkIn, monitorConfig, scope) } // AddBreadcrumb records a new breadcrumb. diff --git a/hub_test.go b/hub_test.go index ac46893c3..23ae3746f 100644 --- a/hub_test.go +++ b/hub_test.go @@ -5,7 +5,6 @@ import ( "fmt" "sync" "testing" - "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -191,18 +190,6 @@ func TestLastEventIDUpdatesAfterCaptures(t *testing.T) { eventID := hub.CaptureEvent(&Event{Message: "wat"}) assertEqual(t, *eventID, hub.LastEventID()) - - checkInID := hub.CaptureCheckIn(&CheckIn{ - MonitorSlug: "job", - Status: CheckInStatusOK, - Duration: time.Second * 10, - }, &MonitorConfig{ - Schedule: CrontabSchedule("8 * * * *"), - CheckInMargin: 100, - MaxRuntime: 200, - Timezone: "Asia/Singapore", - }) - assertEqual(t, *checkInID, hub.LastEventID()) } func TestLastEventIDNotChangedForTransactions(t *testing.T) {