DSC Mutability and Measurements

[AbhiPrasad]

Problem statement:

Mis-match between transaction name set in DSC at beginning of trace and in transaction payloads later in trace. This is occurring at a high enough rate to indicate we cannot reliably sample based on trace.

Proposal:

Adjust the mutability of the DSC in an attempt to get a more accurate transaction name in the DSC of the overall trace, for sampling decisions to be more accurately reflect what users configure on product side.

To that end we would also like to gather metrics to further drive what types of changes, how often, and when they occur in the wild, so we can better determine when DSC should be "frozen"

Tasks:

Todo

In Progress

Done

• Experiment with metadata spec @AbhiPrasad draft: Add metadata around transaction name changes #5709
• Update transaction to record metadata around name changes feat(types): Add TransactionNameChange interface #5714
• Only record transaction name changes if data is different fix(tracing): Only record transaction name changes if data is different #5728
• Track name changes in transaction class ref(tracing): Record transaction name changes #5723
• Update tracing sdk to record propagations ref(tracing): Add propagations to tracing SDK #5719
• Update node sdk to record propagations ref(node): Add propagations to http integration #5720
• Update core event processing to record transaction name changes feat(core): Set custom transaction source for event processors #5722
• Update Relay to record new metadata measurements @jan-auer feat(protocol): Track transaction name changes relay#1466
• make DSC mutable (unfreeze) only in JS (@lforst) ref: Make dynamic sampling context mutable #5710
• Update TransactionNameChange interface to track prev source as per Relay changes ref(tracing): Use previous source when logging name changes #5733

---

Diagram for visual learners (courtesy of @jan-auer):

[AbhiPrasad]

Current state: https://develop.sentry.dev/sdk/event-payloads/transaction/#transaction-annotations

I'm proposing we update the transaction_info field to be like so

interface TransactionInfo {
  source: TransactionSource;
  // unix timestamp of first name change
  first_name_change?: number;
  // number of propogations that occured before initial name change
  // only set if `first_name_change` is defined
  num_initial_propogations?: number;
  // number of times name was changed
  // only set if `first_name_change` is defined
  name_change_count?: number;
}

[AbhiPrasad]

Opened #5681, #5682, #5683 to make it easier to add the transaction info changes

[AbhiPrasad]

Note: we'll be doing this only in JS for now, and will evaluate bringing this to other SDKs based on what the metrics we collect say.

[lobsterkatie]

Current state: develop.sentry.dev/sdk/event-payloads/transaction/#transaction-annotations

I'm proposing we update the transaction_info field to be like so

interface TransactionInfo {
  source: TransactionSource;
  // unix timestamp of first name change
  first_name_change?: number;
  // number of propogations that occured before initial name change
  // only set if `first_name_change` is defined
  num_initial_propogations?: number;
  // number of times name was changed
  // only set if `first_name_change` is defined
  name_change_count?: number;
}

Two thoughts here:

• Is it better to store firstNameChange in absolute or relative time? Clearly one is computable from the other, but if what we really want to know is how feasible/advisable it might be to set a limit on how long the DSC is mutable (which would be an amount of time rather than a timestamp), it might be easier to just store the data that way to begin with.

• I think it's important for us also to store the total number of propagations. Both of these measurements are really answers to the question of how far through a transaction we are before the name is finalized (to get a sense of how much we'll be missing by letting some propagations go out with non-final DSC data), and for that we need to know the total extent of the transaction.

[Lms24]

Hmm I'm not sure about the proposed interface. To me it's a little "opinionated" in the sense, that we could collect more general data which could allow us to analyze the timing and stuff in Relay/wherever. What about something like this:

interface TransactionInfo {
  source: TransactionSource;
  nameChanges?: TransactionNameChange[]
  // only set if `nameChanges` is defined and not empty (we can also put this into TransactionNameChange)
  num_initial_propagations?: number;
}

type TransactionNameChange = {
  name: string, // new name
  timestamp: number // unix timestamp when the name was changed (or just the delta to the tx start)
  source: TransactionSource, // optionally, the new source
}

I think this shouldn't be harder/more complex to implement than the other interface but provides us more data

Anyway, feel free to go with whatever makes more sense. Just my 2 cents before I'm off for vacation 😎

[AbhiPrasad]

I think let's settle on something like this, per @Lms24 great suggestion above.

interface TransactionInfo {
  source: TransactionSource;
  name_changes?: TransactionNameChange[]
}

interface TransactionNameChange {
  name: string; // new name
  timestamp: number // unix timestamp when the name was changed (or just the delta to the tx start)
  source?: TransactionSource; // new source
  propagations: number; number of propagations since start of transaction.
}

[lobsterkatie]

I'm good with the above change, but I also still think it's important to store the total number of propagations. As Jan said in the meeting, what we ultimately want is a percentage.

[AbhiPrasad]

Final spec:

export interface TransactionInfo {
    source: TransactionSource;
    changes: TransactionNameChange[];
    propagations: number;
};

/**
 * Object representing metadata about when a transaction name was changed.
 */
export interface TransactionNameChange {
  /**
   * Unix timestamp when the name was changed. Same type as the start and
   * end timestamps of a transaction and span.
   */
  timestamp: number;

  /** Previous source used before transaction name change */
  source: TransactionSource;

  /** Number of propagations since start of transaction */
  propagations: number;
}

Implementation in Relay: getsentry/relay#1466

[AbhiPrasad]

Feature has been released with https://github.com/getsentry/sentry-javascript/releases/tag/7.13.0