Add Datadog tracing interceptor contrib package - #1778
Conversation
|
|
|
We cannot review without CLA acceptance. |
| ) | ||
|
|
||
| __all__ = [ | ||
| "DatadogTracingInterceptor", |
There was a problem hiding this comment.
From our perspective I think it would be ideal to (perhaps additionally) expose it as a plugin. That has a few benefits: a consistent way of registering it with other plugins, and data on plugin usage in temporal cloud.
| ) | ||
| return DatadogTracingWorkflowInboundInterceptor | ||
|
|
||
| def _configure_workflow_tracing(self): |
There was a problem hiding this comment.
This doesn't really seem like a configure method since it just returns things.
| input.unsafe_extern_functions[ | ||
| "__temporal_datadog_configure_workflow_tracing" | ||
| ] = self._configure_workflow_tracing | ||
| # NOTE: fork-specific workaround; reconsider if this integration is ever proposed upstream. |
There was a problem hiding this comment.
I'd have to play around with it to be sure, but I think you could potentially remove some or all of the extern functions if you use the plugin pattern to mark this and/or datadog itself as pass through:
def workflow_runner(runner: WorkflowRunner | None) -> WorkflowRunner:
if not runner:
raise ValueError(
"No WorkflowRunner provided to the OpenTelemetry plugin."
)
# If in sandbox, add additional passthrough
if isinstance(runner, SandboxedWorkflowRunner):
return dataclasses.replace(
runner,
restrictions=runner.restrictions.with_passthrough_modules(
"opentelemetry"
),
)
return runner
Above example from the otel plugin.
|
|
||
| ### Difference from the upstream OpenTelemetry interceptor | ||
|
|
||
| The upstream OpenTelemetry tracing interceptor works around the sandbox |
There was a problem hiding this comment.
This is true of the older interceptor, but not of the newer one.
| results, but the interceptor code runs first — without the guard, a fresh span | ||
| would be emitted for every replayed command. | ||
|
|
||
| Queries and update validators are never suppressed: queries are not in |
There was a problem hiding this comment.
Just FYI if you wanted to use shared code at some point, is_replaying_history_events expresses this difference. It's false during query and update validators but true during workflow replay, so it is safe in both.
tconley1428
left a comment
tconley1428
left a comment
There was a problem hiding this comment.
Overall it seems like it works, my high level feedback is that it's better than the older otel interceptor in python, but doesn't provide quite as much functionality as the newer one. You can decide whether or not to try to accomplish that though.
The newer one allows the user to create custom otel spans during their workflow code, which I don't believe this allows for ddtrace. If that's something we'd eventually want to enable, we may want to do that now as it is a pretty big change to the design. If we only want these built in spans, this approach is likely suitable.
3 participants
This is a port of Datadog's internal Python tracing interceptor, used internally at Datadog with the Python SDK, adapted for open-source contribution. It has feature parity with the Go SDK's Datadog tracing interceptor, including deterministic span/trace IDs and replay-safe span emission, so traces stay coherent across worker restarts regardless of which SDK produced them.