Disallow new Span()

[dyladan]

Currently it is possible to call new Span(). We should make the constructor private to disallow this.

Current test usages:

• 1 use of new span in contrib tests: https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-dns/test/functionals/utils.test.ts#L150
• ? uses of new Span in JS tests
• honeycomb distro is known to use Span in tests
• dd-trace may also use new Span in tests? @dyladan will look into this

Possible additional goals:

• do not use a class for span (improve minification)
• create test utility library

[JamieDanielson]

Note: To do this, we can make Span private and for TypeScript we should be able to access with obj[“somePrivateProperty”]

[pichlermarc]

We may also be able to convert uses of Span to ReadableSpan wherever interfaces expect a ReadableSpan instead of an actual SDK span.

[david-luna]

Couple of questions:

• it should be no problem to leave usage of new Span in sdk-trace-base tests right?
• should sdk-trace-base export the Span interface from @opentelemetry/api rather than the Span class?

[pichlermarc]

• it should be no problem to leave usage of new Span in sdk-trace-base tests right?
• should sdk-trace-base export the Span interface from @opentelemetry/api rather than the Span class?

Hmm, though questions.

I think the underlying goal should maybe rather be: don't export the Span class from @opentelemetry/sdk-trace-base at all, instead export an interface SdkSpan extends ReadableSpan, APISpan (this may need changes to ReadableSpan to align the spanContext property).

There's nothing wrong with keeping the Span type for internal use in that package, but we should reference the interface wherever possible, not the concrete class.

When going this route, the answer to the questions would be:

it should be no problem to leave usage of new Span in sdk-trace-base tests right?

yes, leaving it is no problem

should sdk-trace-base export the Span interface from @opentelemetry/api rather than the Span class?

Yes, wherever this fulfills the goals of the interface.

An API span is not readable. That's so that users don't use attributes they added to a span in their business logic to convey information, which makes a registered Otel SDK required for their app to function, but business logic should not be impacted, whether you have an OTel SDK registered or not.

In some SDK interfaces, the Span is readable on purpose though. SpanProcessor is an example - you need to be able to read the span to process it. We should provide access to the minimum interface to get the job done 🙂

[david-luna]

@pichlermarc I think this one can be closed.