feat: use gRPC to write spans to cloud trace

[reggiemcdonald]

Description

• Adds a cloudtrace.v2.TraceService client to TraceExporter to be used to write span data to cloud trace
• Updated tests to work with the new rpc client

Testing

• Passes existing unit tests
• Ran the following opentelemetry example found here modified to use the exporter in this PR, and it produced expected results
• Tested on a live instance

Relevant Info

• Docs for RPC service can be found here

Fixes #94

[reggiemcdonald]

@nlehrer

[codecov]

Codecov Report

Merging #97 into master will increase coverage by 0.12%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
+ Coverage   95.16%   95.29%   +0.12%     
==========================================
  Files          11       11              
  Lines         331      340       +9     
  Branches       69       68       -1     
==========================================
+ Hits          315      324       +9     
  Misses         16       16              

Impacted Files	Coverage Δ
...es/opentelemetry-cloud-trace-exporter/src/types.ts	100.00% <ø> (ø)
...es/opentelemetry-cloud-trace-exporter/src/trace.ts	96.36% <100.00%> (+0.53%)	⬆️
...pentelemetry-cloud-trace-exporter/src/transform.ts	95.91% <100.00%> (+0.17%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 501ca1a...825c09c. Read the comment docs.

[aabmass]

@reggiemcdonald I did a little more digging, protobufjs (which is already a transitive dependency so no additional overhead to add) includes the scripts pbts/pbjs which can generate either functional code (so you don't need proto-loader) or just a .d.ts stub file. Some tradeoffs listed here. As far as creating a bunch of files, I think you could actually do it without checking additional files in as a build step since google-proto-files gives you the protos like you mentioned.

Do you think it's doable?

[reggiemcdonald]

@reggiemcdonald I did a little more digging, protobufjs (which is already a transitive dependency so no additional overhead to add) includes the scripts pbts/pbjs which can generate either functional code (so you don't need proto-loader) or just a .d.ts stub file. Some tradeoffs listed here. As far as creating a bunch of files, I think you could actually do it without checking additional files in as a build step since google-proto-files gives you the protos like you mentioned.

Do you think it's doable?

Yep! I think that its possible and would be happy to make the change. Just playing around with protobufjs right now, it appears that grpc cannot load protobufjs static files, so I just need to figure out how to work with the protobufjs rpcImpl
@mayurkale22 @dancalvert-google @kintel what do you think?

[kintel]

I'm not familiar with gRPC, so I cannot really comment on this, but it sounds like your investigation is going in the right direction.

[reggiemcdonald]

I've implemented the protobufjs solution and tested it on a live instance. You can see the code here.

The reasons that I like this implementation are:

• Types are automatically generated
• The protobufjs cli is an npm module and can be easily integrated into the build step without the need for external binaries

I left it off this branch for now in case this isn't the type of change that we want to make. Let me know what you think

[aabmass]

I'm in favor but will defer to the folks who have been working in this repo longer :) This will also be useful for metrics

[dancalvert-google]

Sounds reasonable to me as well, but I'm in a similar situation to Aaron as far as having any authority to make this call. Does anyone have concerns with the approach?

…

On Tue, Jul 7, 2020 at 5:02 PM Aaron Abbott ***@***.***> wrote: I'm in favor but will defer to the folks who have been working in this repo longer :) This will also be useful for metrics — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#97 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANP2KZMXHYXNIUEMCMMPORLR2OEOBANCNFSM4N7VST3A> .

[mayurkale22]

At quick glance proto-loader approach looks good to me. That's what we did in OpenTelemetry collector exporter and in OpenCensus agent exporter.

[aabmass]

@reggiemcdonald IIUC this won't work with web, is that right?

See #114

[aabmass]

@reggiemcdonald is this ready for another review (going ahead with proto-loader approach), or were you planning more updates?

[reggiemcdonald]

@aabmass should be ready for re-review! 🙂

[aabmass]

Awesome LGTM!

@reggiemcdonald can you mark any comments that are done as resolved?

[aabmass]

Awesome, thanks Reggie!