Add some namespacing in API

[Flarna]

• This only affects the JavaScript OpenTelemetry library
• This may affect other libraries, but I would like to get opinions here first

Currently most functions in API are exported on top level. The API covers quite orithogonal topics - currently metrics and traces - but more may come.

Should we consider to add some namespacing to allow us to keep function names short but still have some context to distinguish between metric, trace,... related APIs?

By the way, is there some API polishing round planed before the GA release to cover topics like this? I assume once trace is GA it's hard to change the look and feel.

[dyladan]

We already have this unless I'm misunderstanding what you ask for.

import { trace, metrics } from "@opentelemetry/api";

// These API calls are separated
trace.someApiCall();
metrics.someApiCall(); 

[dyladan]

By the way, is there some API polishing round planed before the GA release to cover topics like this? I assume once trace is GA it's hard to change the look and feel.

If you have suggestions, now is the time to bring them up.

[Flarna]

The most confusing exports for me are context and interface Context (uppercase C).

There are a lot helper exported on top level:

examples for context relevant exports: ROOT_CONTEXT, createContextKey(), getBaggage(), suppressInstrumentation(), ...
examples for trace related exports: isSpanContextValid(), INVALID_SPANID, NOOP_SPAN, NoopSpan, ProxyTracer, StatusCode, ...
examples for metrics exports: ValueType, NOOP_BOUND_COUNTER, NoopBatchObserver, NOOP_METER_PROVIDER, ...
finaly propagation releated exports: defaultTextMapGetter, defaultTextMapSetter

Besides that one could argue that propagation and context is bound to trace.

I assume here that all these top level exports are seen as part of public API and follow semver.

To much namespacing is for sure not the best thing. But I think having e.g. getBaggage and similar somewhere inside context would be useful.
Also generic named types like StatusCode, ValueType could benefit from some limiting scope.

see also #1749 (comment)

[MSNev]

As discussed in the SIG, while simply wrapping functions into a namespace can make it clear, but it has several downsides on the Web community, specifically around minification and tree-shaking.

For minification a simplistic overview of what can't get minified are

• Keywords: this; function; var; true; false; for; arguments; Object*; Array*; etc (https://www.w3schools.com/js/js_reserved.asp)
• Global objects: window; document; navigator; really any Globally defined object.
• Any object property names or functions, whether defined by language or by the code: x.indexOf(..); x.myFunction(); x.myProperty; x.prototype (this INCLUDES namespaces as a namespace is effectively an object)

And on the tree-shaking side of things, when you have functions linked with (on) a namespace (especially helper functions), when that namespace is included into the code, even if you are only using a single function from the namespace all of the code referenced is (by default) not available for removal and therefore the resulting bundle size created by whatever packaging system being used (webpack, rollup, etc) includes all of the functions.

A better suggestion (in my view) would be to following the following

• keep the functions at the top level
• use a prefix on the function name (which means something like tracerSuppressInstrumentation() could get minified to a(); while tracer.suppressInstrumentation() would result in a.suppressInstrumentation())
• If functions do need to exist on an object / class / namespace, keep it as SMALL as possible eg suppressInstrumentation() => suppressInstr() (saving 10 bytes for every usage in this case -- if it is on an object/namespace ) etc.

[dyladan]

The most confusing exports for me are context and interface Context (uppercase C).

Definitely understandable. Do you have a naming suggestion for these?

There are a lot helper exported on top level:

The interfaces, classes, and helpers are exported on the top level, and most of the API interactions are exported in the namespace structure. Maybe we could remove some of the top level exports not needed to interact with the API and rename ones like EntryTtl which have unclear names.

Besides that one could argue that propagation and context is bound to trace.

The spec doesn't support this. Context will be used for all signals including metrics/baggage, and baggage will also need to be propagated.

I assume here that all these top level exports are seen as part of public API and follow semver.

yes

Also generic named types like StatusCode, ValueType could benefit from some limiting scope.

Given @MSNev's concerns around minification, would you prefer we namespace these types of things like api.trace.StatusCode or that we name them more specfically like SpanStatusCode/TraceStatusCode?

@MSNev you mention keeping functions at the top level, but most of the exports are actually interfaces which don't matter for tree shaking anyways. Most external interaction with the API will be through the api.[signal].[function] functions like api.trace.getTracer. I fear the ship may be sailed for a truly minification-friendly API design because most of our interactions are object oriented and suffer from the problems you mentioned (e.g. get a tracer and call methods on it).

If functions do need to exist on an object / class / namespace, keep it as SMALL as possible eg suppressInstrumentation() => suppressInstr() (saving 10 bytes for every usage in this case -- if it is on an object/namespace ) etc.

I am hesitant to abbreviate names unless it is exceedingly obvious what the abbreviations stand for. New users may not know that suppressInstr means suppress instrumentation.

Takeaway

• Is it really needed to export noop implementations?
• Remove proxy exports like ProxyTracer
• Rename interfaces that are too general like EntryTtl => BaggageEntryTtl

[Flarna]

Definitely understandable. Do you have a naming suggestion for these?

I was never the best contact to ask for good API names...
Anyhow, what about renaming context => contextApi, propagation => propagationApi,.. and keep Context.

Given @MSNev's concerns around minification, would you prefer we namespace these types of things like api.trace.StatusCode or that we name them more specfically like SpanStatusCode/TraceStatusCode?

Personally I prefer namespaces as this allows to import the relevant one where needed and then go on and work with short names. I guess that typically just one or two aspects are needed within one module and only in rare cases the whole Api.
But I'm on node so minification is not that relevant.

[MSNev]

Just a couple of minor comments

most of the exports are actually interfaces

By declaring them on the interface it means that the real implementations will have to have them and therefore application code will need to use them and they won't be minified.
And I'm all for defining the payloads (objects) as interfaces (vs's classes) as it provides greater flexibility for implementations (not withstanding the function and resulting minification issues), but there are options when required (there just not elegant)

I am hesitant to abbreviate names unless it is exceedingly obvious

Agree, was just providing an example

I fear the ship may be sailed for a truly minification-friendly API design

Also agree, I think I'm coming late to the party on this one, but as above there are non-elegant solutions like using local variables (for multiple usages) and string name based indexed lookups if size issues are really an issue
eg.
api.trace.StatusCode
becomes (note not a realistic "one-off" example)
var strTrace = "trace";
var strStatusCode = "StatusCode";
var result = api[strTrace][strStatusCode];

[obecny]

If we want to squeeze and minify as much as possible from the api I think we should ignore the namespaces in favor of having more descriptive function name. So with regards to mentioned example

trace.someApiCall();
metrics.someApiCall();

we could have instead

traceSomeApiCall();
metricsSomeApiCall();

and of course only if both trace and metrics will have the same function name.

[Flarna]

Closing as API release candidate is ready therefore no breaking changes like moving something around will be done.