chore(internal): move rate limiter to PyO3

[brettlangdon]

This PR sets up a new ddtrace.internal._core module which is a Rust module defined in src/core/ and uses PyO3.

The first component being moved over is the ddtrace.internal.rate_limiter.RateLimter.

This is a well isolated component which has minimal need to be in Python. There are clear performance gains from moving this component to native.

The main motivation from this change is to start to build the basis for adding/moving performance critical components to PyO3.

Risks

This introduces a requirement on the rust compiler for building from source. We have built this into our dev image for awhile, but there are other places where we do not yet have the proper setup and so processes may fail.

For example, the benchmarking platform image does not have rust compiler yet.

Testing

Since we kept the same public interface as the original Python module, we are using the existing Python tests as the validation of this change. They should be comprehensive enough to validate the new native version.

Benchmarks

The benchmarking image is not yet updated to include rust compiler, so the following results are from running locally on my machine only.

The new rate_limiter benchmark shows a roughly 4x performance improvement.

Benchmark	main	rate_limiter
ratelimiter-defaults	732 ns	178 ns: 4.12x faster
ratelimiter-no_rate_limit	267 ns	174 ns: 1.54x faster
ratelimiter-low_rate_limit	724 ns	175 ns: 4.13x faster
ratelimiter-high_rate_limit	870 ns	178 ns: 4.89x faster
ratelimiter-short_window	726 ns	177 ns: 4.10x faster
ratelimiter-long_window	737 ns	177 ns: 4.16x faster
Geometric mean	(ref)	3.60x faster

This is a great improvement, however, the rate limiter is such a small portion of an actual trace, because right now it's biggest impact is on starting a new trace and making a sampling decision. So applications with the biggest possible improvement are those which start a lot of small traces at high volume.

Benchmarks for the span suite show a minor improvement, this is because the rate limiter today only takes up a small portion of the total lifecycle of a span.

Benchmark	main	rate_limiter
span-add-metrics	39.2 ms	38.3 ms: 1.02x faster
span-start-finish	16.6 ms	15.9 ms: 1.05x faster
span-start-finish-traceid128	17.8 ms	16.5 ms: 1.08x faster
Geometric mean	(ref)	1.02x faster

Benchmarks for the tracer suite showing similar results to span.

Benchmark	main	rate_limiter
tracer-small	94.0 us	91.2 us: 1.03x faster
tracer-medium	844 us	818 us: 1.03x faster
Geometric mean	(ref)	1.02x faster

tracer-large results are not show a statistically significant enough difference in overhead.

Benchmarks for the flask_simple suite showing almost no improvement at all.

Benchmark	main	rate_limiter
flasksimple-tracer	1.13 ms	1.15 ms: 1.02x slower
flasksimple-debugger	566 us	573 us: 1.01x slower
flasksimple-appsec-post	1.03 ms	1.05 ms: 1.02x slower
flasksimple-appsec-telemetry	1.15 ms	1.17 ms: 1.01x slower
Geometric mean	(ref)	1.01x slower

Follow-up future work

• Move the RateLimiter tests over to rust
• Add Rust formatting, static analysis, testing steps to the CI process

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.

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

[brettlangdon]

I am having an issue with a test, I cannot figure out how to resolve.

https://app.circleci.com/pipelines/github/DataDog/dd-trace-py/61223/workflows/603631bb-3f6b-4b64-8f12-de4bc2a5f46a/jobs/3836322

FAILED tests/internal/test_serverless.py::test_slow_imports - ImportError: PyO3 modules compiled for CPython 3.8 or older may only be initialized once per interpreter process
ERROR tests/internal/test_forksafe.py::test_double_fork - RuntimeError: ModuleWatchdog is already installed
ERROR tests/internal/test_forksafe.py::test_rlock_fork - RuntimeError: ModuleWatchdog is already installed
ERROR tests/internal/test_forksafe.py::test_rlock_basic - RuntimeError: ModuleWatchdog is already installed
ERROR tests/internal/test_forksafe.py::test_event_basic - RuntimeError: ModuleWatchdog is already installed
ERROR tests/internal/test_forksafe.py::test_event_fork - RuntimeError: ModuleWatchdog is already installed
ERROR tests/internal/test_forksafe.py::test_forksafe - RuntimeError: ModuleWatchdog is already installed
ERROR tests/internal/test_forksafe.py::test_method_usage - RuntimeError: ModuleWatchdog is already installed
ERROR tests/internal/test_forksafe.py::test_duplicates - RuntimeError: ModuleWatchdog is already installed
ERROR tests/internal/test_forksafe.py::test_lock_fork - RuntimeError: ModuleWatchdog is already installed
ERROR tests/internal/test_forksafe.py::test_hook_exception - RuntimeError: ModuleWatchdog is already installed
ERROR tests/internal/test_forksafe.py::test_lock_basic - RuntimeError: ModuleWatchdog is already installed
ERROR tests/internal/test_forksafe.py::test_registry - RuntimeError: ModuleWatchdog is already installed

[datadog-dd-trace-py-rkomorn]

Datadog Report

Branch report: brettlangdon/core.rate_limiter
Commit report: 6a64a0f
Test service: dd-trace-py

✅ 0 Failed, 174625 Passed, 1086 Skipped, 11h 50m 47.27s Total duration (18m 55.5s time saved)

[brettlangdon]

cc @emmettbutler

Should we move this to ddtrace/internal/core/_core.so and then re-export under ddtrace/internal/core/__init__.py. to keep everything under ddtrace.internal.core.

This change is made.

[emmettbutler]

Having these files live in internal/core would follow a pattern that's used for other subpackages containing native code, making this new code easier to learn. Is that technically straightforward enough to try?

[brettlangdon]

Easy enough to try.

I think the general pattern other repositories take is:

- package_name
  -  __init__.py
  - _native.so
- src
  - native.c
- pyproject.toml

https://github.com/pola-rs/polars/tree/main/py-polars

Just because others are doing it, doesn't mean we need to.

Eventually I'd like to see core stuff broken out into it's own package, rather than module.

[brettlangdon]

Something feels weird about it to me, but yes we can. Let me know if you'd prefer me to update it to nest under internal/core/