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

feat: Tracing without Performance #1516

Merged
merged 34 commits into from Jun 12, 2023

Conversation

cleptric
Copy link
Member

@cleptric cleptric commented Apr 18, 2023

TL;DR

These changes will kick off a movement where the performance feature is no longer required to implement distributed tracing.

At its core is a new PropagationContext, an object holding all required properties to implement distributed tracing in Sentry: a trace ID, span ID, parent span ID, and a Dynamic Sampling Context.

The PropagationContext is attached to each scope and is used as a fallback if tracing is not enabled (Options::isTracingEnabled() is false).

New API

Naming is not final!

Sentry\continueTrace

A new top-level function to continue a trace. This function is context-aware, as in it will either mutate the PropagationContext on the scope or return a populated TransactionContext if tracing is enabled, based on the values of a passed in sentry-trace and baggage header.
While this is not a great API, we have to maintain BC and are willing to live with this for now.

Sentry\traceparent

A new top-level function to receive the trace context to be used as an HTML meta tag for SSR applications or as a header value to be attached to outgoing HTTP requests. This function is context-aware, as in it will either return the trace context of the current span if tracing is enabled or fall back to this value from the PropagationContext stored on the scope.

Sentry\baggage

A new top-level function to receive the Dynamic Sampling Context to be used as an HTML meta tag for SSR applications or as a header value to be attached to outgoing HTTP requests. This function is context-aware, as in it will either return the Dynamic Sampling Context of the current transaction if tracing is enabled or fall back to this value from the PropagationContext stored on the scope.

Each event always contains a trace context

If tracing is disabled, we will now use the PropagationContext as a fallback to attach a trace context to each event, in the case of this SDK, to all error, transaction, and check-in events.

Each event always contains a Dynamic Sampling Context (trace) envelope header item

As this SDK still uses the /store endpoint if tracing is disabled, we can only attach the DSC to check-in events in this case.

DynamicSamplingContext::fromOptions

To propagate a Dynamic Sampling Context in case tracing is disabled, a new method was added that constructs the Dynamic Sampling Context from the current options of the client.

Part of #1525

@cleptric cleptric force-pushed the tracing-without-performance-hub-based branch 8 times, most recently from 3ed04a4 to 2ff8748 Compare April 24, 2023 13:27
@cleptric cleptric changed the title feat: Tracing without perf experiments feat: Tracing without Performance May 8, 2023
@cleptric cleptric self-assigned this May 8, 2023
@cleptric cleptric linked an issue May 8, 2023 that may be closed by this pull request
@cleptric cleptric force-pushed the tracing-without-performance-hub-based branch from 2ff8748 to 7943096 Compare May 8, 2023 00:19
@cleptric cleptric force-pushed the tracing-without-performance-hub-based branch 2 times, most recently from af0d12c to 2dddafb Compare May 16, 2023 15:21
@cleptric cleptric marked this pull request as ready for review June 12, 2023 08:48
@cleptric cleptric requested a review from stayallive June 12, 2023 08:56
Copy link
Collaborator

@stayallive stayallive left a comment

Choose a reason for hiding this comment

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

I have a hard time finding any wrong coce-wise. Looks good!

Just a single TODO we (might) need to address but otherwise good to go!

src/Tracing/GuzzleTracingMiddleware.php Show resolved Hide resolved
@cleptric cleptric merged commit 6b17289 into master Jun 12, 2023
27 checks passed
@cleptric cleptric deleted the tracing-without-performance-hub-based branch June 12, 2023 13:41
@cleptric cleptric mentioned this pull request Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracing without Performance
2 participants