Upgrade Datadog plugin to ddtrace 4.x public API#1563
Upgrade Datadog plugin to ddtrace 4.x public API#1563Dev-iL wants to merge 2 commits intoapache:mainfrom
Conversation
64b35ca to
1dcfe4a
Compare
skrawcz
left a comment
There was a problem hiding this comment.
Great work upgrading the Datadog plugin to the ddtrace 4.x API! The migration from ddtrace.Span/ddtrace.context to ddtrace.trace.Span/ddtrace.trace.Context is clean and correct. The tests are well-structured and provide good coverage of the core lifecycle.
One blocking issue: the uv.lock file at the repository root appears to be accidentally included — it's 9,425 lines and doesn't exist on main. The root of the repository doesn't use uv for dependency management (only ui/backend/ does). Please remove it from the PR (git rm uv.lock and amend or add a new commit). It accounts for 9,425 of the 9,585 additions.
Suggestions (non-blocking):
-
Consider pinning to
ddtrace>=4.0,<5.0instead of unbounded>=4.0— the jump from 2.x→4.x already demonstrated that ddtrace major versions can have breaking import changes. An upper bound would prevent future breakage without requiring another PR. -
In
test_serialize_deserialize_span_dict, you could additionally assert the type:assert isinstance(deserialized["key1"], Context)to verify the deserialized values are actuallyContextinstances, not just that the key exists.
Also a nice bonus: the sf-hamilton → apache-hamilton references and the ddog → datadog extra name fix in the docstring. 👍
Local test results: 7/7 tests pass with ddtrace 4.8.0, ruff check and format both pass clean.
ddtrace 3.0 removed `Span` and `context` from the top-level package. Migrate to the canonical `ddtrace.trace` module, bump the version constraint to `>=4.0,<5`, fix stale `sf-hamilton` package references, and add smoke tests for the plugin. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1dcfe4a to
a53314e
Compare
|
@skrawcz Feedback addressed + fixed some random CI issues. |
related: #1288
fixes: #1562
ddtrace 3.0 removed
Spanandcontextfrom the top-level package. Migrate to the canonicalddtrace.tracemodule, bump the version constraint to>=4.0, fix stalesf-hamiltonpackage references, and add smoke tests for the plugin.Summary
h_ddog.pyimports from removed top-levelddtrace.Span/ddtrace.contextto the canonicalddtrace.tracemodule (ddtrace.trace.Span,ddtrace.trace.Context)ddtrace<3.0toddtrace>=4.0in both thedatadogoptional dependency and thedocsdependency groupsf-hamiltonpackage references toapache-hamiltonin the ImportError message and DDOGTracer docstringtests/plugins/test_h_ddog.py)Motivation
ddtrace 3.0 removed
Spanandcontextfrom the top-levelddtracepackage (DataDog/dd-trace-py#12186). The Hamilton plugin was pinned toddtrace<3.0as a temporary workaround. This PR migrates to the stable public API atddtrace.trace, which is the canonical location in ddtrace 4.x.Changes
hamilton/plugins/h_ddog.pyfrom ddtrace import Span, context, tracersplit intofrom ddtrace import tracer+from ddtrace.trace import Context, Span;context.Contextreplaced withContextin type annotations and constructor calls; TODO comment removed;sf-hamiltonreferences updated toapache-hamiltonpyproject.tomlddtrace<3.0changed toddtrace>=4.0in[project.optional-dependencies].datadoganddocstests/plugins/test_h_ddog.pyNo breaking changes
The public API surface of
DDOGTracerandAsyncDDOGTraceris unchanged: same constructor signatures, same methods, same base class hierarchy. Only internal import paths changed.Test plan
DDOGTracerspan lifecycle:before_graph->before_node->after_node->after_graph__exit__before_task/after_taskAsyncDDOGTracerinstantiation with all parameter combinations__getstate__/__setstate__serialization round-trip_serialize_span_dict/_deserialize_span_dictround-tripddtrace<3.0pins orsf-hamiltonreferences remainChecklist