add working examples with Jaeger tracer. #12

ror6ax · 2018-05-08T13:24:13Z

I've added examples that use Jaeger instead of Lightstep, introduce serialization to the wire, and are bare minimal for ease of understanding.

Before running both files you need to set JAEGER_HOST and WEBSERVER_HOST environment variables.

@bhs @yurishkuro - please let me know if the approach seems good for you. I'll be writing README after your thumbs up on general direction.

yurishkuro · 2018-05-08T15:47:21Z

example/client.py

-ls_tracer = lightstep.tracer.init_tracer(group_name="example client", access_token="{your_lightstep_token}")
-# this tracer does not trace all requests, so the @tracer.trace() decorator must be used
-tracer = FlaskTracer(ls_tracer)
+JAEGER_HOST = environ.get('JAEGER_HOST')


maybe default to local host?

yurishkuro · 2018-05-08T15:48:14Z

example/client.py


+config = Config(config={'sampler': {'type': 'const', 'param': 1},


this could use a comment explaining what's happening, i.e. "configuring and creating an instance of Jaeger Tracer with 100% sampling and debug logging of spans".

yurishkuro · 2018-05-08T15:48:30Z

example/client.py

-	for i in range(numrequests):
-		send_request()
-	return "Requests sent"
+install_all_patches()


could use a comment explaining what this does

yurishkuro · 2018-05-08T15:49:35Z

example/client.py

+    span.set_tag(tags.HTTP_URL, url)
+    span.set_tag(tags.SPAN_KIND, tags.SPAN_KIND_RPC_CLIENT)
+    headers = {}
+    tracer.inject(span, Format.HTTP_HEADERS, headers)


sorry, why is this needed? Since you're calling install_all_patches() above, the requests module is automatically instrumented and will do the exact same thing.

yurishkuro · 2018-05-08T15:50:32Z

example/client.py


+    time.sleep(2)


this may confuse people, I recommend adding a comment

# allow tracer to flush the spans - https://github.com/jaegertracing/jaeger-client-python/issues/50

yurishkuro · 2018-05-08T15:51:37Z

example/server.py

+                                'local_agent':
+                                {'reporting_host': JAEGER_HOST}},
+                        service_name="jaeger_opentracing_example")
+        ls_tracer = config.initialize_tracer()


rename to jaeger_tracer

yurishkuro · 2018-05-08T15:52:50Z

example/server.py

+        @tracer.trace()
+        def log():
+
+                span_ctx = ls_tracer.extract(Format.HTTP_HEADERS, request.headers)


similar to client, the point of using @tracer.trace() is that the handlers are automatically instrumented, but you're doing it again manually

bhs

I'm approving this for the LightStep removals only (i.e., I'm not commenting on the Jaeger pieces). Glad to see this change!

One error of omission: you should scrub the LightStep bits from the README, too.

ror6ax · 2018-05-14T12:20:28Z

@bhs , @yurishkuro - this is ready for re-review. I've added new images, removed code duplication as per Yuri's comments and re-written parts README.

This is a server-client example and I'll be adding server-server one as a separate PR.

yurishkuro · 2018-05-14T22:40:47Z

example/README.md

-app, and the trace will include a span on both the client and server sides. This
-occurs automatically since both the client and server functions are decorated
-with @tracer.trace().
+To see the traced requests, go to Jaeger web interface.


"and refresh the page"

yurishkuro · 2018-05-14T22:42:50Z

example/README.md

-![span with log](https://raw.githubusercontent.com/kcamenzind/flask_opentracing/master/example/img/log.png)
+Browse to http://localhost:5000/log and compare the trace in Jaeger.
+The last one has 2 spans instead of 3. The span of webserver's GET method is missing.
+That is because in client.py we pass already existing span over the wire, whereas the request from your browser has to tracing data in it.


That is because client.py starts a trace and passes trace context over the wire, whereas the request from your browser has to tracing context in it.

yurishkuro · 2018-05-14T22:43:21Z

example/README.md

-span. This example demonstrates how you can trace non-RPC function calls in your
-app, through accessing the current span as follows:
+In log function of the server app, we are creating current_span. This is done to
+trace the work that is being done to render the response to /log. Suppose there's


"response to /log endpoint"

yurishkuro · 2018-05-14T22:44:41Z

example/README.md

+trace the work that is being done to render the response to /log. Suppose there's
+a database connection happening. By creating a separate span for it, you'll be able
+to trace the DB request separately from rendering or the response. This gives a
+lot of flexibility to the user.


maybe it's worth mentioning that install_patches will do that automatically for some frameworks like SqlAlchemy, Redis client, etc.

yurishkuro · 2018-05-14T22:52:16Z

example/server.py

+        tracer = FlaskTracer(jaeger_tracer)
+
+        @app.route('/log')
+        # Indicate that /log endpoint should be traced


nit: this would read better on the same line as the decorator

yurishkuro · 2018-05-14T22:57:56Z

example/server.py

+                # Extract the span information for request object.
+                request_span = tracer.get_span(request)
+                # Start internal span to trace some kind of business logic for /log.
+                current_span = jaeger_tracer.start_span("python webserver internal span of log method",


the preferred syntax here would be

with jaeger_tracer.start_span("inner-span", child_of=request_span) as span: # Perform some computations to be traced. span.log_kv({'event': 'my computer knows math!', 'result': c})

ror6ax · 2018-05-15T10:48:05Z

@yurishkuro Fixed per your comments.

yurishkuro · 2018-05-15T14:47:28Z

example/README.md

+  -p 16686:16686 \
+  -p 14268:14268 \
+  -p 9411:9411 \
+  jaegertracing/all-in-one:latest


would it make sense to just use host networking? Most of these ports are not actually necessary for this example anyway.

It's the way Jaeger manual presents it. I don't want to confuse people who may not be into details of Docker. In any case, ports do not hurt anyone.
Agree?

yurishkuro · 2018-05-15T14:48:21Z

example/README.md

-### Add spans to the trace manually:
+Speaking about databases, using install_all_patches() method from
+opentracing_instrumentation package gives you a way to trace
+your MySQLdb, SQLAlchemy, Redis queries with without writing boilerplate code.


typo: "with without"

yurishkuro · 2018-05-15T14:49:54Z

example/server.py

+        def log():
+                request_span = tracer.get_span(request)
+                # Extract the span information for request object.
+                with jaeger_tracer.start_span("python webserver internal span of log method",


personally I would not use such span name even in the example, because it might make people think this is a good practice.

added working examples with Jaeger tracer

6e2ba7a

ror6ax requested review from bhs and yurishkuro May 8, 2018 13:24

yurishkuro requested changes May 8, 2018

View reviewed changes

bhs approved these changes May 8, 2018

View reviewed changes

ror6ax force-pushed the example branch from 35a5a2d to 05178cc Compare May 14, 2018 12:11

added README

618daa1

ror6ax force-pushed the example branch from 05178cc to 618daa1 Compare May 14, 2018 12:17

yurishkuro reviewed May 14, 2018

View reviewed changes

yurishkuro reviewed May 15, 2018

View reviewed changes

yurishkuro approved these changes May 15, 2018

View reviewed changes

yurishkuro reviewed May 15, 2018

View reviewed changes

adressed review comments

06adfde

ror6ax force-pushed the example branch from 9bf9740 to 06adfde Compare May 29, 2018 13:33

ror6ax merged commit 05a5c78 into master May 29, 2018

ror6ax changed the title ~~WIP: add working examples with Jaeger tracer.~~ add working examples with Jaeger tracer. May 29, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add working examples with Jaeger tracer. #12

add working examples with Jaeger tracer. #12

ror6ax commented May 8, 2018

yurishkuro May 8, 2018

yurishkuro May 8, 2018

yurishkuro May 8, 2018

yurishkuro May 8, 2018

yurishkuro May 8, 2018

yurishkuro May 8, 2018

yurishkuro May 8, 2018

bhs left a comment

ror6ax commented May 14, 2018

yurishkuro May 14, 2018

yurishkuro May 14, 2018

yurishkuro May 14, 2018

yurishkuro May 14, 2018

yurishkuro May 14, 2018

yurishkuro May 14, 2018

ror6ax commented May 15, 2018

yurishkuro May 15, 2018

ror6ax May 29, 2018

yurishkuro May 15, 2018

yurishkuro May 15, 2018


		config = Config(config={'sampler': {'type': 'const', 'param': 1},

add working examples with Jaeger tracer. #12

add working examples with Jaeger tracer. #12

Conversation

ror6ax commented May 8, 2018

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

bhs left a comment

Choose a reason for hiding this comment

ror6ax commented May 14, 2018

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

ror6ax commented May 15, 2018

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