Global initializer requirements proposal #11
Conversation
jmacd
requested review from
AloisReitbauer,
bogdandrutu,
c24t,
carlosalberto,
iredelmeier,
reyang,
SergeyKanzhelev,
songy23,
tedsuo and
yurishkuro
as code owners
|
@jmacd what's the current status on this PR? What are we blocked on, if anything? Thanks in advance... |
0010-global-init.md
Outdated
| synchronization overhead at startup, overhead we expect to fall after | ||
| the SDK is installed. We recommend explicitly installing a No-op SDK | ||
| to fully disable instrumentation, as this approach will have a lower | ||
| overhead than leaving the OpenTracing library uninitialized. |
There was a problem hiding this comment.
Btw, in OpenTracing/OpenTelemetry Java a default no-op SDK is installed, so no explicit installation of a no-op SDK is needed at all for this case, just FYI ;)
There was a problem hiding this comment.
I was trying to emphasize a minor point, which is that there may be a difference in performance between the default/implicit noop implementation and an explicit noop implementation. The explicit noop implementation can do nothing. The implicit noop implementation, if it is to "start working after the SDK is first registered", must do more than the explicit noop implementation. This is a super minor point, but if I wanted the lowest overhead possible, I should explicitly install a noop.
In Java, the presence of a Service Provider Interface sidesteps this concern. In Java, the elected SDK can be installed before its first use.
0010-global-init.md
Outdated
|
|
||
| What other options should be passed to the explicit global initializer? | ||
|
|
||
| Is there a public test for "is the SDK installed; is it a no-op"? |
There was a problem hiding this comment.
In OpenTracing we have GlobalTracer.isRegistered() (to tell whether any SDK has been installed) but that works mostly as a hint ;)
There was a problem hiding this comment.
👍
I'd lean toward having the Tracer and Meter interfaces support a "Am I not a Noop?" method.
0010-global-init.md
Outdated
| addressed in a requirement stated below. | ||
|
|
||
| The explicit initializer method should take independent `Tracer` and | ||
| `Meter` objects (e.g., `opentelemetry.Init(Tracer, Meter)`). The SDK |
There was a problem hiding this comment.
I suggest that we have separate initialization for Tracer and Meter, even in the examples here.
There was a problem hiding this comment.
I believe it's possible with this API to independently construct and configure the Tracer and Meter that will be used, all that this requires is that they be provided in a single initializer call. I believe that we should encourage a single initializer call so that when a combined implementation (i.e., an SDK that presents both APIs) is registered, it can be sure that its registration is all-or-none.
In any case, it should be possible to provide Tracer and Meter implementations that are completely independent, or to pass a Noop implementation of either in the initializer.
0010-global-init.md
Outdated
| initialized before the SDK is installed will redirect to the global | ||
| SDK after it is installed. | ||
|
|
||
| ### Concrete types |
There was a problem hiding this comment.
I wonder what shall we do with propagators (e.g. should propagators be part of tracing API). We can explore this in a separate PR though.
There was a problem hiding this comment.
I see a separate issue here, yes.
|
@jmacd please respond to the comments. |
|
I've responded to the comments. I don't see this proposal as preventing the use of independent Tracer and Meter implementations. |
text/0005-global-init.md
Outdated
| synchronization overhead at startup, overhead we expect to fall after | ||
| the SDK is installed. We recommend explicitly installing a No-op SDK | ||
| to fully disable instrumentation, as this approach will have a lower | ||
| overhead than leaving the OpenTracing library uninitialized. |
There was a problem hiding this comment.
s/OpenTracing/OpenTelemetry
|
@jmacd ping, can you please fix the last comment. |
|
Done. |
|
After #16, I believe we'll change the global initialization logic to install factories rather than concrete Tracer and Meter interfaces. |
* Document global initializion * Global initializer requirements * Minor revision * Set status * Rename 0010-global-init.md to text/0005-global-init.md * OTr->OTel
* Document global initializion * Global initializer requirements * Minor revision * Set status * Rename 0010-global-init.md to text/0005-global-init.md * OTr->OTel
* Document global initializion * Global initializer requirements * Minor revision * Set status * Rename 0010-global-init.md to text/0005-global-init.md * OTr->OTel
* Document global initializion * Global initializer requirements * Minor revision * Set status * Rename 0010-global-init.md to text/0005-global-init.md * OTr->OTel
This proposal is supported by demonstration code here:
jmacd/opentelemetry-go#3
This is heavily influenced by: open-telemetry/opentelemetry-ruby#19
This is attempting to resolve: open-telemetry/opentelemetry-go#52