feat: Add Transactions to sentry-go #235

AbhiPrasad · 2020-06-01T21:38:51Z

Motivation

This PR aims to add the concept of a transaction to sentry-go. This is important for us so that we can leverage the sentry-go transport in the otel-collector. This also setups the interfaces so that we can possibly add AM support to sentry-go in the future.

Implementation

We add a struct type Span to represent a Sentry span. We also edit the Event struct to add three new fields, StartTimestamp, Spans, and Type. By setting Type == "transaction", users will send transaction events.

The transport code was also changed to account for this. If a type == "transaction" is detected, it will use the envelopes endpoint instead.

Testing

A few unit tests have been added to test the new functions added. This functionality was also demoed with the collector, and was found to be fully functional with no regressions found.

AbhiPrasad · 2020-06-02T16:49:03Z

Seems CI is broken due to go-errors/errors#27

dsn.go

interfaces.go

interfaces_test.go

transport.go

transport_test.go

- fix: Change how DSN API URL is formatted - test: Don't calculate DSN every loop in transport test - test: Compare envelope payload using cpm.Diff

AbhiPrasad · 2020-06-03T14:16:25Z

@rhcarvalho This is ready for another pass.

Summary of changes since last review:

I adjusted the event marshalJSON to account for transaction specific fields (type, start_timestamp, spans). I also added new tests to account for this new functionality in interfaces_test.go
I made the change from using %v to %s in the DSN fmt
I adjusted the transport test to not re-parse DSN every test case loop
~~I changed the envelope payload test to compare bytes, but still kept some functionality from before to account for the fact that the sent_at field is set using time.Now()~~ Updated the envelope creation function and test
I fixed up comments and formatting

The changes were again tested with the collector and found to be working 👍

transport.go

transport_test.go

transport.go

- Move new types down - Sort +- logically Span fields - set `omitempty` for `Span.Status`

To keep code related to DSN manipulation consistently together.

The object is to document better what is being done there and why, and to avoid repeating tricks to work around differences between error events and transactions. Also fixed tests that were broken by my previous commits directly on GitHub (change of order in struct fields change serialized JSON). Also removed optional newline at the end of the envelope -- less bytes is better? A typical request payload does not require the extra new line, and the net/http library handles HTTP protocol under the hood (e.g. setting Content-Length header).

With the line in, the comment does not show up in godoc output.

rhcarvalho

@AbhiPrasad I think we're almost done. Please have a review yourself too. I have one comment that could be addressed in a follow up.

rhcarvalho · 2020-06-03T18:34:40Z

transport.go

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


There are two builtin transport implementations, we need this to be applied to HTTPSyncTransport too, so it becomes an alternative to send transactions/envelopes too.

Will address this in next PR

* 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 <[email protected]>

AbhiPrasad added 2 commits May 29, 2020 09:30

feat: Add transaction

b23d6a8

ref: Append Event struct with transaction

fbdbc36

AbhiPrasad force-pushed the abhi/feat/transactions branch from 1d9eeea to fbdbc36 Compare June 1, 2020 21:46

test: Send transactions through envelopes

4f758d8

AbhiPrasad changed the title ~~[WIP] feat: Add Transactions to sentry-go~~ feat: Add Transactions to sentry-go Jun 2, 2020

AbhiPrasad marked this pull request as ready for review June 2, 2020 13:12

AbhiPrasad requested a review from rhcarvalho June 2, 2020 13:12

AbhiPrasad added 2 commits June 2, 2020 09:13

test: fix CI errors with UTC

0d0bf23

fix: Lint errors

30a42c7

AbhiPrasad force-pushed the abhi/feat/transactions branch from 2c8d11e to 30a42c7 Compare June 2, 2020 14:28