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

[loader] Too easy to configure no-op implementation by mistake #45

Closed
Oberon00 opened this issue Jul 10, 2019 · 3 comments
Closed

[loader] Too easy to configure no-op implementation by mistake #45

Oberon00 opened this issue Jul 10, 2019 · 3 comments
Labels
api Affects the API package. usability
Milestone

Comments

@Oberon00
Copy link
Member

From #29 (comment)

The current lazy-loading behavior makes it easy to accidentally create a no-op tracer that sticks around. E.g. if the configured API implementation is decided after making an HTTP request and the used HTTP library is instrumented, the no-op tracer would be installed by the loader (and it's not exchangeable).

Several possible solutions come to my mind:

  • Just provide setters that directly set the global objects. That would also resolve [loader] Decide setter/envvar interaction & behavior with multiple setter calls #44. Disadvantage: We allow changing an existing implementation and any loader logic is effectively circumvented if the app uses the setter.
  • Don't load implicitly, explicitly require a call to loader.load (this would require moving the storage of the global objects and factory callbacks back to the loader to be implementable). Before that, all getters will return no-op.
  • Provide loader.finish_configuration(). Similar to load but does not load immediately, only signals to getters that next time they are called they should load their component.
@Oberon00 Oberon00 added the api Affects the API package. label Sep 24, 2019
@c24t c24t added this to the Alpha v0.3 milestone Oct 11, 2019
@c24t c24t modified the milestones: Alpha v0.3, Alpha v0.4 Dec 5, 2019
@c24t c24t added the usability label Dec 5, 2019
@toumorokoshi toumorokoshi modified the milestones: Alpha v0.4, Beta Feb 27, 2020
@codeboten
Copy link
Contributor

Should this be closed with the changes in #123?

@andrewhsu
Copy link
Member

Looks like related PR #466 did changes for configuration.

@c24t
Copy link
Member

c24t commented Mar 17, 2020

Yup, this is obsolete as of #466.

@c24t c24t closed this as completed Mar 17, 2020
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this issue Nov 1, 2020
* Add DefaultSpan: no-op implementation of Span

* Implement setAttributes method based on pull/44

* Remove protected methods

* Rename DefaultSpan -> BaseSpan, return non-null SpanContext, Add TODOs

* Rename BaseSpan -> NoopSpan

* NoopSpan: return a real SpanContext for propagation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Affects the API package. usability
Projects
None yet
Development

No branches or pull requests

5 participants