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

Initial observer API #1

Merged
merged 5 commits into from
Jun 15, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# An Observer API for OpenTracing-go Tracers

OTObserver can be used to watch the span events like StartSpan(),
SetOperationName(), SetTag() and Finish(). A need for observers
arose when information (metrics) more than just the latency information was
required for the spans, in the distributed tracers. But, there can be a lot
of metrics in different domains and adding such metrics to any one (client)
tracer breaks cross-platform compatibility. There are various ways to
avoid such issues, however, an observer pattern is cleaner and provides loose
coupling between the packages exporting metrics (on span events) and the
tracer.

This information can be in the form of hardware metrics, RPC metrics,
useful metrics exported out of the kernel or other metrics, profiled for a
span. These additional metrics can help us in getting better Root-cause
analysis. With that being said, its not just for calculation of metrics,
it can be used for anything which needs watching the span events.

## Installation and Usage

The `otobserver` package provides an API. The (client) tracers (if they want
to implement the observer interface) can implement this interface. To do
that, first fetch the package using go get :

Choose a reason for hiding this comment

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

this description seems a bit off. The API provided by the tracers would be something like a functional option to tracer constructor that passes an Observer. The actual observer API is implemented by a 3rd party, not by the tracer itself.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah. Rephrased this in the new commit.


```
go get -v github.com/opentracing-contrib/go-observer
```

and say :

```go
import "github.com/opentracing-contrib/go-observer"
```

and then, define the required span event callbacks. These registered
callbacks would then be called on span events if an observer is created.
Multiple observers can be registered for a given distributed tracer. Have a

Choose a reason for hiding this comment

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

nit: Tracers may allow registering multiple observers (i.e. s/can/may/)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

look at the [jaeger's observer](https://github.com/uber/jaeger-client-go/blob/master/observer.go).

With the required setup implemented in the backend tracers, packages
watching the span events need to implement the observer api defining what
they need to do for the observed span events.

## Span events

An observer registered with this api, can observe for the following four
span events :

```go
StartSpan()
SetOperationName()
SetTag()
Finish()
```

### Tradeoffs

As noble as our thoughts might be in fetching additional metrics (other than
latency) for a span using an observer, there are some overhead costs. Not all
observers need to observe all the span events, in which case, we are keeping
a (or, some) handler function(s) empty. In effect, we will still call

Choose a reason for hiding this comment

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

maybe reword/simplify this phrase?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@yurishkuro the new commit rephrased this and removed the other confusing comments.

these functions, and that will incur overhead. To know more about this and
other tradeoffs, checkout this [discussion](https://github.com/opentracing/opentracing-go/pull/135#discussion_r105497329)
40 changes: 40 additions & 0 deletions observer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright (c) 2017 Hemant Kumar, IBM Corp.
// (c) 2017 Yuri Shkuro, Uber Technologies, Inc.

Choose a reason for hiding this comment

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

you don't need to include this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@yurishkuro Removed that from this file and added a copyright for opentracing-contrib in the LICENSE file (following the convention in other opentracing-contrib projects).

//
// This project is licensed under the Apache License 2.0, see LICENSE.

package otobserver

// Observer can be registered with a Tracer to recieve notifications
// about new Spans. Tracers are not required to support the Observer API.
// The actual registration depends on the implementation, which might look
// like the below e.g :
// observer := myobserver.NewObserver()
// tracer := client.NewTracer(..., client.WithObserver(observer))
//
type Observer interface {
// Create and return a span observer. Called when a span starts.
// E.g :
// func StartSpan(opName string, opts ...opentracing.StartSpanOption) {
// var sp opentracing.Span
// sso := opentracing.StartSpanOptions{}
// var spObs otobserver.SpanObserver = observer.OnStartSpan(span, opName, sso)
// ...
// }
// OnStartSpan function needs to be defined for a package exporting
// metrics as well.

Choose a reason for hiding this comment

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

should this comment be here? it's a bit confusing

OnStartSpan(sp Span, operationName string, options StartSpanOptions) SpanObserver
}

// SpanObserver is created by the Observer and receives notifications about
// other Span events.
// Client tracers should define these functions for each of the span operations
// which should call the registered (observer) callbacks.

Choose a reason for hiding this comment

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

don't think this sentence is useful.

type SpanObserver interface {
// Callback called from opentracing.Span.SetOperationName()
OnSetOperationName(operationName string)
// Callback called from opentracing.Span.SetTag()
OnSetTag(key string, value interface{})
// Callback called from opentracing.Span.Finish()
OnFinish(options FinishOptions)
}