Sentry is attaching Tracing Headers without running transactions

DarkByteZero commented 5 months ago

Is there an existing issue for this?

[X] I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
[X] I have reviewed the documentation https://docs.sentry.io/
[X] I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Self-hosted/on-premise

Which SDK are you using?

@sentry/react

SDK Version

7.100.1

Framework Version

React 18.2.0

Link to Sentry event

No response

SDK Setup

Sentry.init({
    dsn: config.sentryDSN,
    environment: deploymentConfig.environment,
    release: config.version || 'local_development',
    ...(config.version && { tunnel: '/sentry' }),
    integrations: [
      Sentry.reactRouterV6BrowserTracingIntegration({
        beforeStartSpan(context: TransactionContext): TransactionContext {
          const modifiedContext = cloneDeep(context);
          // check if transaction name starts with '//' replace with '/' if true
          if (modifiedContext.name.startsWith('//')) {
            modifiedContext.name = modifiedContext.name.replace('//', '/');
          }
          return modifiedContext;
        },
        instrumentNavigation: true,
        instrumentPageLoad: true,
        useEffect,
        useLocation,
        useNavigationType,
        createRoutesFromChildren,
        matchRoutes,
      }),
      Sentry.browserProfilingIntegration(),
      Sentry.replayIntegration({
        maskAllText: false,
        maskAllInputs: true,
        blockAllMedia: false,
        networkDetailAllowUrls: [
          deploymentConfig.baseApiURL,
          deploymentConfig.baseAdminApiURL,
          deploymentConfig.baseAudiencaSidekickURL,
          deploymentConfig.baseMatchlyticsSidekickURL,
        ],
      }),
      Sentry.feedbackIntegration({
        colorScheme: 'system',
        showBranding: false,
        submitButtonLabel: t('framework:sentryUserFeedback.submitButtonLabel'),
        cancelButtonLabel: t('framework:sentryUserFeedback.cancelButtonLabel'),
        formTitle: t('framework:sentryUserFeedback.formTitle'),
        nameLabel: t('framework:sentryUserFeedback.nameLabel'),
        namePlaceholder: t('framework:sentryUserFeedback.namePlaceholder'),
        emailLabel: t('framework:sentryUserFeedback.emailLabel'),
        emailPlaceholder: t('framework:sentryUserFeedback.emailPlaceholder'),
        messageLabel: t('framework:sentryUserFeedback.messageLabel'),
        messagePlaceholder: t('framework:sentryUserFeedback.messagePlaceholder'),
        successMessageText: t('framework:sentryUserFeedback.successMessageText'),
        buttonLabel: t('framework:sentryUserFeedback.supportButton'),
      }),
      extraErrorDataIntegration(),
    ],
    sendDefaultPii: true,
    tracesSampleRate: deploymentConfig.sentryTracesSampleRate,
    normalizeDepth: 20,
    normalizeMaxBreadth: 25,
    replaysSessionSampleRate: deploymentConfig.sentryReplaySampleRate,
    replaysOnErrorSampleRate: 1,
    profilesSampleRate: deploymentConfig.sentryProfilesSampleRate,
    tracePropagationTargets: [
      deploymentConfig.baseApiURL,
      deploymentConfig.baseAdminApiURL,
      deploymentConfig.baseAudiencaSidekickURL,
      deploymentConfig.baseMatchlyticsSidekickURL,
      deploymentConfig.basePortraitShopURL,
      deploymentConfig.baseCallLeaderURL,
      deploymentConfig.baseAppURL,
      deploymentConfig.baseMatchLyticsAssessmentURL,
      deploymentConfig.baseMatchLyticsManagementURL,
      deploymentConfig.baseProfileURL,
      deploymentConfig.baseReflectionURL,
      'localhost',
    ],
  });

Steps to Reproduce

I open my web app, and a pageload transaction is created. Following API Request are sent with the tracing data like this:


Baggage: sentry-environment=dev,sentry-release=local_development,sentry-public_key=xxxxx,sentry-trace_id=acc3a2d245d4404789d4cabe533996d7,sentry-replay_id=312e9506fc0c4ea883aa4b13d9662a77,sentry-sample_rate=1,sentry-sampled=true

Sentry-Trace: acc3a2d245d4404789d4cabe533996d7-bbd6bd5567ae8a87-1


2. after a few seconds, after the page load transaction ends, every new api request still has the tracing headers

Baggage: sentry-environment=dev,sentry-release=local_development,sentry-public_key=xxx,sentry-trace_id=d1f4373147294b14ab220450dc8ca2c6,sentry-replay_id=312e9506fc0c4ea883aa4b13d9662a77

Sentry-Trace: d1f4373147294b14ab220450dc8ca2c6-9f0c782faa520627



### Expected Result

Outgoing API requests should only get Tracing Headers when there is a running transaction.

### Actual Result

All API Requests get Tracing Headers, this leads to many traces with missing root transactions. Because there was none in the web app.

![image](https://github.com/getsentry/sentry-javascript/assets/22873369/97c52e1e-942f-44fa-bc81-1b44231b7312)

AbhiPrasad commented 5 months ago

hey @DarkByteZero - attaching the headers is by intention given distributed tracing does not depend on performance monitoring (generating spans) in Sentry.

When there is no active pageload though the sentry trace should be not attaching a sampled flag. This is indicated by no -1 being attached to d1f4373147294b14ab220450dc8ca2c6-9f0c782faa520627, like we see in the earlier header acc3a2d245d4404789d4cabe533996d7-bbd6bd5567ae8a87-1.

The Go SDK should be parsing a sample rate of 0 if this is dropped, @cleptric could you help sanity check this?

DarkByteZero commented 5 months ago

I appreciate the intention behind attaching headers for distributed tracing, but what is the reason for sending these trace headers, if there isn't even a parent transaction in the sentry system collected. Another Problem is the frequency at with these traces are changed, like in my image you can see that multiple API requests are aggregated into one trace, but there is a big gap between them, this makes it difficult to debug.

lforst commented 5 months ago

@AbhiPrasad @DarkByteZero We need to change how we do trace propagation without active spans in the frontend. The current implementation has issues and leads to traces like @DarkByteZero outlined. These traces have absolutely no value.

Concretely, we have to do one of two things:

Stop having an always-ambient trace propagation context.
Stop picking up the context when performance is enabled.

DarkByteZero commented 5 months ago

hey @DarkByteZero - attaching the headers is by intention given distributed tracing does not depend on performance monitoring (generating spans) in Sentry.

When there is no active pageload though the sentry trace should be not attaching a sampled flag. This is indicated by no -1 being attached to d1f4373147294b14ab220450dc8ca2c6-9f0c782faa520627, like we see in the earlier header acc3a2d245d4404789d4cabe533996d7-bbd6bd5567ae8a87-1.

The Go SDK should be parsing a sample rate of 0 if this is dropped, @cleptric could you help sanity check this?

The GO SDK does not interpret the absence of the sampled flag as a sampled false. Actually, if it would do, all api transactions would not be sampled, which would be bad.

// updateFromSentryTrace parses a sentry-trace HTTP header (as returned by
// ToSentryTrace) and updates fields of the span. If the header cannot be
// recognized as valid, the span is left unchanged. The returned value indicates
// whether the span was updated.
func (s *Span) updateFromSentryTrace(header []byte) (updated bool) {
    m := sentryTracePattern.FindSubmatch(header)
    if m == nil {
        // no match
        return false
    }
    _, _ = hex.Decode(s.TraceID[:], m[1])
    _, _ = hex.Decode(s.ParentSpanID[:], m[2])
    if len(m[3]) != 0 {
        switch m[3][0] {
        case '0':
            s.Sampled = SampledFalse
        case '1':
            s.Sampled = SampledTrue
        }
    }
    return true
}

cleptric commented 5 months ago

@AbhiPrasad the absence of the sampling flag delegates the sampling decision to the receiving SDK. There is a defined order of what takes precedence, defined in https://develop.sentry.dev/sdk/performance/#sampling and implemented here https://github.com/getsentry/sentry-go/blob/master/tracing.go#L427-L511.

lforst commented 5 months ago

I think this issue right here is a symptom of the JS SDK having a buggy/confusing trace propagation when no spans are active. We will fix this in the upcoming major (v8). It will lead to slightly less connected-ness within your data but at least unrelated stuff isn't gonna be connected anymore.

getsentry / sentry-javascript