Monitoring platform events with zipkin-go and opentracing #50

hkshaw1990 · 2017-02-15T08:43:16Z

Perf events can provide additional insights into the performance of a
span or a sequence of spans in a trace. This patch includes the use of
a library called
"perfevents" (https://github.com/opentracing-contrib/perfevents) which
can be simply imported and used. This library is an abstraction of the
platform side of counters and operations. Right now, only the generic
perf events are supported. With this patch, one should be able to find
out the cpu-cycles, instructions, cache-references, cache-misses,
branch-instructions, branch-misses and bus cycles for a span.

The required event(s) should be supplied in the form of a string(comma
separated list if there are multiple events to be monitored) to the
StartSpan call and then, the opentracing-go library and the
zipkin-go-opentracing library should take over from there. As soon as
the span is closed, the perf event(s) count is logged with span.Log().

An example to use this with a span :
// start a span with perfevents
sp = tracer.StartSpan("read_file",
opentracing.PerfString("cache-misses,cpu-cycles"))

Upon closing the span, the events stop monitoring and are destroyed afer
the data is sent for logging.

Perf events can provide additional insights into the performance of a span or a sequence of spans in a trace. This patch includes the use of a library called "perfevents" (https://github.com/opentracing-contrib/perfevents) which can be simply imported and used. This library is an abstraction of the platform side of counters and operations. Right now, only the generic perf events are supported. With this patch, one should be able to find out the cpu-cycles, instructions, cache-references, cache-misses, branch-instructions, branch-misses and bus cycles for a span. The required event(s) should be supplied in the form of a string(comma separated list if there are multiple events to be monitored) to the StartSpan call and then, the opentracing-go library and the zipkin-go-opentracing library should take over from there. As soon as the span is closed, the perf event(s) count is logged with span.Log(). An example to use this with a span : // start a span with perfevents sp = tracer.StartSpan("read_file", opentracing.PerfString("cache-misses,cpu-cycles")) Upon closing the span, the events stop monitoring and are destroyed afer the data are sent for logging. Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>

hkshaw1990 · 2017-02-15T08:54:36Z

opentracing/spec Issue in refernce : opentracing/specification#36
PR to opentracing-go : opentracing/opentracing-go#135

Also, the travis error mentioned above is because the perfevents package is not yet upstream :
PR to perfevents : opentracing-contrib/perfevents#1

Use PerfEvent tag to check if perfevents needs to be initialized, if yes, initialize them. Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>

hkshaw1990 · 2017-02-21T07:02:07Z

Added one more commit to this PR to use tags instead of span options to start the perfevents.

basvanbeek · 2017-02-24T10:13:11Z

I like the idea of being able to have these metrics inside spans, but not in the proposed current implementation where it breaks cross platform compatibility. I think most will depend on the outcome of: opentracing/opentracing-go#136

hkshaw1990 · 2017-02-24T16:52:03Z

Hi @basvanbeek
I am currently looking into the approach of an observer interface (as in the issue you referenced and this PR from Yuri : jaegertracing/jaeger-client-go#94)
Thanks!

hkshaw1990 · 2017-03-15T13:48:03Z

Hi,
Regarding the discussion which is going on here opentracing/opentracing-go#135 (comment) for an Observer interface, would love to hear some thoughts on having an Observer interface and an implementation in zipkin.

basvanbeek · 2017-03-20T10:38:36Z

@hkshaw1990 unfortunately I have too much on my plate at the moment to look at an Observer interface more closely but I definitely don't dismiss the idea of one existing.

In the mean time you could investigate a POC and adjust the PR to include this logic through an Observer interface and make it easier for all of us to look at the implications for the project as a whole. Since I consider the platform events logic as something for consumers to opt-in on, I would like the implementation of the Observer interface be neither obtrusive nor performance impeding.

Metrics are useful to gain insights in a distributed application. But there can be a lot of metrics in different domains. Adding such metrics from one domain (metrics exporter pkg) into zipkin is not good for cross platform compatibility. This commit adds a new observer and span observer API which defines a standard interface to add such metrics. To expose the metrics, one would have to implement the observer interface in the metrics exporter. zipkin will need to implement this interface and install callbacks for methods such as StartSpan, SetOperationName, SetTag and Finish. The registered callbacks would then be called on the span events if an observer is created. This is based on the work done by Yuri here : jaegertracing/jaeger-client-go#94 Signed-off-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>

hkshaw1990 · 2017-03-21T08:52:55Z

@basvanbeek I have added a commit as a POC to add an API for the Observer interface and its implementation.

basvanbeek

A few items to take care of and then LGTM

basvanbeek · 2017-06-18T09:35:53Z

observer.go

+// observer := myobserver.NewObserver()
+// tracer := zipkin.NewTracer(..., zipkin.WithObserver(observer))
+//
+type Observer interface {


Please remove the Observer interface here and use the one you created in https://github.com/opentracing-contrib/go-observer. You can pull PR opentracing-contrib/go-observer#2 to fix the missing dependencies to opentracing-go

WIll do and thanks for the PR!

basvanbeek · 2017-06-18T09:36:31Z

observer.go

+// other Span events.
+// zipkin should define these functions for each of the span operations
+// which should call the registered (observer) callbacks.
+type SpanObserver interface {


Same as Observer. This one can be removed

basvanbeek · 2017-06-18T09:37:09Z

observer.go

+
+// noopSpanObserver is used when there are no observers registered on the
+// Tracer or none of them returns span observers
+var noopSpanObserver = spanObserver{}


I added a NoopSpanObserver implementation to opentracing-contrib/go-observer.

If you use that one we don't try to range over SpanObservers when calling one of the SpanObserver methods

EDIT: Yuri told me it is fine to return nil, isn't that a much better idea?

Yeah, makes sense to return a nil.

basvanbeek · 2017-06-18T09:42:17Z

span.go

@@ -29,6 +29,7 @@ type Span interface {
 type spanImpl struct {
 	tracer     *tracerImpl
 	event      func(SpanEvent)
+	Observer   SpanObserver


do not export the SpanObserver variable as it is not needed and direct external access to it is undesirable.

Ok, will change this.

hkshaw1990 · 2017-06-22T15:49:39Z

@basvanbeek added the changes which uses the go-observer interfaces and addresses the review comments.

basvanbeek · 2017-06-22T16:24:08Z

Thanks @hkshaw1990 for your patience with this.

hkshaw1990 mentioned this pull request Feb 15, 2017

Add support for perfevents and an Observer API opentracing/opentracing-go#135

Closed

This was referenced Feb 15, 2017

Supporting Linux Perf events with Opentracing opentracing/specification#36

Open

Integration with Opentracing opentracing-contrib/perfevents#2

Closed

Adjust to the new spec for perfevents

ddf037c

Use PerfEvent tag to check if perfevents needs to be initialized, if yes, initialize them. Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>

Add a comment for the exported func WithObserver

e3d109a

basvanbeek requested changes Jun 18, 2017

View reviewed changes

Use the go-observer interface

7552a47

basvanbeek approved these changes Jun 22, 2017

View reviewed changes

basvanbeek merged commit 1cafbdf into openzipkin-contrib:master Jun 22, 2017

hkshaw1990 mentioned this pull request Jul 12, 2017

Integration with opentracing-go through observer opentracing-contrib/perfevents#3

Closed

basvanbeek mentioned this pull request May 19, 2019

Use zipkin-go #123

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Monitoring platform events with zipkin-go and opentracing #50

Monitoring platform events with zipkin-go and opentracing #50

hkshaw1990 commented Feb 15, 2017

hkshaw1990 commented Feb 15, 2017

hkshaw1990 commented Feb 21, 2017

basvanbeek commented Feb 24, 2017

hkshaw1990 commented Feb 24, 2017

hkshaw1990 commented Mar 15, 2017 •

edited

Loading

basvanbeek commented Mar 20, 2017

hkshaw1990 commented Mar 21, 2017

basvanbeek left a comment •

edited

Loading

basvanbeek Jun 18, 2017

hkshaw1990 Jun 20, 2017

basvanbeek Jun 18, 2017

hkshaw1990 Jun 20, 2017

basvanbeek Jun 18, 2017 •

edited

Loading

hkshaw1990 Jun 20, 2017 •

edited

Loading

basvanbeek Jun 18, 2017

hkshaw1990 Jun 20, 2017

hkshaw1990 commented Jun 22, 2017

basvanbeek commented Jun 22, 2017

Monitoring platform events with zipkin-go and opentracing #50

Monitoring platform events with zipkin-go and opentracing #50

Conversation

hkshaw1990 commented Feb 15, 2017

hkshaw1990 commented Feb 15, 2017

hkshaw1990 commented Feb 21, 2017

basvanbeek commented Feb 24, 2017

hkshaw1990 commented Feb 24, 2017

hkshaw1990 commented Mar 15, 2017 • edited Loading

basvanbeek commented Mar 20, 2017

hkshaw1990 commented Mar 21, 2017

basvanbeek left a comment • edited Loading

Choose a reason for hiding this comment

basvanbeek Jun 18, 2017

Choose a reason for hiding this comment

hkshaw1990 Jun 20, 2017

Choose a reason for hiding this comment

basvanbeek Jun 18, 2017

Choose a reason for hiding this comment

hkshaw1990 Jun 20, 2017

Choose a reason for hiding this comment

basvanbeek Jun 18, 2017 • edited Loading

Choose a reason for hiding this comment

hkshaw1990 Jun 20, 2017 • edited Loading

Choose a reason for hiding this comment

basvanbeek Jun 18, 2017

Choose a reason for hiding this comment

hkshaw1990 Jun 20, 2017

Choose a reason for hiding this comment

hkshaw1990 commented Jun 22, 2017

basvanbeek commented Jun 22, 2017

hkshaw1990 commented Mar 15, 2017 •

edited

Loading

basvanbeek left a comment •

edited

Loading

basvanbeek Jun 18, 2017 •

edited

Loading

hkshaw1990 Jun 20, 2017 •

edited

Loading