DataDog / dd-trace-py

Datadog Python APM Client
https://ddtrace.readthedocs.io/
Other
506 stars 398 forks source link

How to handle deprecation of `ddtrace.context.Context` when it is still present in public API? #9492

Open matka-zn opened 1 month ago

matka-zn commented 1 month ago

Summary of problem

ddtrace.context.Context has been (quietly) deprecated and removed from the package's publicly accessible namespace, but is still present in a lot of public APIs. How should we handle the deprecation?

Which version of dd-trace-py are you using?

2.8.5 (upgraded from 2.6.0)

Description

First a minor sidenote: It would be nice if you could include changes to public API in the release notes, to assist library users in upgrading to new versions. Digging through commits and PRs in the repo reveals that Context was deprecated in the 2.7.0 release.

Another small note: ddtrace.context still exports Context even though it is deprecated, but mypy is not quite happy with the implicit re-export when using strict mode:

from ddtrace.context import Context # mypy: Module "ddtrace.context" does not explicitly export attribute "Context" mypy[attr-defined](https://mypy.readthedocs.io/en/stable/_refs.html#code-attr-defined)

And then for the primary concern: How should we use the public API that takes/returns Context objects when the type is private? Since this is Python we can of course ignore all encapsulation and just import the type anyway, but this is obviously not a reliable way to work with ddtrace. With a static type checker, it is not really possible to annotate functions/types that involve these now-private types.

We have in particular been using the Span.link_span API in a distributed producer/consumer pattern to link traces together by passing the producer's trace context through a queue (sqs+s3) and then linking it to a consumer's trace later to tie together traces for async job processing (which ahem would be doubly-neat if the link could be navigated both ways through the web UI.. but we digress).

The documentation also seems to be not updated (not in the "docs-builds-are-stale" sense, but in the textual-content sense), since it still references and lists ddtrace.context.Context:

Note in the "advanced usage" cases that the various propagation sections still indicate that the Context object needs to be passed around, which seems to be at odds with it being deprecated/private.

Private types in public APIs are a bit weird, and at least throws a wrench in static type checking.

I guess the same concerns apply for Span et al.

Assuming that static type checking is a good thing that we want to keep having, how do we use the ddtrace APIs which involve private types?

emmettbutler commented 4 weeks ago

Thanks for bringing this to our attention @matka-zn. I agree the current state of public/private mixing in the same interface is confusing. We'll align on a recommendation and let you know here.

cc @majorgreys who committed the deprecation change