fix(flask): fix crashes with flask-like frameworks

[sabrenner]

trafficstars

What does this PR do

Adds provide_automatic_options as an extra named argument to patched_add_url_rule. Previously, when using a framework like flask-openapi3, and running the Hello World example linked using ddtrace-run, users would see the following:

File "/Users/sam.brenner/Development/ml-obs/sandboxes/support/app.py", line 18, in <module>
    def get_book(query: BookQuery):
  File "/Users/sam.brenner/Development/ml-obs/sandboxes/support/.venv/lib/python3.10/site-packages/flask_openapi3/scaffold.py", line 182, in decorator
    self._add_url_rule(rule, view_func=view_func, **options)
  File "/Users/sam.brenner/Development/ml-obs/sandboxes/support/.venv/lib/python3.10/site-packages/flask_openapi3/openapi.py", line 411, in _add_url_rule
    self.add_url_rule(rule, endpoint, view_func, provide_automatic_options, **options)
  File "/Users/sam.brenner/Development/ml-obs/sandboxes/support/.venv/lib/python3.10/site-packages/ddtrace/contrib/flask/patch.py", line 433, in patched_add_url_rule
    return _wrap(*args, **kwargs)
TypeError: patched_add_url_rule.<locals>._wrap() takes from 1 to 3 positional arguments but 4 were given

After this change, this error is no longer present.

This is because flask-openapi3 passes in provide_automatic_options as an unnamed argument as the fourth positional argument, but is read on Flask as a keyword arg.

Testing

Added a regression test that instruments a barebones flask-openapi3 app, and runs Datadog instrumentation on it, asserting no errors. Additionally, this change was verified manually in a running app, for both instrumentation and execution.

Checklist

• [x] Change(s) are motivated and described in the PR description
• [x] Testing strategy is described if automated tests are not included in the PR
• [x] Risks are described (performance impact, potential for breakage, maintainability)
• [x] Change is maintainable (easy to change, telemetry, documentation)
• [x] Library release note guidelines are followed or label changelog/no-changelog is set
• [x] Documentation is included (in-code, generated user docs, public corp docs)
• [x] Backport labels are set (if applicable)
• [x] If this PR changes the public interface, I've notified @DataDog/apm-tees.

Reviewer Checklist

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

[datadog-dd-trace-py-rkomorn[bot]]

Datadog Report

Branch report: sabrenner/flaskopenapi3-fix Commit report: 914bec2 Test service: dd-trace-py

:white_check_mark: 0 Failed, 175663 Passed, 1875 Skipped, 10h 12m 15.21s Total duration (31m 26.65s time saved)

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 8.70%. Comparing base (deadfcd) to head (4680bfa). Report is 5 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #9555       +/-   ##
===========================================
- Coverage   75.61%    8.70%   -66.92%     
===========================================
  Files        1336     1349       +13     
  Lines      125991   125223      -768     
===========================================
- Hits        95271    10902    -84369     
- Misses      30720   114321    +83601     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[pr-commenter[bot]]

Benchmarks

Benchmark execution time: 2024-07-08 16:31:52

Comparing candidate commit 914bec2020c71f45fe20ca85658977db70aae290 in PR branch sabrenner/flaskopenapi3-fix with baseline commit 41014bd9392c865e44c90939554894dcce57aa47 in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 221 metrics, 9 unstable metrics.

[sabrenner]

Hey @emmettbutler ! Sorry for delays, I just got back around to this PR today.

I'm surprised that this code change makes a difference in behavior. Could you add a regression test that proves that it has the intended effect?

I added a test. I also had to re-run scripts/ddtest scripts/compile-and-prune-test-requirements, which generated a bunch of LOC for riot requirements files. Let me know if I did this incorrectly or inefficiently, happy to rerun a command if it's doable with a smaller amount of changes 😅

As for the test itself, ran it locally without the changes to patch.py, and had the error show up:

ddtrace_run_python_code_in_subprocess = <function ddtrace_run_python_code_in_subprocess.<locals>._run at 0x108e6df30>

    def test_flask_openapi3_instrumentation(ddtrace_run_python_code_in_subprocess):
        code = """
    from flask_openapi3 import Info, Tag
    from flask_openapi3 import OpenAPI
    
    info = Info(title="my API", version="1.0.0")
    app = OpenAPI(__name__, info=info)
    
    @app.get("/")
    def hello_world():
        return 'Hello, World!'
    """
        env = os.environ.copy()
        out, err, status, pid = ddtrace_run_python_code_in_subprocess(code, env=env)
>       assert status == 0, (out, err)
E       AssertionError: (b'', b'Traceback (most recent call last):
E           File "/private/var/folders/94/lnn2crkx7914c2835zmwrwvh0000gq/T/pytest-of...**kwargs)
E         TypeError: patched_add_url_rule.<locals>._wrap() takes from 1 to 3 positional arguments but 4 were given
E         ')
E       assert 1 == 0

tests/contrib/flask/test_flask_openapi3.py:17: AssertionError

As shown in the CI runs on this PR, with the fix in place, those errors don't show up. Happy to expand on this test if this isn't what you had in mind!