docs(grpc): add grpc example #326

markwolff · 2019-09-24T17:11:43Z

Which problem is this PR solving?

Closes examples: provide an example for gRPC plugin #250
Requires fix(grpc): fix client/server span propagation #325

Short description of the changes

static_codegen example taken from grpc docs instrumented with OpenTelemetry.

bg451 · 2019-09-24T17:17:06Z

examples/grpc/README.md

+#### Jaeger UI
+
+`jaeger:server` script should output the `traceid` in the terminal (e.g `traceid: 4815c3d576d930189725f1f1d1bdfcc6`).
+Go to zipkin with your browser [http://localhost:50051/trace/(your-trace-id)]() (e.g http://localhost:50051/trace/4815c3d576d930189725f1f1d1bdfcc6)


s/zipkin/jaeger/

mayurkale22

Nice!

mayurkale22 · 2019-09-24T17:38:32Z

examples/grpc/client.js

+const tracer = opentelemetry.getTracer();
+
+/** A function which makes requests and handles response. */
+function main(port) {


Do we need to pass port here?

No - moved to just use local var instead

mayurkale22 · 2019-09-24T17:44:35Z

examples/grpc/client.js

@@ -0,0 +1,48 @@
+'use strict';


Probably, we can move this example in a grpc/static_codegen folder and add another example, under grpc/dynamic_codegen which can be the dynamic code generation (generated at runtime using Protobuf.js) variant of gRPC example. WDYT?

examples/grpc/setup.js

OlivierAlbertini

Nice!

OlivierAlbertini · 2019-09-24T19:56:06Z

examples/grpc/README.md

+
+Setup [Zipkin Tracing](https://zipkin.io/pages/quickstart.html)
+or
+Setup [Jaeger Tracing](https://www.jaegertracing.io/docs/1.12/getting-started/#all-in-one)


Suggested change

Setup [Jaeger Tracing](https://www.jaegertracing.io/docs/1.12/getting-started/#all-in-one)

Setup [Jaeger Tracing](https://www.jaegertracing.io/docs/latest/getting-started/#all-in-one)

examples/grpc/setup.js

OlivierAlbertini · 2019-09-24T19:59:44Z

examples/grpc/setup.js

+        enabled: true,
+        // if it can't find the module, put the absolute path since the packages are not published yet
+        path: '@opentelemetry/plugin-grpc',
+        ignoreOutgoingUrls: [/spans/]


Do you use it ?

Nope, removed

OlivierAlbertini · 2019-09-24T20:26:21Z

examples/grpc/README.md

+#### Jaeger UI
+
+`jaeger:server` script should output the `traceid` in the terminal (e.g `traceid: 4815c3d576d930189725f1f1d1bdfcc6`).
+Go to zipkin with your browser [http://localhost:50051/trace/(your-trace-id)]() (e.g http://localhost:50051/trace/4815c3d576d930189725f1f1d1bdfcc6)


Suggested change

Go to zipkin with your browser [http://localhost:50051/trace/(your-trace-id)]() (e.g http://localhost:50051/trace/4815c3d576d930189725f1f1d1bdfcc6)

Go to Jaeger with your browser [http://localhost:50051/trace/(your-trace-id)]() (e.g http://localhost:50051/trace/4815c3d576d930189725f1f1d1bdfcc6)

OlivierAlbertini · 2019-09-24T20:26:40Z

examples/grpc/README.md

+
+#### Zipkin UI
+`zipkin:server` script should output the `traceid` in the terminal (e.g `traceid: 4815c3d576d930189725f1f1d1bdfcc6`).
+Go to zipkin with your browser [http://localhost:9411/zipkin/traces/(your-trace-id)]() (e.g http://localhost:9411/zipkin/traces/4815c3d576d930189725f1f1d1bdfcc6)


Suggested change

Go to zipkin with your browser [http://localhost:9411/zipkin/traces/(your-trace-id)]() (e.g http://localhost:9411/zipkin/traces/4815c3d576d930189725f1f1d1bdfcc6)

Go to Zipkin with your browser [http://localhost:9411/zipkin/traces/(your-trace-id)]() (e.g http://localhost:9411/zipkin/traces/4815c3d576d930189725f1f1d1bdfcc6)

OlivierAlbertini · 2019-09-24T20:27:51Z

examples/grpc/client.js

+
+/**
+ * The trace instance needs to be initialized first, if you want to enable
+ * automatic tracing for built-in plugins (HTTP in this case).


Suggested change

* automatic tracing for built-in plugins (HTTP in this case).

* automatic tracing for built-in plugins.

OlivierAlbertini · 2019-09-24T20:28:47Z

examples/grpc/server.js

+
+/**
+ * The trace instance needs to be initialized first, if you want to enable
+ * automatic tracing for built-in plugins (HTTP in this case).


Suggested change

* automatic tracing for built-in plugins (HTTP in this case).

* automatic tracing for built-in plugins.

OlivierAlbertini · 2019-09-24T20:33:32Z

examples/grpc/README.md

@@ -35,7 +35,7 @@ Setup [Jaeger Tracing](https://www.jaegertracing.io/docs/1.12/getting-started/#a
 `zipkin:server` script should output the `traceid` in the terminal (e.g `traceid: 4815c3d576d930189725f1f1d1bdfcc6`).
 Go to zipkin with your browser [http://localhost:9411/zipkin/traces/(your-trace-id)]() (e.g http://localhost:9411/zipkin/traces/4815c3d576d930189725f1f1d1bdfcc6)

-<!-- <p align="center"><img src="./images/zipkin-ui.png?raw=true"/></p> -->
+<p align="center"><img src="https://user-images.githubusercontent.com/5515583/65533964-7a11a600-deb3-11e9-8af0-a91243db6fdc.png"/></p>


Should we store it in the repo ?

Yes, stored in ./images as in http example

mayurkale22 · 2019-09-25T16:49:13Z

@markwolff Please fix the comments, otherwise good to go.

mayurkale22 · 2019-09-25T21:27:50Z

@markwolff If you want, use below image for Jaeger UI. I ran this example using both the exporters, everything look good to me (dep. on #325).

* docs(grpc): add grpc example * docs(readme): add zipkin ui image * fix: typos, add local images, refactor * fix: typo

docs(grpc): add grpc example

af2a787

markwolff requested review from bg451, danielkhan, iredelmeier, mayurkale22, OlivierAlbertini, rochdev and vmarchaud as code owners September 24, 2019 17:11

docs(readme): add zipkin ui image

7e3f0d4

markwolff mentioned this pull request Sep 24, 2019

Plugin/gRPC: patching does not work when used with makeGenericClientConstructor #327

Closed

bg451 reviewed Sep 24, 2019

View reviewed changes

mayurkale22 reviewed Sep 24, 2019

View reviewed changes

OlivierAlbertini reviewed Sep 24, 2019

View reviewed changes

draffensperger approved these changes Sep 25, 2019

View reviewed changes

mayurkale22 mentioned this pull request Sep 25, 2019

Add usage instructions for gRPC plugin package #339

Closed

mayurkale22 approved these changes Sep 25, 2019

View reviewed changes

markwolff and others added 4 commits September 25, 2019 14:48

fix: typos, add local images, refactor

43b3c62

fix: typo

bb87739

Merge branch 'master' into grpc-example

38a8365

Merge branch 'master' into grpc-example

6eeb4c9

mayurkale22 merged commit 0aa4fb5 into open-telemetry:master Sep 26, 2019

OlivierAlbertini pushed a commit to VilledeMontreal/opentelemetry-js that referenced this pull request Sep 26, 2019

docs(grpc): add grpc example (open-telemetry#326)

919731f

* docs(grpc): add grpc example * docs(readme): add zipkin ui image * fix: typos, add local images, refactor * fix: typo

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs(grpc): add grpc example #326

docs(grpc): add grpc example #326

markwolff commented Sep 24, 2019

bg451 Sep 24, 2019

mayurkale22 left a comment

mayurkale22 Sep 24, 2019

markwolff Sep 25, 2019

mayurkale22 Sep 24, 2019

OlivierAlbertini left a comment

OlivierAlbertini Sep 24, 2019

OlivierAlbertini Sep 24, 2019

markwolff Sep 25, 2019

OlivierAlbertini Sep 24, 2019

OlivierAlbertini Sep 24, 2019

OlivierAlbertini Sep 24, 2019

OlivierAlbertini Sep 24, 2019

OlivierAlbertini Sep 24, 2019

markwolff Sep 25, 2019

mayurkale22 commented Sep 25, 2019

mayurkale22 commented Sep 25, 2019

	Setup [Jaeger Tracing](https://www.jaegertracing.io/docs/1.12/getting-started/#all-in-one)
	Setup [Jaeger Tracing](https://www.jaegertracing.io/docs/latest/getting-started/#all-in-one)

	Go to zipkin with your browser [http://localhost:50051/trace/(your-trace-id)]() (e.g http://localhost:50051/trace/4815c3d576d930189725f1f1d1bdfcc6)
	Go to Jaeger with your browser [http://localhost:50051/trace/(your-trace-id)]() (e.g http://localhost:50051/trace/4815c3d576d930189725f1f1d1bdfcc6)

	* automatic tracing for built-in plugins (HTTP in this case).
	* automatic tracing for built-in plugins.

docs(grpc): add grpc example #326

docs(grpc): add grpc example #326

Conversation

markwolff commented Sep 24, 2019

Which problem is this PR solving?

Short description of the changes

Choose a reason for hiding this comment

mayurkale22 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OlivierAlbertini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mayurkale22 commented Sep 25, 2019

mayurkale22 commented Sep 25, 2019