Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Updates to 0005 Global Initialization #45

Closed
wants to merge 8 commits into from
Closed

Updates to 0005 Global Initialization #45

Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7ad5e5e
Create WIP PR
jmacd Sep 11, 2019
3476dc7
Update to 0005 based on https://github.com/open-telemetry/opentelemet…
jmacd Oct 14, 2019
90e5c59
Typo
jmacd Oct 14, 2019
4952a5b
Remove sentence
jmacd Oct 15, 2019
413081b
Add motivation
jmacd Oct 16, 2019
dc5a032
Upstream
jmacd Dec 20, 2019
e22ba90
Update and revise
jmacd Dec 20, 2019
a1e3a33
Rename 74
jmacd Dec 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions text/0005-global-init.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Global SDK initialization

*Status: proposed*
*Status: accepted*

Specify the behavior of OpenTelemetry APIs and implementations at startup.

Expand Down Expand Up @@ -51,10 +51,12 @@ party libraries to use the global SDK before it is installed, which is
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
may be installed no more than once. After the first SDK installed,
subsequent calls to the explicit initializer shall log console
warnings.
`Meter` factories (e.g., `opentelemetry.Init(TracerFactory,
MeterFactory)`), factories because they construct explicitly named
Copy link
Member

Choose a reason for hiding this comment

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

Seems like there would be benefit to split this into two calls (being able to initialize the tracer and meter independently). Any particular reason to keep these tied together in the same api call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't feel very strongly about this. I was imagining that it's important for an application to know that their metrics and traces began coming through at the same moment in the process lifetime.

`Tracer` and `Meter` instances. The SDK must not be installed more
than once. After the first SDK installed, subsequent calls to the
Copy link
Member

Choose a reason for hiding this comment

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

While I agree that resetting the globals should be discouraged, completely restricting it feels unnecessary to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do you feel about the alleged risk? If anyone can install an SDK whenever they want, how do you (a main() function) prevent others from installing SDKs when you know what you want?

Copy link
Member

Choose a reason for hiding this comment

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

my worry is how multi-host processes will deal with the global tracer? If one process hosts two apps, each of them maintainig it's dependency injection and creates own tracerfactory. For the libraries that needs to use "global" tracer, which do not use DI, second app can register itself somehow in the global tracer later.

It may be C# specific, but I can see how it also may be a case in Java.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This OTEP is about how the globals are initialized, not about whether or not to support globals. I believe that debate has been had 100 times and globals are going to stay. That said, I'm trying to restrict them as much as possible by allowing them to be initialized once. Applications that wish to use multiple SDKs should avoid the globals and use DI. If there are libraries that use the globals, then there will be a problem in this scenario--either don't use globals or do use globals, they don't mix well.

Copy link
Member

Choose a reason for hiding this comment

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

no, I'm not entering that debate =). In C# and .NET Core this problem is in many places. The best practice is to not use globals and use dependency injection (DI) instead. However there are still plenty of libraries that uses globals. So when you are registering yourself in DI, you also need to register in global.

The problem occurs when a single process is shared by two apps. Each of them use DI for all libraries that adopter DI. For libraries relying on static globals - ideally - should be a way to decide at call site which tracer to use. And each of those two apps can register themselves in that global tracer for awareness.

So in this situation there is a case of later update of a global tracer.

Copy link
Member

Choose a reason for hiding this comment

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

Question is not about the debate, but about clearly documenting the need and reasons for supporting globals in OTel.

Also, iirc, when we had this debate in the spring, we agreed that any documentation / facilities for globals should come with a disclaimer DO NOT USE THIS.

explicit initializer shall log console warnings but not replace the
active SDK.

In common language, uses of the global SDK instance (i.e., the Tracer
and Meter) must "begin working" once the SDK is installed, with the
Expand All @@ -71,9 +73,8 @@ they continue as No-op spans.

There may be loss of metrics at startup.

Metric SubMeasure objects (i.e., metrics w/ predefined labels)
initialized before the SDK is installed will redirect to the global
SDK after it is installed.
Metric instruments and handles initialized before the SDK is installed
will redirect to the global SDK after it is installed.

### Concrete types

Expand All @@ -85,9 +86,13 @@ not depend on the SDK being installed.

### Testing support

Testing should be performed without depending on the global SDK.

### Synchronization
Testing should be performed without depending on the global SDK, if
possible. If it is necessary to install a global `Tracer` or `Meter`
factory for code that explicitly relies on the global factories for
testing, test libraries should provide a compatible SDK that can be
registered once with support for dynamically adding and removing
exporters. The use of named `Tracer` and `Meter` instances will help
jmacd marked this conversation as resolved.
Show resolved Hide resolved
avoid test interference in this case.

Since the global Tracer and Meter objects are required to begin
working once the SDK is installed, there is some implied
Expand Down