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

fix(crons): Pass correct event types, fix checkin_margin field, add example #674

Merged
merged 5 commits into from
Jul 26, 2023
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
72 changes: 72 additions & 0 deletions _examples/crons/main.go
Original file line number Diff line number Diff line change
@@ -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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this causes issues. In Laravel, we sent the monitor config along for all check-ins.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sorta feels natural to me: when the monitor already exists (guaranteed by the first check-in), we don't have to pass in the monitor config anymore.

)
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 {}
}
2 changes: 1 addition & 1 deletion check_in.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this almost looks like a typo to me. Will raise with the Crons team.

// 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"`
Expand Down
37 changes: 22 additions & 15 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,11 @@ 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
}
return nil
}

// CaptureEvent captures an event on the currently active client if any.
Expand Down Expand Up @@ -532,24 +536,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
}
Expand Down
31 changes: 22 additions & 9 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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)
}
})
Expand Down
11 changes: 2 additions & 9 deletions hub.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,21 +269,14 @@

// 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)

Check warning on line 279 in hub.go

View check run for this annotation

Codecov / codecov/patch

hub.go#L279

Added line #L279 was not covered by tests
}

// AddBreadcrumb records a new breadcrumb.
Expand Down
13 changes: 0 additions & 13 deletions hub_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"sync"
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion testdata/json/checkin/001.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"value": 1,
"unit": "day"
},
"check_in_margin": 5,
"checkin_margin": 5,
"max_runtime": 30,
"timezone": "America/Los_Angeles"
}
Expand Down
2 changes: 1 addition & 1 deletion testdata/json/checkin/002.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"type": "crontab",
"value": "0 * * * *"
},
"check_in_margin": 5,
"checkin_margin": 5,
"max_runtime": 30,
"timezone": "America/Los_Angeles"
}
Expand Down
4 changes: 2 additions & 2 deletions transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,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)
}
Expand Down
30 changes: 30 additions & 0 deletions transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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://[email protected]/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
Expand Down
Loading