feat(tracing): deprecate [] notation for DD_TRACE_METHODS

[erikayasuda]

Description

This PR deprecates the [] notation for DD_TRACE_METHODS in favor of the : notation.

When customers tried to add django_q methods to DD_TRACE_METHODS, calling __import__ during bootstrapping to figure out whether the provided method was a module or class method caused boostrapping errors. This is because modules like django_q attempt to do configurations upon attempting to import, which we don't want happening during bootstrapping.

The notation change for DD_TRACE_METHODS is primarily to help us distinguish between the legacy/deprecated and new semantics for DD_TRACE_METHODS. With the deprecated semantics, we needed to guess what the base module was by importing, since only the method name is within the brackets even if it's actually a class method. With the new semantics, we are asking customers to include the class with the method name if it's a class method.

Example:

DD_TRACE_METHODS="mymodule.mysubmodule.myclass[myfunc,otherfunc];..."

Here we would attempt to __import__(mymodule.mysubmodule.myclass), fail because the class is included, then re-attempt with __import__(mymodule.mysubmodule), which would succeed and indicate the base module is only mymodule.mysubmodule, and the functions are a class function called myclass.myfunc and myclass.otherfunc.

With the new semantics, this guess work would be taken out.

DD_TRACE_METHODS="mymodule.mysubmodule:myclass.myfunc,myclass.otherfunc;..."

Here we know for sure the base module is just mymodule.mysubmodule, and the function names are also explicitly listed.

Risks

This is a customer facing change, so respective updates to customer facing documentation are included. After the next major release, customers will be required to switch to the updated semantics.

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

[datadog-dd-trace-py-rkomorn]

Datadog Report

Branch report: erikayasuda/dd_trace_methods_syntax
Commit report: 53f4ace
Test service: dd-trace-py

✅ 0 Failed, 4 Passed, 348 Skipped, 24.85s Total duration (8m 18.74s time saved)

[erikayasuda]

@DataDog/apm-tees This change is the first step towards deprecating the legacy DD_TRACE_METHODS syntax that was causing some problems for customers trying to trace django_q methods. Details for the change are outlined in the PR description.

[emmettbutler]

🎉