Skip to content
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: add gRPC Census Example #46

Merged
merged 5 commits into from
Jun 9, 2020
Merged

feat: add gRPC Census Example #46

merged 5 commits into from
Jun 9, 2020

Conversation

nkelly75
Copy link
Contributor

@nkelly75 nkelly75 commented Jun 4, 2020

Signed-off-by: Kelly, Niall [email protected]

Which problem is this PR solving?

Provide an example that verifies the new propagator-grpc-census-binary propagator.

During the review of PR #39 @mayurkale22 asked whether there was an example to verify the new propagator. This PR is to add that example.

Short description of the changes

The example started out based on the existing grpc_dynamic_codegen example - it makes similar gRPC calls. Depending on environment variables, the client and server can be instrumented with either OpenTelemetry (with varying propagators) or OpenCensus. This allows us to test various combinations and understand when the binary propagator is useful.

The README contains extra information about the combinations as well as links to sample output (from console exporters).

@codecov
Copy link

codecov bot commented Jun 4, 2020

Codecov Report

Merging #46 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #46   +/-   ##
=======================================
  Coverage   94.47%   94.47%           
=======================================
  Files          62       62           
  Lines        3494     3494           
  Branches      376      376           
=======================================
  Hits         3301     3301           
  Misses        193      193           

Kelly, Niall added 2 commits June 4, 2020 21:17
@nkelly75 nkelly75 marked this pull request as ready for review June 4, 2020 20:34
Copy link
Member

@mayurkale22 mayurkale22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, added one minor comment. Nice Work!

to propagate context through the `grpc-trace-bin` binary header.

The same source code is used to run various versions of the client and server. Environment
variables (set up through `scripts` in [package.json](./package.json) determine the various
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
variables (set up through `scripts` in [package.json](./package.json) determine the various
variables (set up through `scripts` in [package.json](./package.json)) determine the various

Kelly, Niall added 2 commits June 5, 2020 11:08
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 8, 2020

CLA Check
The committers are authorized under a signed CLA.

@dyladan dyladan merged commit e615f30 into open-telemetry:master Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants