getsentry / team-sdks

A meta repository for tracking work across all SDK teams.
0 stars 0 forks source link

Project: Tracing without Performance #5

Open HazAT opened 1 year ago

HazAT commented 1 year ago

Project Board

See our project board to track this initiative across all SDKs

### Web Frontend SDKs
- [ ] https://github.com/getsentry/sentry-javascript/issues/8352
- [ ] https://github.com/getsentry/sentry-electron/issues/688
### Web Backend SDKs
- [ ] https://github.com/getsentry/sentry-go/issues/656
- [ ] https://github.com/getsentry/sentry-php/issues/1525
- [ ] https://github.com/getsentry/sentry-python/issues/2128
- [ ] https://github.com/getsentry/sentry-java/issues/2768
- [ ] https://github.com/getsentry/sentry-laravel/issues/713
- [ ] https://github.com/getsentry/sentry-symfony/issues/735
- [ ] https://github.com/getsentry/sentry-dotnet/issues/2447
- [ ] https://github.com/getsentry/sentry-ruby/issues/2056
- [ ] https://github.com/getsentry/sentry-unity/issues/1395
### Mobile SDKs
- [ ] https://github.com/getsentry/sentry-cocoa/issues/3136
- [ ] https://github.com/getsentry/sentry-dart/issues/1541
- [ ] https://github.com/getsentry/sentry-react-native/issues/3166

Description

Internal Notion Page: https://www.notion.so/sentry/Tracing-without-performance-efab307eb7f64e71a04f09dc72722530

To always have access to a trace and span ID, add a new internal PropagationContext property to the scope, an object holding a traceId and spanId and an optional dynamicSamplingContext.

Populate the traceId and spanId with valid, random IDs on construction.

You may update the traceId and dynamicSamplingContext from the request headers during an incoming request or if the process was exposed to a SENTRY_TRACE and/or SENTRY_BAGGAGE environment variable if performance is disabled.

The Dynamic Sampling Context should be lazily constructed once needed, using a new fromOptions function that constructs the DSC based on the client options if performance is disabled.

When captureEvent is called, you may use these values on the scope as a fallback to construct the trace context and envelope header item if no transaction exists in the SDK.

For outgoing HTTP requests, use the values in the scope as a fallback if no transaction is present. The sentry-trace will contain a trailing -0 indicating an unsampled transaction.

### Task List for each SDK
- [ ] All events are always sent through the `/envelope` endpoint if performance is enabled. (If possible just always use `/envelope`)
- [ ] Add a mechanism to continue a trace from environment variables.
- [ ] All **error**, **transaction** and **check-in** events sent to Sentry always contain a Dynamic Sampling Context (trace) envelope header item.
- [ ] All **error, transaction** and **check-in** sent to Sentry always contain [a `trace` context](https://develop.sentry.dev/sdk/event-payloads/contexts/#trace-context).
- [ ] All outgoing **HTTP requests** always contain a `sentry-trace` and `baggage` header if the destination is allow-listed in [[tracePropagationTargets](https://develop.sentry.dev/sdk/performance/#tracepropagationtargets)](https://develop.sentry.dev/sdk/performance/#tracepropagationtargets).
- [ ] Create detailed docs about how to setup distributed tracing
- [ ] Set the `replay` context (Python & PHP only) https://develop.sentry.dev/sdk/event-payloads/contexts/#replay-context
sl0thentr0py commented 1 year ago

So I am fine with doing something quick right now to unblock replay without needing majors. But eventually, we should solve this properly in majors by decoupling Propagation from start_transaction and having a separate well-defined abstraction as discussed with @cleptric and @AbhiPrasad ealrier.

HazAT commented 1 year ago

@sl0thentr0py full ack - eventually, with Starfish and potentially having just Spans, we are going to revamp this and make it way more straightforward and also internally less complex.

HazAT commented 1 year ago

ref: https://github.com/getsentry/sentry/issues/45529

bruno-garcia commented 1 year ago

(consider enableTracing)

I recall proposing this earlier when it was being introduced. My understanding is that today this means tracesSampleRate=1.0 which means turning that on by itself will cause someone to burn through their quota quickly. Does it mean the documentation would need to be:

enableTracing=true;
tracesSampleRate=0

?

HazAT commented 1 year ago

@bruno-garcia yes or a new flag - or just tracing origin setting 🙃

bruno-garcia commented 1 year ago

For the Replay 'Connect to Backend Errors' feature to light up, we need https://github.com/getsentry/team-webplatform-meta/issues/45 which doesn't depend on this work. Through https://github.com/getsentry/team-webplatform-meta/issues/45 we at least get errors on backend errors when Performance monitoring is turned on.