Propagation/B3: Enforce strictNullChecks and noUnusedLocals #421
Conversation
mayurkale22
requested review from
draffensperger and
justindsmith
as code owners
Codecov Report
@@ Coverage Diff @@
## master #421 +/- ##
==========================================
+ Coverage 94.81% 94.86% +0.05%
==========================================
Files 136 136
Lines 9000 9042 +42
Branches 666 665 -1
==========================================
+ Hits 8533 8578 +45
+ Misses 467 464 -3
Continue to review full report at Codecov.
|
1. Remove unwanted check on getter. 2. Add check for an array or falsey value 3. Make sentence readable
mayurkale22
force-pushed
the
enforce_strictNullcheck_b3
branch
from
9ef2ecb
to
5d65f62
Compare
| return null; | ||
| const opt = this.parseHeader(getter.getHeader(X_B3_SAMPLED)); | ||
| return { | ||
| traceId: this.parseHeader(getter.getHeader(X_B3_TRACE_ID)), |
There was a problem hiding this comment.
IIUC the extract method should return null if there is no incoming traceID/spanID, rather than fill an object with empty strings.
There was a problem hiding this comment.
This is valid point, I have changed the implementation to return null in case of no incoming traceID/spanID.
| } else { | ||
| setter.setHeader(X_B3_SAMPLED, `${NOT_SAMPLED_VALUE}`); | ||
| } | ||
| setter.setHeader(X_B3_TRACE_ID, spanContext.traceId || ''); |
There was a problem hiding this comment.
On a similar note as above, I'm not sure if we should be writing an empty value to a header. That being said the SpanContext interface should guarantee that traceId and spanId are not blank.
There was a problem hiding this comment.
Agreed, does not make sense to write empty values to a header. Added check for the same. PTAL
| @@ -0,0 +1,72 @@ | |||
| /** | |||
There was a problem hiding this comment.
This file is taken from opencensus-propagation-tracecontext -> https://github.com/census-instrumentation/opencensus-node/blob/master/packages/opencensus-propagation-tracecontext/src/validators.ts
|
@kjin Please take a look again once you have time. |
This is part of #348 and #340