From 8a3c4a7c971a03e14c639005f80a79ffef96753a Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 3 Jun 2020 18:51:52 -0400 Subject: [PATCH] feat: Add Transactions to sentry-go (#235) * feat: Add Transaction and Span structs * ref: Update functions to work with transaction events * ref: Update DSN to work with envelope endpoint * test: Add golden test to snapshot test important interfaces * ref: Update transport to use envelopes endpoint Co-authored-by: Rodolfo Carvalho --- client.go | 3 +- dsn.go | 16 +++- dsn_test.go | 21 +++-- interfaces.go | 89 +++++++++++++++---- interfaces_test.go | 142 ++++++++++++++++++++++++++++++ testdata/error_event.golden | 44 +++++++++ testdata/span.golden | 14 +++ testdata/transaction_event.golden | 32 +++++++ transport.go | 46 ++++++++-- transport_test.go | 72 +++++++++++++++ 10 files changed, 446 insertions(+), 33 deletions(-) create mode 100644 testdata/error_event.golden create mode 100644 testdata/span.golden create mode 100644 testdata/transaction_event.golden diff --git a/client.go b/client.go index aaf9590d9..7386bc1be 100644 --- a/client.go +++ b/client.go @@ -380,7 +380,8 @@ func (client *Client) processEvent(event *Event, hint *EventHint, scope EventMod return nil } - if options.BeforeSend != nil { + // As per spec, transactions do not go through BeforeSend. + if event.Type != transactionType && options.BeforeSend != nil { h := &EventHint{} if hint != nil { h = hint diff --git a/dsn.go b/dsn.go index 59946845d..23019ac6d 100644 --- a/dsn.go +++ b/dsn.go @@ -141,9 +141,19 @@ func (dsn Dsn) String() string { return url } -// StoreAPIURL returns assembled url to be used in the transport. -// It points to configures Sentry instance. +// StoreAPIURL returns the URL of the store endpoint of the project associated +// with the DSN. func (dsn Dsn) StoreAPIURL() *url.URL { + return dsn.getAPIURL("store") +} + +// EnvelopeAPIURL returns the URL of the envelope endpoint of the project +// associated with the DSN. +func (dsn Dsn) EnvelopeAPIURL() *url.URL { + return dsn.getAPIURL("envelope") +} + +func (dsn Dsn) getAPIURL(s string) *url.URL { var rawURL string rawURL += fmt.Sprintf("%s://%s", dsn.scheme, dsn.host) if dsn.port != dsn.scheme.defaultPort() { @@ -152,7 +162,7 @@ func (dsn Dsn) StoreAPIURL() *url.URL { if dsn.path != "" { rawURL += dsn.path } - rawURL += fmt.Sprintf("/api/%d/store/", dsn.projectID) + rawURL += fmt.Sprintf("/api/%d/%s/", dsn.projectID, s) parsedURL, _ := url.Parse(rawURL) return parsedURL } diff --git a/dsn_test.go b/dsn_test.go index d1b586193..bbbbd1801 100644 --- a/dsn_test.go +++ b/dsn_test.go @@ -10,9 +10,10 @@ import ( ) type DsnTest struct { - in string - dsn *Dsn // expected value after parsing - url string // expected Store API URL + in string + dsn *Dsn // expected value after parsing + url string // expected Store API URL + envURL string // expected Envelope API URL } //nolint: gochecknoglobals @@ -28,7 +29,8 @@ var dsnTests = map[string]DsnTest{ path: "/foo/bar", projectID: 42, }, - url: "https://domain:8888/foo/bar/api/42/store/", + url: "https://domain:8888/foo/bar/api/42/store/", + envURL: "https://domain:8888/foo/bar/api/42/envelope/", }, "MinimalSecure": { in: "https://public@domain/42", @@ -39,7 +41,8 @@ var dsnTests = map[string]DsnTest{ port: 443, projectID: 42, }, - url: "https://domain/api/42/store/", + url: "https://domain/api/42/store/", + envURL: "https://domain/api/42/envelope/", }, "MinimalInsecure": { in: "http://public@domain/42", @@ -50,7 +53,8 @@ var dsnTests = map[string]DsnTest{ port: 80, projectID: 42, }, - url: "http://domain/api/42/store/", + url: "http://domain/api/42/store/", + envURL: "http://domain/api/42/envelope/", }, } @@ -71,6 +75,11 @@ func TestNewDsn(t *testing.T) { if diff := cmp.Diff(tt.url, url); diff != "" { t.Errorf("dsn.StoreAPIURL() mismatch (-want +got):\n%s", diff) } + // Envelope API URL + url = dsn.EnvelopeAPIURL().String() + if diff := cmp.Diff(tt.envURL, url); diff != "" { + t.Errorf("dsn.EnvelopeAPIURL() mismatch (-want +got):\n%s", diff) + } }) } } diff --git a/interfaces.go b/interfaces.go index 6a1e004a4..c528dadfb 100644 --- a/interfaces.go +++ b/interfaces.go @@ -16,6 +16,9 @@ import ( // Level marks the severity of the event type Level string +// transactionType is the type of a transaction event. +const transactionType = "transaction" + const ( LevelDebug Level = "debug" LevelInfo Level = "info" @@ -139,7 +142,7 @@ type Exception struct { type EventID string -// https://docs.sentry.io/development/sdk-dev/event-payloads/ +// Event is the fundamental data structure that is sent to Sentry. type Event struct { Breadcrumbs []*Breadcrumb `json:"breadcrumbs,omitempty"` Contexts map[string]interface{} `json:"contexts,omitempty"` @@ -163,24 +166,54 @@ type Event struct { Modules map[string]string `json:"modules,omitempty"` Request *Request `json:"request,omitempty"` Exception []Exception `json:"exception,omitempty"` + + // Experimental: This is part of a beta feature of the SDK. The fields below + // are only relevant for transactions. + Type string `json:"type,omitempty"` + StartTimestamp time.Time `json:"start_timestamp"` + Spans []*Span `json:"spans,omitempty"` } +// MarshalJSON converts the Event struct to JSON. func (e *Event) MarshalJSON() ([]byte, error) { - type alias Event - // encoding/json doesn't support the "omitempty" option for struct types. - // See https://golang.org/issues/11939. - // This implementation of MarshalJSON shadows the original Timestamp field - // forcing it to be omitted when the Timestamp is the zero value of - // time.Time. - if e.Timestamp.IsZero() { - return json.Marshal(&struct { - *alias - Timestamp json.RawMessage `json:"timestamp,omitempty"` - }{ - alias: (*alias)(e), - }) + // event aliases Event to allow calling json.Marshal without an infinite + // loop. It preserves all fields of Event while none of the attached + // methods. + type event Event + + // Transactions are marshaled in the standard way how json.Marshal works. + if e.Type == transactionType { + return json.Marshal((*event)(e)) } - return json.Marshal((*alias)(e)) + + // errorEvent is like Event with some shadowed fields for customizing the + // JSON serialization of regular "error events". + type errorEvent struct { + *event + + // encoding/json doesn't support the omitempty option for struct types. + // See https://golang.org/issues/11939. + // We shadow the original Event.Timestamp field with a json.RawMessage. + // This allows us to include the timestamp when non-zero and omit it + // otherwise. + Timestamp json.RawMessage `json:"timestamp,omitempty"` + + // The fields below are not part of the regular "error events" and only + // make sense to be sent for transactions. They shadow the respective + // fields in Event and are meant to remain nil, triggering the omitempty + // behavior. + Type json.RawMessage `json:"type,omitempty"` + StartTimestamp json.RawMessage `json:"start_timestamp,omitempty"` + Spans json.RawMessage `json:"spans,omitempty"` + } + + x := &errorEvent{event: (*event)(e)} + if !e.Timestamp.IsZero() { + x.Timestamp = append(x.Timestamp, '"') + x.Timestamp = e.Timestamp.UTC().AppendFormat(x.Timestamp, time.RFC3339Nano) + x.Timestamp = append(x.Timestamp, '"') + } + return json.Marshal(x) } func NewEvent() *Event { @@ -211,3 +244,29 @@ type EventHint struct { Request *http.Request Response *http.Response } + +// TraceContext describes the context of the trace. +// +// Experimental: This is part of a beta feature of the SDK. +type TraceContext struct { + TraceID string `json:"trace_id"` + SpanID string `json:"span_id"` + Op string `json:"op,omitempty"` + Description string `json:"description,omitempty"` + Status string `json:"status,omitempty"` +} + +// Span describes a timed unit of work in a trace. +// +// Experimental: This is part of a beta feature of the SDK. +type Span struct { + TraceID string `json:"trace_id"` + SpanID string `json:"span_id"` + ParentSpanID string `json:"parent_span_id,omitempty"` + Op string `json:"op,omitempty"` + Description string `json:"description,omitempty"` + Status string `json:"status,omitempty"` + Tags map[string]string `json:"tags,omitempty"` + StartTimestamp time.Time `json:"start_timestamp"` + EndTimestamp time.Time `json:"timestamp"` +} diff --git a/interfaces_test.go b/interfaces_test.go index ad4508ddd..7273e95df 100644 --- a/interfaces_test.go +++ b/interfaces_test.go @@ -1,13 +1,21 @@ package sentry import ( + "encoding/json" + "flag" + "fmt" + "io/ioutil" "net/http/httptest" + "path/filepath" "strings" "testing" + "time" "github.com/google/go-cmp/cmp" ) +var update = flag.Bool("update", false, "update .golden files") //nolint: gochecknoglobals + func TestNewRequest(t *testing.T) { const payload = `{"test_data": true}` got := NewRequest(httptest.NewRequest("POST", "/test/?q=sentry", strings.NewReader(payload))) @@ -29,3 +37,137 @@ func TestNewRequest(t *testing.T) { t.Errorf("Request mismatch (-want +got):\n%s", diff) } } + +func TestEventMarshalJSON(t *testing.T) { + event := NewEvent() + event.Spans = []*Span{{ + TraceID: "d6c4f03650bd47699ec65c84352b6208", + SpanID: "1cc4b26ab9094ef0", + ParentSpanID: "442bd97bbe564317", + StartTimestamp: time.Unix(8, 0).UTC(), + EndTimestamp: time.Unix(10, 0).UTC(), + Status: "ok", + }} + event.StartTimestamp = time.Unix(7, 0).UTC() + event.Timestamp = time.Unix(14, 0).UTC() + + got, err := json.Marshal(event) + if err != nil { + t.Fatal(err) + } + + // Non transaction event should not have fields Spans and StartTimestamp + want := `{"sdk":{},"user":{},"timestamp":"1970-01-01T00:00:14Z"}` + + if diff := cmp.Diff(want, string(got)); diff != "" { + t.Errorf("Event mismatch (-want +got):\n%s", diff) + } +} + +func TestStructSnapshots(t *testing.T) { + testSpan := &Span{ + TraceID: "d6c4f03650bd47699ec65c84352b6208", + SpanID: "1cc4b26ab9094ef0", + ParentSpanID: "442bd97bbe564317", + Description: `SELECT * FROM user WHERE "user"."id" = {id}`, + Op: "db.sql", + Tags: map[string]string{ + "function_name": "get_users", + "status_message": "MYSQL OK", + }, + StartTimestamp: time.Unix(0, 0).UTC(), + EndTimestamp: time.Unix(5, 0).UTC(), + Status: "ok", + } + + testCases := []struct { + testName string + sentryStruct interface{} + }{ + { + testName: "span", + sentryStruct: testSpan, + }, + { + testName: "error_event", + sentryStruct: &Event{ + Message: "event message", + Environment: "production", + EventID: EventID("0123456789abcdef"), + Fingerprint: []string{"abcd"}, + Level: LevelError, + Platform: "myplatform", + Release: "myrelease", + Sdk: SdkInfo{ + Name: "sentry.go", + Version: "0.0.1", + Integrations: []string{"gin", "iris"}, + Packages: []SdkPackage{{ + Name: "sentry-go", + Version: "0.0.1", + }}, + }, + ServerName: "myhost", + Timestamp: time.Unix(5, 0).UTC(), + Transaction: "mytransaction", + User: User{ID: "foo"}, + Breadcrumbs: []*Breadcrumb{{ + Data: map[string]interface{}{ + "data_key": "data_val", + }, + }}, + Extra: map[string]interface{}{ + "extra_key": "extra_val", + }, + Contexts: map[string]interface{}{ + "context_key": "context_val", + }, + }, + }, + { + testName: "transaction_event", + sentryStruct: &Event{ + Type: transactionType, + Spans: []*Span{testSpan}, + StartTimestamp: time.Unix(3, 0).UTC(), + Timestamp: time.Unix(5, 0).UTC(), + Contexts: map[string]interface{}{ + "trace": TraceContext{ + TraceID: "90d57511038845dcb4164a70fc3a7fdb", + SpanID: "f7f3fd754a9040eb", + Op: "http.GET", + Description: "description", + Status: "ok", + }, + }, + }, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.testName, func(t *testing.T) { + got, err := json.MarshalIndent(test.sentryStruct, "", " ") + if err != nil { + t.Error(err) + } + + golden := filepath.Join(".", "testdata", fmt.Sprintf("%s.golden", test.testName)) + if *update { + err := ioutil.WriteFile(golden, got, 0600) + if err != nil { + t.Fatal(err) + } + } + + want, err := ioutil.ReadFile(golden) + if err != nil { + t.Fatal(err) + } + + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("struct %s mismatch (-want +got):\n%s", test.testName, diff) + } + }) + } +} diff --git a/testdata/error_event.golden b/testdata/error_event.golden new file mode 100644 index 000000000..e40b708c0 --- /dev/null +++ b/testdata/error_event.golden @@ -0,0 +1,44 @@ +{ + "breadcrumbs": [ + { + "data": { + "data_key": "data_val" + } + } + ], + "contexts": { + "context_key": "context_val" + }, + "environment": "production", + "event_id": "0123456789abcdef", + "extra": { + "extra_key": "extra_val" + }, + "fingerprint": [ + "abcd" + ], + "level": "error", + "message": "event message", + "platform": "myplatform", + "release": "myrelease", + "sdk": { + "name": "sentry.go", + "version": "0.0.1", + "integrations": [ + "gin", + "iris" + ], + "packages": [ + { + "name": "sentry-go", + "version": "0.0.1" + } + ] + }, + "server_name": "myhost", + "transaction": "mytransaction", + "user": { + "id": "foo" + }, + "timestamp": "1970-01-01T00:00:05Z" +} \ No newline at end of file diff --git a/testdata/span.golden b/testdata/span.golden new file mode 100644 index 000000000..f8fdd8e0b --- /dev/null +++ b/testdata/span.golden @@ -0,0 +1,14 @@ +{ + "trace_id": "d6c4f03650bd47699ec65c84352b6208", + "span_id": "1cc4b26ab9094ef0", + "parent_span_id": "442bd97bbe564317", + "op": "db.sql", + "description": "SELECT * FROM user WHERE \"user\".\"id\" = {id}", + "status": "ok", + "tags": { + "function_name": "get_users", + "status_message": "MYSQL OK" + }, + "start_timestamp": "1970-01-01T00:00:00Z", + "timestamp": "1970-01-01T00:00:05Z" +} \ No newline at end of file diff --git a/testdata/transaction_event.golden b/testdata/transaction_event.golden new file mode 100644 index 000000000..7e13b653e --- /dev/null +++ b/testdata/transaction_event.golden @@ -0,0 +1,32 @@ +{ + "contexts": { + "trace": { + "trace_id": "90d57511038845dcb4164a70fc3a7fdb", + "span_id": "f7f3fd754a9040eb", + "op": "http.GET", + "description": "description", + "status": "ok" + } + }, + "sdk": {}, + "timestamp": "1970-01-01T00:00:05Z", + "user": {}, + "type": "transaction", + "start_timestamp": "1970-01-01T00:00:03Z", + "spans": [ + { + "trace_id": "d6c4f03650bd47699ec65c84352b6208", + "span_id": "1cc4b26ab9094ef0", + "parent_span_id": "442bd97bbe564317", + "op": "db.sql", + "description": "SELECT * FROM user WHERE \"user\".\"id\" = {id}", + "status": "ok", + "tags": { + "function_name": "get_users", + "status_message": "MYSQL OK" + }, + "start_timestamp": "1970-01-01T00:00:00Z", + "timestamp": "1970-01-01T00:00:05Z" + } + ] +} \ No newline at end of file diff --git a/transport.go b/transport.go index 34ae64594..c8ac67cb6 100644 --- a/transport.go +++ b/transport.go @@ -4,6 +4,8 @@ import ( "bytes" "crypto/tls" "encoding/json" + "errors" + "fmt" "net/http" "net/url" "strconv" @@ -92,6 +94,40 @@ func getRequestBodyFromEvent(event *Event) []byte { return nil } +func getEnvelopeFromBody(body []byte, now time.Time) *bytes.Buffer { + var b bytes.Buffer + fmt.Fprintf(&b, `{"sent_at":"%s"}`, now.UTC().Format(time.RFC3339Nano)) + fmt.Fprint(&b, "\n", `{"type":"transaction"}`, "\n") + b.Write(body) + return &b +} + +func getRequestFromEvent(event *Event, dsn *Dsn) (*http.Request, error) { + body := getRequestBodyFromEvent(event) + if body == nil { + return nil, errors.New("event could not be marshalled") + } + + if event.Type == transactionType { + env := getEnvelopeFromBody(body, time.Now()) + request, _ := http.NewRequest( + http.MethodPost, + dsn.EnvelopeAPIURL().String(), + env, + ) + + return request, nil + } + + request, _ := http.NewRequest( + http.MethodPost, + dsn.StoreAPIURL().String(), + bytes.NewBuffer(body), + ) + + return request, nil +} + // ================================ // HTTPTransport // ================================ @@ -187,17 +223,11 @@ func (t *HTTPTransport) SendEvent(event *Event) { return } - body := getRequestBodyFromEvent(event) - if body == nil { + request, err := getRequestFromEvent(event, t.dsn) + if err != nil { return } - request, _ := http.NewRequest( - http.MethodPost, - t.dsn.StoreAPIURL().String(), - bytes.NewBuffer(body), - ) - for headerKey, headerValue := range t.dsn.RequestHeaders() { request.Header.Set(headerKey, headerValue) } diff --git a/transport_test.go b/transport_test.go index fe260e89e..6234c314a 100644 --- a/transport_test.go +++ b/transport_test.go @@ -5,10 +5,13 @@ import ( "fmt" "net/http" "net/http/httptest" + "strings" "sync" "sync/atomic" "testing" "time" + + "github.com/google/go-cmp/cmp" ) type unserializableType struct { @@ -125,6 +128,75 @@ func TestGetRequestBodyFromEventCompletelyInvalid(t *testing.T) { } } +func TestGetEnvelopeFromBody(t *testing.T) { + body := getRequestBodyFromEvent(&Event{ + Type: transactionType, + Spans: []*Span{}, + StartTimestamp: time.Unix(3, 0).UTC(), + Timestamp: time.Unix(5, 0).UTC(), + }) + env := getEnvelopeFromBody(body, time.Unix(6, 0)) + got := env.String() + //nolint: lll + want := strings.Join([]string{ + `{"sent_at":"1970-01-01T00:00:06Z"}`, + `{"type":"transaction"}`, + `{"sdk":{},"timestamp":"1970-01-01T00:00:05Z","user":{},"type":"transaction","start_timestamp":"1970-01-01T00:00:03Z"}`, + }, "\n") + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Event mismatch (-want +got):\n%s", diff) + } +} + +func TestGetRequestFromEvent(t *testing.T) { + testCases := []struct { + testName string + // input + event *Event + // output + apiURL string + }{ + { + testName: "Sample Event", + event: NewEvent(), + apiURL: "https://host/path/api/42/store/", + }, + { + testName: "Transaction", + event: func() *Event { + event := NewEvent() + event.Type = transactionType + + return event + }(), + apiURL: "https://host/path/api/42/envelope/", + }, + } + + for _, test := range testCases { + test := test + dsn, err := NewDsn("https://key@host/path/42") + if err != nil { + t.Fatal(err) + } + + t.Run(test.testName, func(t *testing.T) { + req, err := getRequestFromEvent(test.event, dsn) + if err != nil { + t.Fatal(err) + } + + if req.Method != http.MethodPost { + t.Errorf("Request %v using http method: %s, supposed to use method: %s", req, req.Method, http.MethodPost) + } + + if req.URL.String() != test.apiURL { + t.Errorf("Incorrect API URL. want: %s, got: %s", test.apiURL, req.URL.String()) + } + }) + } +} + func TestRetryAfterNoHeader(t *testing.T) { r := http.Response{} assertEqual(t, retryAfter(time.Now(), &r), time.Second*60)