Skip to content
This repository has been archived by the owner on May 23, 2023. It is now read-only.

add a OT-Go semver #51

Closed
wants to merge 3 commits into from
Closed
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
7 changes: 6 additions & 1 deletion examples/dapperish/dapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,10 @@ import (

// NewTracer returns a new dapperish Tracer instance.
func NewTracer(processName string) opentracing.Tracer {
return standardtracer.New(NewTrivialRecorder(processName))
return standardtracer.New(
NewTrivialRecorder(processName),
&opentracing.ImplementationID{
Copy link
Contributor

Choose a reason for hiding this comment

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

you're explicitly letting tracers return nil here? Doesn't seem like a bad idea to force tracers to pick a name and version.

Name: "dapperish",
Version: "0.1.0",
})
}
3 changes: 2 additions & 1 deletion ext/tags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package ext_test
import (
"testing"

"github.com/opentracing/opentracing-go"
"github.com/opentracing/opentracing-go/ext"
"github.com/opentracing/opentracing-go/standardtracer"
"github.com/opentracing/opentracing-go/testutils"
Expand All @@ -14,7 +15,7 @@ func TestPeerTags(t *testing.T) {
t.Fatalf("Invalid PeerService %v", ext.PeerService)
}
recorder := testutils.NewInMemoryRecorder()
tracer := standardtracer.New(recorder)
tracer := standardtracer.New(recorder, &opentracing.ImplementationID{})
span := tracer.StartTrace("my-trace")
ext.PeerService.Add(span, "my-service")
ext.PeerHostname.Add(span, "my-hostname")
Expand Down
15 changes: 12 additions & 3 deletions noop.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,13 @@ type noopSpan struct{}
var (
defaultNoopSpan = noopSpan{}
defaultNoopTracer = NoopTracer{}
emptyTags = Tags{}
emptyBytes = []byte{}
emptyStringMap = map[string]string{}
noopImplID = &ImplementationID{
Name: "noop",
Version: "1.0.0",
}
emptyTags = Tags{}
emptyBytes = []byte{}
emptyStringMap = map[string]string{}
)

const (
Expand Down Expand Up @@ -68,3 +72,8 @@ func (n NoopTracer) StartTrace(operationName string) Span {
func (n NoopTracer) JoinTrace(operationName string, parent interface{}) Span {
return defaultNoopSpan
}

// ImplementationID belongs to the Tracer interface.
func (n NoopTracer) ImplementationID() *ImplementationID {
return noopImplID
}
2 changes: 1 addition & 1 deletion standardtracer/spanpropagator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
func TestSpanPropagator(t *testing.T) {
const op = "test"
recorder := testutils.NewInMemoryRecorder()
tracer := standardtracer.New(recorder)
tracer := standardtracer.New(recorder, &opentracing.ImplementationID{})

sp := tracer.StartTrace(op)
sp.SetTraceAttribute("foo", "bar")
Expand Down
8 changes: 7 additions & 1 deletion standardtracer/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,17 @@ import (

// New creates and returns a standard Tracer which defers to `recorder` after
// RawSpans have been assembled.
func New(recorder SpanRecorder) opentracing.Tracer {
func New(recorder SpanRecorder, implID *opentracing.ImplementationID) opentracing.Tracer {
return &tracerImpl{
recorder: recorder,
implID: implID,
}
}

// Implements the `Tracer` interface.
type tracerImpl struct {
recorder SpanRecorder
implID *opentracing.ImplementationID
}

func (s *tracerImpl) StartTrace(
Expand Down Expand Up @@ -58,3 +60,7 @@ func (s *tracerImpl) startSpanGeneric(
}
return span
}

func (t *tracerImpl) ImplementationID() *opentracing.ImplementationID {
return t.implID
}
17 changes: 17 additions & 0 deletions tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,21 @@ type Tracer interface {
// SetTag("lucky_number", 42)
//
StartTrace(operationName string) Span

// ImplementationID returns information about the OpenTracing
// implementation backing the Tracer. The return value should not change
// over the life of a particular Tracer instance.
ImplementationID() *ImplementationID
}

// ImplementationID is a simple, extensible struct that describes an
// OpenTracing implementation.
type ImplementationID struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(the only interesting part of this change)

// The (stable) name of the implementation. E.g., "Dapper" or "Zipkin". The
// Name should not reflect the host language or platform.
Name string

// Version may take any form, but SemVer (http://semver.org/) is strongly
// encouraged.
Version string
Copy link

Choose a reason for hiding this comment

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

@bensigelman , this looks pretty good, but Go seems a bit like the exceptional case with regards to the utility of a version string (since everything in Go is basically statically bound). I'm curious what this might look like for Python. I'm specifically considering the case where the supported version of the OpenTracing standard is what's of interest rather than the underlying implementation version. E.g. if an arbitrary piece of Python code is handed a tracer object, how can it (cleanly) check the supported version of OpenTracing on that object?

(By the way I believe I'm safe in assuming that in the above Version is intended to represent the implementation version given the struct name and the "may take any form" comment.)

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @bcronin, it's the OpenTracing version that the original ticket was asking for. We said it would be best done via Capabilities API rather than a version. At the moment, however, there doesn't seem to be any aspects of the API that are optional and require introspection by the client code.

}