Use binary format for grpc plugin #354
Conversation
| @@ -444,7 +428,7 @@ export class GrpcPlugin extends BasePlugin { | |||
| * https://github.com/GoogleCloudPlatform/cloud-trace-nodejs/blob/src/plugins/plugin-grpc.ts#L96) | |||
| */ | |||
| // tslint:disable-next-line:no-any | |||
| private getMetadata(original: GrpcClientFunc, args: any[], span: Span): | |||
| private getMetadata(original: GrpcClientFunc, args: any[]): | |||
There was a problem hiding this comment.
Rather than using any here, could this just be an empty object {}?
There was a problem hiding this comment.
Ping on this: @mayurkale22 is it possible to use {} here instead of any and remove the tslint disable above the line? (This was already here so if you don't want to do it now that's OK, but figure it's nice to clean up if we can while we're in here).
| * Returns a span context on a Metadata object if it exists and is | ||
| * well-formed, or null otherwise. | ||
| * @param metadata The Metadata object from which span context should be | ||
| * retrieved. |
There was a problem hiding this comment.
Nit: could you indent "retrieved" by 4 spaces to show it is a continuation of the @param JSDoc tag?
| /** | ||
| * Set span context on a Metadata object if it exists. | ||
| * @param metadata The Metadata object to which a span context should be | ||
| * added. |
There was a problem hiding this comment.
Same here on the indent
| @@ -7,7 +7,8 @@ | |||
| "pretty": true, | |||
| "module": "commonjs", | |||
| "target": "es6", | |||
| "strictNullChecks": false | |||
| "strictNullChecks": true, | |||
| "noUnusedLocals": true | |||
Codecov Report
@@ Coverage Diff @@
## master #354 +/- ##
==========================================
+ Coverage 95.28% 95.32% +0.04%
==========================================
Files 124 124
Lines 8339 8390 +51
Branches 619 624 +5
==========================================
+ Hits 7946 7998 +52
+ Misses 393 392 -1
Continue to review full report at Codecov.
|
Good call, done b2e8835 |
| @@ -444,7 +428,7 @@ export class GrpcPlugin extends BasePlugin { | |||
| * https://github.com/GoogleCloudPlatform/cloud-trace-nodejs/blob/src/plugins/plugin-grpc.ts#L96) | |||
| */ | |||
| // tslint:disable-next-line:no-any | |||
| private getMetadata(original: GrpcClientFunc, args: any[], span: Span): | |||
| private getMetadata(original: GrpcClientFunc, args: any[]): | |||
There was a problem hiding this comment.
Ping on this: @mayurkale22 is it possible to use {} here instead of any and remove the tslint disable above the line? (This was already here so if you don't want to do it now that's OK, but figure it's nice to clean up if we can while we're in here).
This is done in many places in gRPC plugin, I would prefer to address this in separate PR. WDYT? |
|
Yes, that's fine to leave the |
Based on: https://github.com/census-instrumentation/opencensus-specs/blob/master/trace/gRPC.md#propagation
This PR contains below things
strictNullCheckson grpc repo. (part of feat: enable strictNullChecks on this repo. #348)noUnusedLocalson grpc repo.index.tsfor[opencensus-propagation-binaryformat]package.