feat: Initial tracing implementation #285

rhcarvalho · 2020-10-16T16:54:34Z

This adds the StartSpan function and related APIs to the SDK.

The initial support focuses on manual instrumentation and HTTP servers
based on net/http.

Tracing is opt-in. Use one of the new options, TracesSampleRate or
TracesSampler, when initializing the SDK to enable sending transactions
and spans to Sentry.

The tracing APIs rely heavily on the standard Context type from Go, and
integrate with the SDKs notion of scopes.

See example/http/main.go for an example of how the new APIs are meant to
be used in practice.

While the basic functionality should be in place, more features are
planned for later.

rhcarvalho

Notes for myself / reviewers.

rhcarvalho · 2020-12-01T23:53:27Z

README.md

-		}
-	}
-}
-```


Removing this example from the README to keep the document more concise. The example is available in full in the example directory, as well as we have some "how to get started" info both in doc comments for the package and in the official docs.

rhcarvalho · 2020-12-01T23:59:25Z

example/http/main.go

-// This is an example web server to demonstrate how to instrument web servers
-// with Sentry.
+// This is an example web server to demonstrate how to instrument error and
+// performance monitoring with Sentry.


🔔 The changes in this file / example should give a taste of what performance instrumentation feels like.

rhcarvalho · 2020-12-02T00:22:18Z

http/sentryhttp.go

@@ -83,11 +83,20 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc {
 		hub := sentry.GetHubFromContext(ctx)
 		if hub == nil {
 			hub = sentry.CurrentHub().Clone()
+			ctx = sentry.SetHubOnContext(ctx, hub)


⚠️ This line moved.

Before, we'd unconditionally called SetHubOnContext, perhaps creating one unnecessary level of contexts (contexts are like a stack, and there would be two stack items with values pointing to the same hub).

Now, we only update the context if there was no hub previously stored. This is the common case and the reason why the old behavior probably did not affect many people.

What should not have changed is that every HTTP handler in user code will be guaranteed to have a hub ready for use. That is, GetHubFromContext never returns nil inside of an handler.

rhcarvalho · 2020-12-02T00:27:21Z

hub.go

@@ -367,6 +367,15 @@ func GetHubFromContext(ctx context.Context) *Hub {
 	return nil
 }

+// hubFromContext returns either a hub stored in the context or the current hub.
+// The return value is guaranteed to be non-nil, unlike GetHubFromContext.
+func hubFromContext(ctx context.Context) *Hub {


This variation of GetHubFromContext is useful in various span methods, because when doing manual instrumentation it makes sense that StartSpan, etc, should always find a hub.

I decided not to export for now as users can still use GetHubFromContext which has more appropriate semantics in HTTP handlers: handlers should never accidentally use the global hub.

rhcarvalho · 2020-12-02T00:29:48Z

internal/crypto/randutil/randutil_test.go

+			t.Fatalf("out of range [0.0, 1.0): %f", n)
+		}
+	}
+	// TODO: verify that distribution is uniform


FWIW, I've verified manually, didn't put time onto a formal validation.

rhcarvalho · 2020-12-02T00:35:18Z

traces_sampler.go

+
+// The TracesSamplerFunc type is an adapter to allow the use of ordinary
+// functions as a TracesSampler.
+type TracesSamplerFunc func(ctx SamplingContext) bool


This should make it easy for people to write their own sampling logic if they wish.

rhcarvalho · 2020-12-02T00:37:22Z

traces_sampler.go

+
+// fixedRateSampler is a TracesSampler that samples root spans randomly at a
+// uniform rate.
+type fixedRateSampler struct {


We'll probably export this sampler in a later release to allow people to compose logic in their custom sampler.

rhcarvalho · 2020-12-02T00:50:24Z

tracing_test.go

+	// child := span.StartChild("top")
+	// SpanCheck{
+	// 	RecorderLen: 1,
+	// }.Check(t, child)


This test is just a stub for SpanFromContext. Contents are commented out because spanFromContext doesn't return a "good enough dummy" to allow for the flow described by the test.

In a future pass through the tracing implementation, we either remove spanFromContext and everything related to it, or have to implement it such to make this test pass.

doc.go

kamilogorek · 2020-12-02T15:44:11Z

tracing.go

+	for _, option := range options {
+		option(&span)
+	}


How does it work exactly? I've seen you do scope.SetTransaction() somewhere as the option to this call.

Short version:

span := sentry.StartSpan(ctx, "http.server", sentry.TransactionName(fmt.Sprintf("%s %s", r.Method, r.URL.Path)), sentry.ContinueFromRequest(r), func(s *sentry.Span) { s.SpanID[4] = 9 s.Description = "my description" // ... }, )

StartSpan takes zero or more functions that take a pointer of span argument (a.k.a. SpanOption type), and those functions can mutate the span.

We provide some helpers like TransactionName and ContinueFromRequest (the equivalent of "FromTraceparent"), but users can also roll their own functions to customize the span, get/set any exported fields.

Long version is https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis

kamilogorek · 2020-12-02T15:46:21Z

tracing.go

+//
+// 	TRACE_ID - SPAN_ID - SAMPLED
+// 	[[:xdigit:]]{32}-[[:xdigit:]]{16}-[01]
+var sentryTracePattern = regexp.MustCompile(`^([[:xdigit:]]{32})-([[:xdigit:]]{16})(?:-([01]))?$`)


Nice, TIL :xdigit:

HazAT

Why don't we have startTransaction?

This adds the StartSpan function and related APIs to the SDK. The initial support focuses on manual instrumentation and HTTP servers based on net/http. Tracing is opt-in. Use one of the new options, TracesSampleRate or TracesSampler, when initializing the SDK to enable sending transactions and spans to Sentry. The tracing APIs rely heavily on the standard Context type from Go, and integrate with the SDKs notion of scopes. See example/http/main.go for an example of how the new APIs are meant to be used in practice. While the basic functionality should be in place, more features are planned for later.

rhcarvalho mentioned this pull request Oct 16, 2020

Tracing examples #286

Closed

jnewland mentioned this pull request Nov 6, 2020

Add transaction and trace info urcomputeringpal/sentry-actions#1

Closed

rhcarvalho linked an issue Nov 23, 2020 that may be closed by this pull request

Sentry metrics #287

Closed

rhcarvalho mentioned this pull request Nov 24, 2020

feat: Add types and rename fields for tracing #280

Merged

rhcarvalho changed the base branch from master to feat/tracing December 1, 2020 22:02

rhcarvalho changed the title ~~Add tracing support~~ feat: Initial tracing implementation Dec 1, 2020

rhcarvalho force-pushed the tracing branch from 9c137c3 to 83afedf Compare December 1, 2020 22:04

rhcarvalho closed this Dec 1, 2020

rhcarvalho deleted the branch master December 1, 2020 22:12

rhcarvalho reopened this Dec 1, 2020

rhcarvalho changed the base branch from feat/tracing to master December 1, 2020 22:16

rhcarvalho force-pushed the tracing branch 4 times, most recently from cbab8df to cca482c Compare December 2, 2020 00:43

rhcarvalho commented Dec 2, 2020

View reviewed changes

rhcarvalho marked this pull request as ready for review December 2, 2020 00:51

rhcarvalho requested review from kamilogorek, bruno-garcia and HazAT December 2, 2020 00:51

kamilogorek reviewed Dec 2, 2020

View reviewed changes

doc.go Outdated Show resolved Hide resolved

kamilogorek reviewed Dec 2, 2020

View reviewed changes

HazAT reviewed Dec 3, 2020

View reviewed changes

rhcarvalho force-pushed the tracing branch from 7b44a67 to 70be529 Compare December 3, 2020 21:07

HazAT previously approved these changes Dec 7, 2020

View reviewed changes

rhcarvalho dismissed HazAT’s stale review via b7b7615 December 7, 2020 17:01

rhcarvalho force-pushed the tracing branch from 70be529 to b7b7615 Compare December 7, 2020 17:01

rhcarvalho merged commit aed9115 into master Dec 7, 2020

rhcarvalho deleted the tracing branch December 7, 2020 17:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Initial tracing implementation #285

feat: Initial tracing implementation #285

rhcarvalho commented Oct 16, 2020 •

edited

Loading

rhcarvalho left a comment

rhcarvalho Dec 1, 2020

rhcarvalho Dec 1, 2020

rhcarvalho Dec 2, 2020

rhcarvalho Dec 2, 2020

rhcarvalho Dec 2, 2020

rhcarvalho Dec 2, 2020

rhcarvalho Dec 2, 2020

rhcarvalho Dec 2, 2020

kamilogorek Dec 2, 2020

rhcarvalho Dec 2, 2020

kamilogorek Dec 2, 2020

HazAT left a comment

feat: Initial tracing implementation #285

feat: Initial tracing implementation #285

Conversation

rhcarvalho commented Oct 16, 2020 • edited Loading

rhcarvalho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HazAT left a comment

Choose a reason for hiding this comment

rhcarvalho commented Oct 16, 2020 •

edited

Loading