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

Optional strict mode to help surface issues around correctness #92

Closed
hekike opened this issue Jul 10, 2019 · 2 comments
Closed

Optional strict mode to help surface issues around correctness #92

hekike opened this issue Jul 10, 2019 · 2 comments
Labels
feature-request up-for-grabs Good for taking. Extra help will be provided by maintainers

Comments

@hekike
Copy link
Member

hekike commented Jul 10, 2019

Is your feature request related to a problem? Please describe.

We have already made some tradeoffs around accepting incorrect configuration or data without blowing the SDK up. For example using getCurrentSpan with NopScopeManager or passing different format to inject(). I expect we will log warnings or errors about these edge cases, but I could imagine an extra strict (default: false) option that would throw instead of logging and could be used for testing.

In OpenCensus I found challenging sometimes to figure out why my traces are broken when I did something wrong.

Describe the solution you'd like
strict option (default: false) that would throw instead of logging.

logInconsistency(msg) {
  if (this.config.strict) {
    throw new Error(msg)
  }

  this.logger.error(msg);
}

getCurrentSpan(): types.Span {
   if (this.scopeManager instanceof NopScopeManager) {
      this.logInconsistency('NopScopeManager does not support getCurrentSpan()');
   }
   return TracerBase.defaultSpan;
}

Describe alternatives you've considered
Just logging.

@hekike hekike changed the title Optional strict mode to help validate correctness Optional strict mode to help surface issues around correctness Jul 10, 2019
@draffensperger
Copy link
Contributor

Related to this, I would like to have the option to make it so that no logging takes place in a production browser context (console.log statements don't really help your end user), and even so that all logging could be stripped out by optimizing compilers like Closure.

I think making the logging utilities live under the platform folder and respond to some global config could enable both of these use cases.

@mayurkale22 mayurkale22 added the up-for-grabs Good for taking. Extra help will be provided by maintainers label Jul 12, 2019
@rauno56
Copy link
Member

rauno56 commented Jul 15, 2022

We haven't set a clear rule about when to throw and when to log the error. Since we’re GA we don’t want to change any of the current behavior.
As a general rule, we should throw on the startup/configuration phase and log the error during runtime to avoid breaking users.

@rauno56 rauno56 closed this as completed Jul 15, 2022
dyladan added a commit to dyladan/opentelemetry-js that referenced this issue Sep 9, 2022
dyladan added a commit to dyladan/opentelemetry-js that referenced this issue Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request up-for-grabs Good for taking. Extra help will be provided by maintainers
Projects
None yet
Development

No branches or pull requests

4 participants