fix(profiling): vendor protobuf

[emmettbutler]

This change fixes issues like #3766 in which incompatibility can arise between the version of protobuf that ddtrace brings with it and the version that the customer's app uses. In practice these issues occur under lib-injection, because in such scenarios ddtrace's requirements are finalized before the app's requirements are known. The reason this change fixes such issues is that it avoids using the google.protobuf module that the application may care about, instead using its own ddtrace.vendor.protobuf.google.protobuf module.

The fix is to vendor the most recent release of protobuf, and use the vendored version for all Python runtime versions that support it. Experimentally, this set of versions seems to be 3.8+. The behavior of the protobuf library under 3.7 is unchanged in this pull request. Vendoring protobuf for 3.7 would require adding another copy of the protobuf code to the repo.

The code in vendor/protobuf was taken from an unzipped wheel of 5.26.0 downloaded from PyPI. I chose this method instead of building from source in setup.py because protobuf has a complex build process managed by Bazel, and I didn't want to add Bazel as a dependency of ddtrace's build process.

There are changes to the riotfile to stop installing protobuf in test environments that will use the vendored version. There is also a new pprof file added that imports the vendored code. This file was created by copypasting the existing pprof file with the highest number and changing the imports.

Passing existing functional tests is sufficient because there's no new functionality added here.

Files you can ignore in code review:

• ddtrace/vendor
• *_pb2*
• Riot lockfiles

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

[pr-commenter]

Benchmarks

Benchmark execution time: 2024-03-29 19:27:55

Comparing candidate commit 89fc729 in PR branch emmett.butler/protobuf-vendor with baseline commit 907b7e8 in branch main.

Found 14 performance improvements and 11 performance regressions! Performance is the same for 176 metrics, 9 unstable metrics.

scenario:coreapiscenario-context_with_data_no_listeners

• 🟥 max_rss_usage [+508.096KB; +739.136KB] or [+2.391%; +3.479%]

scenario:coreapiscenario-core_dispatch_no_listeners

• 🟥 max_rss_usage [+450.597KB; +722.907KB] or [+2.115%; +3.393%]

scenario:coreapiscenario-core_dispatch_with_results_no_listeners

• 🟥 max_rss_usage [+625.515KB; +722.479KB] or [+2.948%; +3.406%]

scenario:flasksimple-debugger

• 🟥 execution_time [+345.541µs; +390.467µs] or [+5.493%; +6.207%]

scenario:httppropagationextract-datadog_tracecontext_tracestate_not_propagated_on_trace_id_no_match

• 🟩 max_rss_usage [-746.930KB; -498.664KB] or [-3.408%; -2.275%]

scenario:httppropagationextract-datadog_tracecontext_tracestate_propagated_on_trace_id_match

• 🟥 max_rss_usage [+679.630KB; +757.247KB] or [+3.213%; +3.580%]

scenario:httppropagationextract-invalid_span_id_header

• 🟥 max_rss_usage [+616.615KB; +703.936KB] or [+2.910%; +3.322%]

scenario:httppropagationextract-large_valid_headers_all

• 🟥 max_rss_usage [+641.265KB; +732.124KB] or [+3.028%; +3.457%]

scenario:httppropagationextract-medium_valid_headers_all

• 🟥 max_rss_usage [+645.229KB; +718.739KB] or [+3.049%; +3.397%]

scenario:httppropagationextract-none_propagation_style

• 🟥 max_rss_usage [+530.846KB; +850.325KB] or [+2.533%; +4.057%]

scenario:httppropagationextract-tracecontext_headers

• 🟩 max_rss_usage [-777.691KB; -681.304KB] or [-3.557%; -3.116%]

scenario:httppropagationextract-valid_headers_all

• 🟩 max_rss_usage [-746.735KB; -554.565KB] or [-3.425%; -2.543%]

scenario:httppropagationextract-wsgi_invalid_priority_header

• 🟥 max_rss_usage [+510.924KB; +717.876KB] or [+2.416%; +3.395%]

scenario:httppropagationextract-wsgi_large_valid_headers_all

• 🟩 max_rss_usage [-712.029KB; -459.018KB] or [-3.275%; -2.111%]

scenario:httppropagationextract-wsgi_medium_valid_headers_all

• 🟩 max_rss_usage [-726.372KB; -503.657KB] or [-3.337%; -2.314%]

scenario:httppropagationextract-wsgi_valid_headers_all

• 🟥 max_rss_usage [+639.313KB; +713.596KB] or [+3.022%; +3.373%]

scenario:httppropagationinject-with_dd_origin

• 🟩 max_rss_usage [-595.004KB; -544.503KB] or [-2.726%; -2.495%]

scenario:otelspan-start

• 🟩 max_rss_usage [-1039.529KB; -835.620KB] or [-3.096%; -2.489%]

scenario:otelspan-start-finish

• 🟩 max_rss_usage [-747.123KB; -674.599KB] or [-3.230%; -2.917%]

scenario:otelspan-start-finish-telemetry

• 🟩 max_rss_usage [-745.294KB; -666.188KB] or [-3.220%; -2.878%]

scenario:sethttpmeta-obfuscation-worst-case-explicit-query

• 🟩 max_rss_usage [-762.097KB; -520.360KB] or [-3.410%; -2.328%]

scenario:span-start-finish-telemetry

• 🟩 max_rss_usage [-655.442KB; -569.262KB] or [-2.992%; -2.599%]

scenario:span-start-finish-traceid128

• 🟩 max_rss_usage [-672.269KB; -601.177KB] or [-3.066%; -2.742%]

scenario:tracer-large

• 🟩 max_rss_usage [-760.947KB; -679.207KB] or [-3.329%; -2.971%]

scenario:tracer-small

• 🟩 max_rss_usage [-746.690KB; -664.382KB] or [-3.414%; -3.038%]

[datadog-dd-trace-py-rkomorn]

Datadog Report

Branch report: emmett.butler/protobuf-vendor
Commit report: e886d7d
Test service: dd-trace-py

✅ 0 Failed, 4037 Passed, 7278 Skipped, 1h 22m 14.22s Total duration (1h 38m 3.62s time saved)

[Kyle-Verhoog]

Awesome, this change ends ends up being surprisingly minimal in terms of change to ddtrace!

I have a concern about ddsketch having a dependency on protobuf which will still be pulled in. I think we could resolve it by storing the ddsketch protobuf in ddtrace and relaxing the protobuf dependency of the ddsketch package.

[emmettbutler]

New plan, after discovering that vendoring protobuf requires a lot more work than previously thought:
@sanchda will update lib-injection/dl_wheels.py and tests/lib-injection such that it avoids loading protobuf whenever possible (under lib-injection and an alternate profiler exporter implementation is available).