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.

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

Add setup method to configure the OpenTelemetry API #1149

Closed
thisthat opened this issue Apr 27, 2020 · 10 comments · Fixed by #1857
Closed

Add setup method to configure the OpenTelemetry API #1149

thisthat opened this issue Apr 27, 2020 · 10 comments · Fixed by #1857
Labels
Feature Request Suggest an idea for this project release:after-ga

Comments

@thisthat
Copy link
Member

According to the specifications, the OpenTelemetry SDK must be programmatically configurable. The current initialization looks for implementations of the OpenTelemetry API via Service Provider and if no instance is found, it defaults to a no-op implementation. In the SIG call, we agreed that the default behavior should be no-op until a configuration is provided.

#1058 provides a first implementation that allows to set up individually tracer provider, metric provider, and context manager. We should iterate over this proposal and making it friendly to the auto-instrumentation.

@carlosalberto
Copy link
Contributor

I think we should still do the SPI thing we do at this moment, while also supporting explicit, manual configuration as done in #1058

@jkwatson
Copy link
Contributor

I think @bogdandrutu was going to prototype an SDK that could also be a no-op until it had been configured, which would, I think, solve many of the startup-timing issues that make all of this extra tricky.

@bogdandrutu
Copy link
Member

#1311 (comment)

I have some questions about this:

  1. Should we always require setup to be called? Probably yes - part of setup we should ask user to add the span processor.
  2. What is the default behavior before setup? Should this be configurable? If yes, how?

@anuraaga
Copy link
Contributor

anuraaga commented Jun 6, 2020

My opinion is

  1. require setup
  2. everything no-op until it's called. Some background thing may happen before a user gets to setup the SDK (e.g., initialize db connection pool) and it would add noise if these got instrumented but without the proper setup.

@jkwatson
Copy link
Contributor

Can we push this off until after GA? I think this is a purely additive change to the API/SDK, so it seems like this could be pushed off, especially as the instrumentation agent seems to be working fine for now.

@carlosalberto
Copy link
Contributor

Technically we should implement this as part of the Specification (as mentioned by @thisthat in the issue description) - yet I feel this is indeed something that could be left as "Required for GA" but low priority (in case we cannot address all the items by deadline time).

@thisthat @anuraaga Sounds great? If not, please elaborate ;)

@jkwatson
Copy link
Contributor

Heh. I wrote that specification, so I do think we should be programmatically configurable. But, I also think we could probably get away with only SPI for GA, if we need to.

@iNikem
Copy link
Contributor

iNikem commented Sep 15, 2020

If SPI covers all configuration options that we want to provide, then it is certainly enough for GA. Other options, like setup method or such, can be added after GA.

@jkwatson jkwatson added Feature Request Suggest an idea for this project release:after-ga and removed priority:p2 Medium priority issues and bugs. release:required-for-ga Required for 1.0 GA release labels Sep 15, 2020
@jkwatson
Copy link
Contributor

Moving to after-ga. Please yell if you think this is actually needed for GA.

@jkwatson
Copy link
Contributor

jkwatson commented Oct 9, 2020

I think this would be satisfied by #1737 , yes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Suggest an idea for this project release:after-ga
Projects
None yet
6 participants