feat(http): allow to disable span without parent for outgoing requests

[vmarchaud]

I'm trying to setup tracing across my microservices (~10) and one is sending periodic request to internal GCP stuff.
I know i could ignore them one by one using the ignoreOutgoingUrls option but that would not really be future proof in case new code get added in the underlying library i'm using :/
I can't use something like ignoreOutgoingUrls: /.*/ because that would also disable spans created inside a trace.

My request would be to add an option called disableOutgoingTraces: true that would just avoid creating a new span if none exist in the context. WDYT ?

I'm happy to make the PR ofc

[dyladan]

This is a topic the spec has been talking about recently actually. They want to create a context var that disables tracing and I have a nodejs prototype. Works like this:

context.with(disableTracing(context.active()), () => {
	// do something here with tracing disabled
})

does that cover your use case?

[vmarchaud]

context.with(disableTracing(context.active()), () => {
// do something here with tracing disabled
})

Not really because the span are created by the http plugin, not my own code. However in other cases, i agree that this feature would be helpful for end users

[dyladan]

Where are you suggesting this option? Are you trying to disable the http plugin entirely? I guess i'm not fully understanding the usecase here.

[vmarchaud]

I'm trying to disable the creation of new span for outgoing request when there are no context/no parent. That would require to add a check here like so:

const currentSpan = plugin.tracer.getCurrentSpan()
if (plugin.options.disableOutgoingTraces && currentSpan === null) {
 return original.apply(this, arguments)
}

[dyladan]

Ah I understand. i think disableOutgoingTraces is just a little misleading because it implies all traces are disabled. I think disableOutgoingRootSpans or something similar that makes it clear the only spans being disabled are ones with no parent.

[Flarna]

I think it would make sense that each plugin has an requiresParent (or whatever name sounds better).
Each plugin should use a reasonable default value which may differ from plugin to plugin.

[vmarchaud]

Will open PRs for express and http tomorrow then :)

[dyladan]

Will open PRs for express and http tomorrow then :)

It might make more sense to introduce this as a single PR that adds the option to all plugins with a per-plugin default value. e.g. express would default to not creating root spans, and http would default to creating them.

[Flarna]

Just realized that there is just a single http plugin, not one for server and one for client. Therefore I think a single setting is too limiting for the http plugin.

[dyladan]

Having a single plugin for client and server is, in my opinion, a mistake anyways. it may be too late to fix though

[Flarna]

Other plugins for outgoing requests like MongoDb requires always a parent (see https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-plugin-mongodb/src/mongodb.ts#L110).
I think the default is ok here but an options would be nice.

[vmarchaud]

Just realized that there is just a single http plugin, not one for server and one for client. Therefore I think a single setting is too limiting for the http plugin.

We can totally have one option for both, i will implement that.

[pauldraper]

Spec discussion about disabling via context: open-telemetry/opentelemetry-specification#530

[vmarchaud]

#948 has been merged, closing