TransactionTracer.SetData is not equivalent to TransactionTracer.Contexts.Trace.SetData

[Flash0ver]

"data" set via TransactionTracer.SetData is not written to "Data" of the Trace-Context, and does not get copied over to SentryTransaction when created from the tracer.
But "data" set via TransactionTracer.Contexts.Trace.SetData is written to "Data" of the Trace-Context, and gets copied over to SentryTransaction when created from the tracer.

However, SentryTransaction.SetData and SentryTransaction.Contexts.Trace.SetData are equivalent setting "Data" of the Trace-Context.
With Spans, both setting "data" via SpanTracer.SetData and SentrySpan.SetData are eventually serialized to spans.data ("Data" does get copied over to SentrySpan when created from a SpanTracer);

Steps to Reproduce

SentrySdk.Init(options =>
{
    options.Dsn = "..";
    options.TracesSampleRate = 1.0;

    options.SetBeforeSendTransaction(transaction =>
    {
        transaction.SetData("setbeforesendtransaction", "transaction-setdata");
        transaction.Contexts.Trace.SetData("setbeforesendtransaction", "transaction-contexts-trace-setdata");
        return transaction;
    });
});

var transaction = SentrySdk.StartTransaction("my-transaction", "start-transaction");
SentrySdk.ConfigureScope(scope => scope.Transaction = transaction);
transaction.SetData("transaction-data-key", "transaction-data-value");
transaction.SetData("transaction-array", new[] {42, 43, 44});
// Do some work.
transaction.Finish();

Expected Result

{
  "contexts": {
    "trace": {
      "data": {
        "setbeforesendtransaction": "transaction-contexts-trace-setdata",
        "transaction-array": [42, 43, 44],
        "transaction-data-key": "transaction-data-value"
      }
    }
  }
}

Actual Result

{
  "contexts": {
    "trace": {
      "data": {
        "setbeforesendtransaction": "transaction-contexts-trace-setdata"
      }
    }
  }
}

Additional Notes

• Uncovered through Updates the Unity SDK docs with new SetData methods on spans & transa… sentry-docs#13457

[jamescrosswell]

Worth reading the conversations in:

• Emit transaction.data inside contexts.trace.data #3936

@aritchie might have more context as that was his first PR.

[Flash0ver]

Thanks for the context (on the contexts 😉).
Opened a PR containing questions as comments whether I took the right approach.

[jamescrosswell]

btw I think the description for this issue is a bit confusing:

TransactionTracer.SetData is not serialized to contexts.trace.data
SentryTransaction.SetData is serialized to contexts.trace.data

TransactionTracer never gets serialised... it's the in-memory mutable representation of a trace that is used while the trace is active.

Once the trace is finished, it gets turned into a SentryTransaction, which is essentially a (mostly) immutable DTO used to serialise the finalised transaction information and send it across the wire to Sentry.

From the code example you provided, it looks like the issue is more about the distinction between transaction.Contexts.Trace.SetData and transaction.SetData.

From memory, we knew this was inconsistent and intentionally didn't fix it for Transaction because it's likely we're going to get rid of the entire concept of transactions at some point in the near future.

@bruno-garcia @aritchie have I got that completely backwards? The requirements on the first ticket did my head in as well... I'm really not sure what problem we're trying to solve here.

[aritchie]

@jamescrosswell I think you're correct for that exact reasons you stated. There was a lot of confusion when we did that PR, but we decided to leave it.

[Flash0ver]

@jamescrosswell thanks for the hint. I rephrased the summary of the description.
@jamescrosswell @aritchie does this mean it works as intended and we should close this?
Or am I misunderstanding there something?

[jamescrosswell]

@jamescrosswell @aritchie does this mean it works as intended and we should close this?

I think it was more, "This isn't a priority since it's throw away work anyway".

Now that the work is (mostly) done we may as well merge it in. It will give us greater consistency until the Transactions get thrown out and, to be fair, we've been talking about deprecating those for years so your work on this issue may end up adding some value for longer than we think.