chore: make span.sampled reference span.context.sampling_priority and deprecate it

[ZStriker19]

With span.sampled being outdated, and no longer reliable as a way to tell if a span is actually being sampled, we make this change to make its value reference span.context.sampling_priority which is what actually matters when it comes to sampling. This deprecates span.sampled and removes its use internally, replacing it with checks for span.context.sampling_priority to mimic behavior.

If the value of span.context.sampling_priority, is None we know that sampling hasn't yet run, and then maintain the original behavior of span.sampled being True until sampling is run, if it's >0 we know that the span has been sampled, if <=0 we know the span has not been sampled.

Checklist

• Change(s) are motivated and described in the PR description
• Testing strategy is described if automated tests are not included in the PR
• Risks are described (performance impact, potential for breakage, maintainability)
• Change is maintainable (easy to change, telemetry, documentation)
• Library release note guidelines are followed or label changelog/no-changelog is set
• Documentation is included (in-code, generated user docs, public corp docs)
• Backport labels are set (if applicable)
• If this PR changes the public interface, I've notified @DataDog/apm-tees.
• If change touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.

Reviewer Checklist

• Title is accurate
• All changes are related to the pull request's stated goal
• Description motivates each change
• Avoids breaking API changes
• Testing strategy adequately addresses listed risks
• Change is maintainable (easy to change, telemetry, documentation)
• Release note makes sense to a user of the library
• Author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment
• Backport labels are set in a manner that is consistent with the release branch maintenance policy

[ZStriker19]

Or do I make everything reference _sampling_priority_v1? and then deprecate span.sampled?

[datadog-dd-trace-py-rkomorn]

Datadog Report

Branch report: zachg/have_sampled_reference_sampling_priority_v1
Commit report: 719ef33
Test service: dd-trace-py

✅ 0 Failed, 26960 Passed, 140954 Skipped, 2h 55m 23.33s Total duration (6h 12m 38.2s time saved)

[emmettbutler]

Want a review before fixing the test failures?

[ZStriker19]

nope sorry, thought it was there, but realized a critical flaw.

[pr-commenter]

Benchmarks

Benchmark execution time: 2024-02-27 20:00:44

Comparing candidate commit 4c88a54 in PR branch zachg/have_sampled_reference_sampling_priority_v1 with baseline commit acc0297 in branch main.

Found 3 performance improvements and 5 performance regressions! Performance is the same for 111 metrics, 9 unstable metrics.

scenario:coreapiscenario-context_with_data_no_listeners

• 🟥 max_rss_usage [+705.416KB; +837.957KB] or [+2.412%; +2.866%]

scenario:coreapiscenario-core_dispatch_listeners

• 🟥 max_rss_usage [+657.052KB; +819.556KB] or [+2.246%; +2.802%]

scenario:coreapiscenario-core_dispatch_no_listeners

• 🟥 max_rss_usage [+680.495KB; +844.036KB] or [+2.329%; +2.889%]

scenario:coreapiscenario-set_item

• 🟥 max_rss_usage [+611.161KB; +797.044KB] or [+2.091%; +2.727%]

scenario:httppropagationinject-ids_only

• 🟩 max_rss_usage [-749.993KB; -654.526KB] or [-2.504%; -2.185%]

scenario:httppropagationinject-with_tags_max_size

• 🟩 max_rss_usage [-757.150KB; -676.860KB] or [-2.525%; -2.257%]

scenario:sethttpmeta-obfuscation-regular-case-implicit-query

• 🟥 max_rss_usage [+603.832KB; +741.704KB] or [+2.024%; +2.486%]

scenario:sethttpmeta-useragentvariant_not_exists_2

• 🟩 max_rss_usage [-999.950KB; -680.639KB] or [-3.290%; -2.239%]

[Yun-Kim]

LGTM for ml_obs 👍