Code Enhancement/Refactor #403
Conversation
Codecov Report
@@ Coverage Diff @@
## master #403 +/- ##
==========================================
+ Coverage 94.77% 94.79% +0.02%
==========================================
Files 136 136
Lines 8942 8939 -3
Branches 650 656 +6
==========================================
- Hits 8475 8474 -1
+ Misses 467 465 -2
Continue to review full report at Codecov.
|
| this.notifyStartSpan(root); | ||
| if (!this.active) return; | ||
| if (!root) { | ||
| return this.logger.debug('cannot start trace - no active trace found'); |
There was a problem hiding this comment.
What would you think about separating the return from the logger statement? Though it compiles like this, it looks unexpected to me as if it were returning a non-undefined value from the logger statement.
| /** An object to log information to */ | ||
| private logger: core.Logger = null; | ||
| /** An object to log information to. Logs to the JS console by default. */ | ||
| private logger: core.Logger = console; |
There was a problem hiding this comment.
Optional: I believe there is also a logger instance in https://github.com/census-instrumentation/opencensus-node/blob/master/packages/opencensus-core/src/common/console-logger.ts
That does formatting a bit differently from the default console instance I believe. I used console in OC Web because I didn't want to introduce extra bytes of code for the logger, but that's not really much of an issue for Node.
Change console instance to logger instance (ConsoleLogger). Remove return from the logger statement.
This PR contains following things
parentSpanIdfromstartChildSpanAPI.noUnusedLocalson opencensus-nodejs package.