Always send traces on error

[Aareksio]

Problem Statement

When tracesSampleRate is used, traces may not be included with the error. When working on a real world™ application, the operator may be incentivized to greatly reduce sample rate to prevent clutter and cost overrun. At the same time, traces may prove useful for analyzing the errors.

Solution Brainstorm

Similar concern has already been raised in #10812, for that particular use case, "Session Replay" was a viable solution, the issue was closed as completed. However, JavaScript SDK can also be used in an environment in which Session Replay is not viable - eg. in node.js runtime.

From the discussion in the mentioned issue I understand current architecture decides whether to keep or discard the trace at the head. Perhaps this could be re-evaluated, especially as v9 already seem to include some minor changes to sampling.

The goal with this Feature Request is to keep track of the idea, especially as the competition has introduced similar capability per #10812 (comment)

[Lms24]

Hey @Aareksio thanks for writing in! At this point there are no plans to rework our tracing approach to this behaviour. Among many things, one of the most important reasons as to why we head-sample is that in case of a negative sampling decision, there's much less performance overhead than in a positive decision if we don't actually record all the data points. If we were to sample tail-based within the SDK, you'd always pay the performance overhead price. This is by the way the same in Session Replay to an extent.

I haven't tested this extensively but given that most of OpenTelemetry instrumentation sets a span status, you can give filtering transactions based on their status a try:

Sentry.init({
  tracesSampleRate: 1.0,
  beforeSendTransaction: (txnEvent) => {
    const status = txnEvent.contexts.trace?.status;
    if (!status || status === "ok") {
      return null;
    }
    return txnEvent;
  },
});

I'll close this issue as I don't think we'll change our sampling model in the SDK in near future.

[net]

@Lms24 how will this method affect Sentry's trace metric extrapolation based on sample rate? Is there a way to tell Sentry to exclude a specific transaction and its spans from metric extrapolation?

It seems the ideal way to do this would be to set tracesSampleRate: 1.0, and then to implement your own sampling function inside beforeSendTransaction:

Sentry.init({
  tracesSampleRate: 1.0,
  beforeSendTransaction: (txnEvent) => {
    const status = txnEvent.contexts.trace?.status;

    if (!status || status === "ok") {
      // also, inherit the sampling decision from the parent here

      const sampleRate = 0.01;
      
      if (Math.random() > sampleRate) {
        return null;
      } else {
        // somehow annotate the transaction (and its spans?) with
        // the sample rate
        return txnEvent;
      }
    }

    // somehow annotate the transaction (and its spans?) such
    // that they are excluded from metric extrapolation
    return txnEvent;
  },
});

---

I also disagree that, in most cases, it is important to avoid the overhead of capturing (and then discarding) tracing data in the case of negative sampling decisions. The overhead is negligible compared to the value of capturing traces for every error. Even the Sentry docs recommend capturing 100% of traces:

To give Sentry the fullest, most accurate picture of your application’s health, we recommend sending us 100% of your events. You can do this by setting your tracesSampleRate to 1.0.

Implementing sampling logic in beforeSendTransaction also lets us use the final root span name to make a sampling decision, which is especially important if you want to set a sample rate that's based on something like GraphQL query name, since the initial root span name will look something like POST /api/graphql and only after processing the query will the root span name get updated to something like query FooQuery.

E.g., this example from the docs works:

Sentry.init({
  tracesSampler: ({ name, _attributes, inheritOrSampleWith }) => {
    // Sample all checkout transactions
    if (name.includes("/checkout")) return 1.0;

    // Sample 50% of login transactions
    if (name.includes("/login")) return 0.5;

    // Sample 10% of everything else
    return inheritOrSampleWith(0.1);
  },
});

but this does not:

Sentry.init({
  tracesSampler: ({ name, _attributes, inheritOrSampleWith }) => {
    // Sample all checkout transactions
    if (name.includes("mutation CheckoutMutation")) return 1.0;

    // Sample 50% of login transactions
    if (name.includes("mutation LoginMutation")) return 0.5;

    // Sample 10% of everything else
    return inheritOrSampleWith(0.1);
  },
});

---

I think this issue merits further reconsideration with regards to for support optional tail-sampling. It seems to me that optional tail-sampling, if not already supported, could be supported with very minimal changes.