fix: overwrite DSC sample_rate when sampling

[Flash0ver]

Fixes #4392

• fix: overwrite DSC sample_rate when sampling #4392

---

Overwrite Dynamic Sampling Context (DSC) sample_rate when sampling decision is made

Fixes: https://linear.app/getsentry/project/update-the-dscs-sample-rate-e4e8398e6acc/overview
See also: getsentry/team-sdks#117

Symptom

The Trace-View's extrapolation is based on the "sample_rate" of the Dynamic Sampling Context.

Changes

When a DSC is available, then its immutability is ignored and its sample_rate overwritten

1. when TracesSampler is used and applied, then its resulting Sample-Rate is set
2. when TracesSampleRate is used and applied, that Sample-Rate is set
   • unless ITransactionContext.IsSampled is set (when an upstream sampling decision has been made), then the DSC "sample_rate" is kept unchanged

Also, refactored test/Sentry.Tests/HubTests.cs

• replaced hub.ConfigureScope with hub.ScopeManager.ConfigureScope for assertions
  • the former catches all exceptions, hiding assertion failures
  • the latter does not catch any exception, surfacing assertion failures
  • see also fix: overwrite DSC sample_rate when sampling #4374 (comment)
• made some tests more explicit / consistent

Unchanged

• no changes to making the sampling decision
• no changes to setting the SampleRate on neither the resulting TransactionTracer nor UnsampledTransaction
• possible follow-up: feat: Forcing a Sampling Decision #4386

Note

The sample_rate overwritten in the DSC is now the same SampleRate set on the resulting TransactionTracer or UnsampledTransaction (TransactionTracer.SampleRate / UnsampledTransaction.SampleRate) with the following precedence

1. when SentryOptions.TracesSampler reaches a sampling decision (is not null and returns not null)
   • overwrite DSC
2. when ITransactionContext.IsSampled is set (an upstream sampling decision has been made)
   • keep DSC as is
3. when SentryOptions.TracesSampleRate reaches a sampling decision (is not null)
   • overwrite DSC
4. otherwise
   • keep DSC as is

[jamescrosswell]

TLDR; I think we can remove the if block here.

Long version

Apologies, I think I got us confused on Slack:

• context is of type ITransactionContext... I think when an upstream span is sampled, context.IsParentSampled will be true (not context.IsSampled).
• Whether or not the parent span was sampled is probably not relevant. It's possible that our process has a different sample-rate configured to the upstream node... as long as we use the same sample rand, we maximise (not guarantee) the chance of having a complete trace. If we have a sample-rand of 0.5 and the upstream node has a sample rate of 0.75 but we have a sample rate of 0.25 (and assuming I've understood the requirements) I think we need to sample out in this case and put our own sample-rate (0.25) on the header... so we just overwrite in this scenario.
  • I'm not 100% sure whether that's how sampling is implemented at Sentry though... maybe we only support different sample rates on different nodes via the TracesSampler function. @cleptric do you know?
• If context.IsSampled == true I think that means the SDK user has forced sampling to true (passing it explicitly via the call to StartTransaction).
  • Note that we do sometimes set this to false ourselves internally (e.g. here and here)
  • This is what I think I got us confused about. I suggested this field might be true when the upstream span was sampled in... with fresh eyes, I think I was mistaken and that's what the IsParentSampled field is for.

On the bright side, if all of the above is correct, it means it should be trivial to implement the logic to force sampling (per the other SDKs)... basically we can force a sample-rate of 0.0 or 1.0 if context.IsSampled is set.

[Flash0ver]

followed-up via #4386