Skip to content

Improve OpenTelemetry Interoperability#1201

Open
kraftp wants to merge 9 commits intomainfrom
kraftp/otel
Open

Improve OpenTelemetry Interoperability#1201
kraftp wants to merge 9 commits intomainfrom
kraftp/otel

Conversation

@kraftp
Copy link
Member

@kraftp kraftp commented Mar 10, 2026

Add a new tracingEnabled flag to DBOS configuration. When set, this flag enables DBOS trace generation, but not the DBOS export pipeline. This flag requires an existing TracerProvider to be configured (probably by an APM like dd-trace). Traces will be collected and exported by the existing TracerProvider.

Closes #1199

@yl3w
Copy link

yl3w commented Mar 10, 2026

Thanks for taking a look at my request - which had an error in one of its assumptions. This looks good, i am happy to test this out.

--
As an aside, ymmv on this comment and depends on how you'd like to design the SDK interface

could you avoid introducing the additional flag by relying on the detection on the tracer provider, something like

  const registered = trace.setGlobalTracerProvider(provider);
  if (!registered) {
    // Someone else owns the provider — enable span creation, skip our collector
    globalParams.tracingEnabled = true;
  }

@kraftp
Copy link
Member Author

kraftp commented Mar 10, 2026

Thanks for taking a look at my request - which had an error in one of its assumptions. This looks good, i am happy to test this out.

-- As an aside, ymmv on this comment and depends on how you'd like to design the SDK interface

could you avoid introducing the additional flag by relying on the detection on the tracer provider, something like

  const registered = trace.setGlobalTracerProvider(provider);
  if (!registered) {
    // Someone else owns the provider — enable span creation, skip our collector
    globalParams.tracingEnabled = true;
  }

We could do that, but I'd rather be explicit than implicit. You should need to explicitly opt into DBOS-provided tracing, either with your existing pipeline (by setting tracingEnabled) or through a DBOS-managed pipeline (by setting enableOTLP). I'll update the docs to reflect this.

@kraftp kraftp marked this pull request as ready for review March 10, 2026 20:25
@kraftp
Copy link
Member Author

kraftp commented Mar 10, 2026

Thanks for taking a look at my request - which had an error in one of its assumptions. This looks good, i am happy to test this out.

-- As an aside, ymmv on this comment and depends on how you'd like to design the SDK interface

could you avoid introducing the additional flag by relying on the detection on the tracer provider, something like

  const registered = trace.setGlobalTracerProvider(provider);
  if (!registered) {
    // Someone else owns the provider — enable span creation, skip our collector
    globalParams.tracingEnabled = true;
  }

But yeah, I think the solution is working and ready now, in tests it solves the problem the issue describes, exporting correlated traces to the existing TracerProvider and exporters. Would greatly appreciate it if you could test it and see if it works for you!

@qianl15
Copy link
Member

qianl15 commented Mar 10, 2026

Why do we need a separate tracingEnabled flag? Can we do the same thing as we did in Python by using the enableOTLP flag?

@kraftp
Copy link
Member Author

kraftp commented Mar 10, 2026

Why do we need a separate tracingEnabled flag? Can we do the same thing as we did in Python by using the enableOTLP flag?

That flag already exists. It enables the entire DBOS OTel pipeline. This new flag allows you to get JUST traces, sent to your existing TracerProvider, without spinning up a bunch of redundant infrastructure.

@qianl15
Copy link
Member

qianl15 commented Mar 10, 2026

Why do we need a separate tracingEnabled flag? Can we do the same thing as we did in Python by using the enableOTLP flag?

That flag already exists. It enables the entire DBOS OTel pipeline. This new flag allows you to get JUST traces, sent to your existing TracerProvider, without spinning up a bunch of redundant infrastructure.

Trying to figure out if we want to change Python too.

@kraftp
Copy link
Member Author

kraftp commented Mar 10, 2026

Why do we need a separate tracingEnabled flag? Can we do the same thing as we did in Python by using the enableOTLP flag?

That flag already exists. It enables the entire DBOS OTel pipeline. This new flag allows you to get JUST traces, sent to your existing TracerProvider, without spinning up a bunch of redundant infrastructure.

Trying to figure out if we want to change Python too.

Let's confirm the fix in TS first.

const provider = new NodeTracerProvider({
spanProcessors: [new SimpleSpanProcessor(memoryExporter)],
});
provider.register();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the provider is first-registered win, then how does the test work?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wouldn't it work?

@qianl15
Copy link
Member

qianl15 commented Mar 10, 2026

Why do we need a separate tracingEnabled flag? Can we do the same thing as we did in Python by using the enableOTLP flag?

That flag already exists. It enables the entire DBOS OTel pipeline. This new flag allows you to get JUST traces, sent to your existing TracerProvider, without spinning up a bunch of redundant infrastructure.

Trying to figure out if we want to change Python too.

Let's confirm the fix in TS first.

The flags are kind of confusing... I'm starting to think maybe instead of multiple boolean variables, we should explicitly set them to several literals like "DBOS_EXPORTERS", "TRACE_ONLY_EXTERNAL", "LOGS_ONLY_EXTERNAL", etc.

@kraftp
Copy link
Member Author

kraftp commented Mar 11, 2026

Why do we need a separate tracingEnabled flag? Can we do the same thing as we did in Python by using the enableOTLP flag?

That flag already exists. It enables the entire DBOS OTel pipeline. This new flag allows you to get JUST traces, sent to your existing TracerProvider, without spinning up a bunch of redundant infrastructure.

Trying to figure out if we want to change Python too.

Let's confirm the fix in TS first.

The flags are kind of confusing... I'm starting to think maybe instead of multiple boolean variables, we should explicitly set them to several literals like "DBOS_EXPORTERS", "TRACE_ONLY_EXTERNAL", "LOGS_ONLY_EXTERNAL", etc.

This would be a breaking change. If we end up needing multiple additional modes, we'll change the interface, but two is fine I think.

@yl3w
Copy link

yl3w commented Mar 11, 2026

But yeah, I think the solution is working and ready now, in tests it solves the problem the issue describes, exporting correlated traces to the existing TracerProvider and exporters. Would greatly appreciate it if you could test it and see if it works for you!

I will test this soon and get back. Wednesdays are usually very busy, might be a day or two.

@kraftp
Copy link
Member Author

kraftp commented Mar 11, 2026

But yeah, I think the solution is working and ready now, in tests it solves the problem the issue describes, exporting correlated traces to the existing TracerProvider and exporters. Would greatly appreciate it if you could test it and see if it works for you!

I will test this soon and get back. Wednesdays are usually very busy, might be a day or two.

Thanks, let me know if this fixes your issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: Respect Existing OpenTelemetry TracerProvider

3 participants