-
Notifications
You must be signed in to change notification settings - Fork 826
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
feat: make scopemanager optional param #257
feat: make scopemanager optional param #257
Conversation
/cc @draffensperger Please review, I can't add as a code reviewer. |
Codecov Report
@@ Coverage Diff @@
## master #257 +/- ##
==========================================
- Coverage 98.94% 98.94% -0.01%
==========================================
Files 66 66
Lines 2652 2648 -4
Branches 173 170 -3
==========================================
- Hits 2624 2620 -4
Misses 28 28
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
f47b3f0
to
c75a76c
Compare
@@ -35,6 +35,7 @@ import { mergeConfig } from './utility'; | |||
import { SpanProcessor } from './SpanProcessor'; | |||
import { NoopSpanProcessor } from './NoopSpanProcessor'; | |||
import { MultiSpanProcessor } from './MultiSpanProcessor'; | |||
import { defaultConfig } from './config'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since defaultConfig
is a shared immutable constant, should it be DEFAULT_CONFIG in all caps?
c8b6411
to
52be98e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Ping, for more reviews. |
Which problem is this PR solving?
Originates from API/SDK usage based on current implementation #194
This is as per today's (09/11) SIG discussion. Let me know if you still disagree with this.
BasicTracer
is configured withNoopScopeManager
as a default value.NodeTracer
is configured withAsyncHooksScopeManager
as a default value.