From 79e44f71bf4c086676d38da40982499a4f254c3a Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Mon, 28 Dec 2015 15:45:43 -0500 Subject: [PATCH 1/4] Adds ext/tags mechanism for adding common tags to the span --- Makefile | 4 +- examples/{dapperish/main.go => dapperish.go} | 3 +- examples/dapperish/dapper.go | 2 +- examples/dapperish/random.go | 2 +- examples/dapperish/trivialrecorder.go | 2 +- opentracing/ext/tags.go | 51 +++++++++++++++ opentracing/ext/tags_test.go | 35 +++++++++++ opentracing/raw.go | 4 +- opentracing/standardtracer/standardimpl.go | 4 +- testutils/assert/assert.go | 57 +++++++++++++++++ testutils/assert/assert_test.go | 11 ++++ testutils/recorder.go | 66 ++++++++++++++++++++ testutils/recorder_test.go | 42 +++++++++++++ 13 files changed, 272 insertions(+), 11 deletions(-) rename examples/{dapperish/main.go => dapperish.go} (93%) create mode 100644 opentracing/ext/tags.go create mode 100644 opentracing/ext/tags_test.go create mode 100644 testutils/assert/assert.go create mode 100644 testutils/assert/assert_test.go create mode 100644 testutils/recorder.go create mode 100644 testutils/recorder_test.go diff --git a/Makefile b/Makefile index 9179c2b..240106c 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,7 @@ .PHONY: test test: - go test ./... + go test -v -cover ./... .PHONY: lint lint: @@ -16,5 +16,5 @@ vet: .PHONY: example example: - go build -o build/dapperish-example ./examples/dapperish/. + go build -o build/dapperish-example ./examples/dapperish.go ./build/dapperish-example diff --git a/examples/dapperish/main.go b/examples/dapperish.go similarity index 93% rename from examples/dapperish/main.go rename to examples/dapperish.go index 012f895..4dd9aa9 100644 --- a/examples/dapperish/main.go +++ b/examples/dapperish.go @@ -13,6 +13,7 @@ import ( "strings" "github.com/opentracing/api-golang/opentracing" + "github.com/opentracing/api-golang/examples/dapperish" ) func client() { @@ -78,7 +79,7 @@ func server() { } func main() { - opentracing.InitDefaultTracer(NewTracer("dapperish_tester")) + opentracing.InitDefaultTracer(dapperish.NewTracer("dapperish_tester")) go server() go client() diff --git a/examples/dapperish/dapper.go b/examples/dapperish/dapper.go index 59361e6..03f3045 100644 --- a/examples/dapperish/dapper.go +++ b/examples/dapperish/dapper.go @@ -1,4 +1,4 @@ -package main +package dapperish import ( "bytes" diff --git a/examples/dapperish/random.go b/examples/dapperish/random.go index 14b4b94..3dd78c6 100644 --- a/examples/dapperish/random.go +++ b/examples/dapperish/random.go @@ -1,4 +1,4 @@ -package main +package dapperish import ( "math/rand" diff --git a/examples/dapperish/trivialrecorder.go b/examples/dapperish/trivialrecorder.go index c98edd9..b240dd0 100644 --- a/examples/dapperish/trivialrecorder.go +++ b/examples/dapperish/trivialrecorder.go @@ -1,4 +1,4 @@ -package main +package dapperish import ( "fmt" diff --git a/opentracing/ext/tags.go b/opentracing/ext/tags.go new file mode 100644 index 0000000..7eb3b6b --- /dev/null +++ b/opentracing/ext/tags.go @@ -0,0 +1,51 @@ +package ext + +import ( + "github.com/opentracing/api-golang/opentracing" +) + +var ( + // RPC tags as symmetrical, each can be emitted by either client-side of server-side + + // RPCService records the service name of the RPC peer + RPCService = &stringTag{"rpc.service"} + + // RPCHostname records the host name of the RPC peer + RPCHostname = &stringTag{"rpc.hostname"} + + // RPCHostIPv4 records IP v4 host address of the RPC peer + RPCHostIPv4 = &uint32Tag{"rpc.ipv4"} + + // RPCHostIPv6 records IP v6 host address of the RPC peer + RPCHostIPv6 = &stringTag{"rpc.ipv6"} + + // RPCPort records port number of the RPC peer + RPCPort = &uint16Tag{"rpc.port"} +) + +type stringTag struct { + Key string +} + +// Add adds a string tag to the `span` +func (tag *stringTag) Add(span opentracing.Span, value string) { + span.SetTag(tag.Key, value) +} + +type uint32Tag struct { + Key string +} + +// Add adds a uint32 tag to the `span` +func (tag *uint32Tag) Add(span opentracing.Span, value uint32) { + span.SetTag(tag.Key, value) +} + +type uint16Tag struct { + Key string +} + +// Add adds a uint16 tag to the `span` +func (tag *uint16Tag) Add(span opentracing.Span, value uint16) { + span.SetTag(tag.Key, value) +} diff --git a/opentracing/ext/tags_test.go b/opentracing/ext/tags_test.go new file mode 100644 index 0000000..c84e9f9 --- /dev/null +++ b/opentracing/ext/tags_test.go @@ -0,0 +1,35 @@ +package ext_test + +import ( + "testing" + + "github.com/opentracing/api-golang/examples/dapperish" + "github.com/opentracing/api-golang/opentracing/ext" + "github.com/opentracing/api-golang/opentracing/standardtracer" + "github.com/opentracing/api-golang/testutils" + "github.com/opentracing/api-golang/testutils/assert" +) + +func TestRPCTags(t *testing.T) { + if ext.RPCService.Key != "rpc.service" { + t.Fatalf("Invalid RPCService.Key %v", ext.RPCService.Key) + } + recorder := testutils.NewInMemoryRecorder("test-process") + tracer := standardtracer.New(recorder, dapperish.NewTraceContextSource()) + span := tracer.StartTrace("my-trace") + ext.RPCService.Add(span, "my-service") + ext.RPCHostname.Add(span, "my-hostname") + ext.RPCHostIPv4.Add(span, uint32(127<<24|1)) + ext.RPCHostIPv6.Add(span, "::") + ext.RPCPort.Add(span, uint16(8080)) + span.Finish() + if len(recorder.GetSpans()) != 1 { + t.Fatal("Span not recorded") + } + rawSpan := recorder.GetSpans()[0] + assert.EqualValues(t, "my-service", rawSpan.Tags[ext.RPCService.Key]) + assert.EqualValues(t, "my-hostname", rawSpan.Tags[ext.RPCHostname.Key]) + assert.EqualValues(t, uint32(127<<24|1), rawSpan.Tags[ext.RPCHostIPv4.Key]) + assert.EqualValues(t, "::", rawSpan.Tags[ext.RPCHostIPv6.Key]) + assert.EqualValues(t, 8080, rawSpan.Tags[ext.RPCPort.Key]) +} diff --git a/opentracing/raw.go b/opentracing/raw.go index 200ffac..a53d586 100644 --- a/opentracing/raw.go +++ b/opentracing/raw.go @@ -3,9 +3,7 @@ package opentracing import "time" // Tags is a generic map from an arbitrary string key to an opaque value type. -// -// TODO: decide on restrictions (if any) for the values: just strings and POD? -// just simple maps and slices of same? arbitrary objects? +// The underlying tracing system is responsible for interpreting and serializing the values. type Tags map[string]interface{} // RawSpan encapsulates all state associated with a (finished) Span. diff --git a/opentracing/standardtracer/standardimpl.go b/opentracing/standardtracer/standardimpl.go index f4d0036..ff08885 100644 --- a/opentracing/standardtracer/standardimpl.go +++ b/opentracing/standardtracer/standardimpl.go @@ -37,7 +37,7 @@ func (s *standardSpan) StartChild(operationName string) opentracing.Span { func (s *standardSpan) SetTag(key string, value interface{}) opentracing.Span { s.lock.Lock() defer s.lock.Unlock() - s.raw.Tags[key] = fmt.Sprint(value) + s.raw.Tags[key] = value return s } @@ -46,7 +46,7 @@ func (s *standardSpan) SetTags(tags opentracing.Tags) opentracing.Span { defer s.lock.Unlock() for k, v := range tags { - s.raw.Tags[k] = fmt.Sprint(v) + s.raw.Tags[k] = v } return s } diff --git a/testutils/assert/assert.go b/testutils/assert/assert.go new file mode 100644 index 0000000..af5ee34 --- /dev/null +++ b/testutils/assert/assert.go @@ -0,0 +1,57 @@ +package assert + +import ( + "reflect" + "testing" +) + +// NOTE: the functions in this module are borrowed from https://github.com/stretchr/testify +// to avoid creating a dependency (until Go comes up with proper dependency manager) + +// EqualValues asserts that two objects are equal or convertable to the same types +// and equal. +// +// assert.EqualValues(t, uint32(123), int32(123), "123 and 123 should be equal") +// +// If assertion is not successful, execution halts via t.Fatalf() call +func EqualValues(t *testing.T, expected, actual interface{}) bool { + if !ObjectsAreEqualValues(expected, actual) { + t.Fatalf("Not equal: %#v (expected)\n"+ + " != %#v (actual)", expected, actual) + } + return true +} + +// ObjectsAreEqual determines if two objects are considered equal. +func ObjectsAreEqual(expected, actual interface{}) bool { + + if expected == nil || actual == nil { + return expected == actual + } + + if reflect.DeepEqual(expected, actual) { + return true + } + + return false + +} + +// ObjectsAreEqualValues determines whether two objects are equal, +// or if their values are equal. +func ObjectsAreEqualValues(expected, actual interface{}) bool { + if ObjectsAreEqual(expected, actual) { + return true + } + + actualType := reflect.TypeOf(actual) + expectedValue := reflect.ValueOf(expected) + if expectedValue.Type().ConvertibleTo(actualType) { + // Attempt comparison after type conversion + if reflect.DeepEqual(actual, expectedValue.Convert(actualType).Interface()) { + return true + } + } + + return false +} diff --git a/testutils/assert/assert_test.go b/testutils/assert/assert_test.go new file mode 100644 index 0000000..86fb657 --- /dev/null +++ b/testutils/assert/assert_test.go @@ -0,0 +1,11 @@ +package assert_test + +import ( + "github.com/opentracing/api-golang/testutils/assert" + "testing" +) + +func TestAssert(t *testing.T) { + assert.EqualValues(t, int16(123), int16(123)) + assert.EqualValues(t, int16(123), int32(123)) +} diff --git a/testutils/recorder.go b/testutils/recorder.go new file mode 100644 index 0000000..772235b --- /dev/null +++ b/testutils/recorder.go @@ -0,0 +1,66 @@ +package testutils + +import ( + "sync" + + "github.com/opentracing/api-golang/opentracing" +) + +// InMemoryRecorder is a simple thread-safe implementation of opentracing.Recorder +// that stores all reported spans in memory, accessible via reporter.GetSpans() +type InMemoryRecorder struct { + processName string + spans []*opentracing.RawSpan + tags opentracing.Tags + lock sync.Mutex +} + +// NewInMemoryRecorder instantiates a new InMemoryRecorder with the given `processName` +func NewInMemoryRecorder(processName string) *InMemoryRecorder { + return &InMemoryRecorder{ + processName: processName, + spans: make([]*opentracing.RawSpan, 0), + tags: make(opentracing.Tags), + } +} + +// ProcessName implements ProcessName() of opentracing.Recorder +func (recorder *InMemoryRecorder) ProcessName() string { + return recorder.processName +} + +// SetTag implements SetTag() of opentracing.Recorder. Tags can be retrieved via recorder.GetTags() +func (recorder *InMemoryRecorder) SetTag(key string, val interface{}) opentracing.ProcessIdentifier { + recorder.lock.Lock() + defer recorder.lock.Unlock() + recorder.tags[key] = val + return recorder +} + +// RecordSpan implements RecordSpan() of opentracing.Recorder. +// The recorder spans can be retrieved via recorder.Spans slice. +func (recorder *InMemoryRecorder) RecordSpan(span *opentracing.RawSpan) { + recorder.lock.Lock() + defer recorder.lock.Unlock() + recorder.spans = append(recorder.spans, span) +} + +// GetSpans returns a snapshot of spans recorded so far. +func (recorder *InMemoryRecorder) GetSpans() []*opentracing.RawSpan { + recorder.lock.Lock() + defer recorder.lock.Unlock() + spans := make([]*opentracing.RawSpan, len(recorder.spans)) + copy(spans, recorder.spans) + return spans +} + +// GetTags returns a snapshot of tags. +func (recorder *InMemoryRecorder) GetTags() opentracing.Tags { + recorder.lock.Lock() + defer recorder.lock.Unlock() + tags := make(opentracing.Tags) + for k, v := range recorder.tags { + tags[k] = v + } + return tags +} diff --git a/testutils/recorder_test.go b/testutils/recorder_test.go new file mode 100644 index 0000000..a4c9241 --- /dev/null +++ b/testutils/recorder_test.go @@ -0,0 +1,42 @@ +package testutils_test + +import ( + "testing" + "time" + + "github.com/opentracing/api-golang/examples/dapperish" + "github.com/opentracing/api-golang/opentracing" + "github.com/opentracing/api-golang/testutils" +) + +func TestInMemoryRecorderSpans(t *testing.T) { + recorder := testutils.NewInMemoryRecorder("unit-test") + var apiRecorder opentracing.Recorder = recorder + if apiRecorder.ProcessName() != "unit-test" { + t.Fatalf("Invalid process name") + } + span := &opentracing.RawSpan{ + TraceContext: &dapperish.DapperishTraceContext{}, + Operation: "test-span", + Start: time.Now(), + Duration: -1, + } + apiRecorder.RecordSpan(span) + if len(recorder.GetSpans()) != 1 { + t.Fatal("No spans recorded") + } + if recorder.GetSpans()[0] != span { + t.Fatal("Span not recorded") + } +} + +func TestInMemoryRecorderTags(t *testing.T) { + recorder := testutils.NewInMemoryRecorder("unit-test") + recorder.SetTag("tag1", "hello") + if len(recorder.GetTags()) != 1 { + t.Fatal("Tag not stored") + } + if recorder.GetTags()["tag1"] != "hello" { + t.Fatal("tag1 != hello") + } +} From c19dd2a3b91ae7856a3e8b9fab4744a5a64689ab Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Mon, 28 Dec 2015 16:17:44 -0500 Subject: [PATCH 2/4] Implement and use a dummy SimpleTraceContextSource instead of using dapperish in main tests --- examples/dapperish.go | 2 +- opentracing/ext/tags_test.go | 3 +- testutils/trace_context.go | 49 +++++++++++++++++++++++++++++++++ testutils/trace_context_test.go | 8 ++++++ 4 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 testutils/trace_context.go create mode 100644 testutils/trace_context_test.go diff --git a/examples/dapperish.go b/examples/dapperish.go index 4dd9aa9..0257516 100644 --- a/examples/dapperish.go +++ b/examples/dapperish.go @@ -12,8 +12,8 @@ import ( "runtime" "strings" - "github.com/opentracing/api-golang/opentracing" "github.com/opentracing/api-golang/examples/dapperish" + "github.com/opentracing/api-golang/opentracing" ) func client() { diff --git a/opentracing/ext/tags_test.go b/opentracing/ext/tags_test.go index c84e9f9..c9c525d 100644 --- a/opentracing/ext/tags_test.go +++ b/opentracing/ext/tags_test.go @@ -3,7 +3,6 @@ package ext_test import ( "testing" - "github.com/opentracing/api-golang/examples/dapperish" "github.com/opentracing/api-golang/opentracing/ext" "github.com/opentracing/api-golang/opentracing/standardtracer" "github.com/opentracing/api-golang/testutils" @@ -15,7 +14,7 @@ func TestRPCTags(t *testing.T) { t.Fatalf("Invalid RPCService.Key %v", ext.RPCService.Key) } recorder := testutils.NewInMemoryRecorder("test-process") - tracer := standardtracer.New(recorder, dapperish.NewTraceContextSource()) + tracer := standardtracer.New(recorder, &testutils.SimpleTraceContextSource{}) span := tracer.StartTrace("my-trace") ext.RPCService.Add(span, "my-service") ext.RPCHostname.Add(span, "my-hostname") diff --git a/testutils/trace_context.go b/testutils/trace_context.go new file mode 100644 index 0000000..d961db9 --- /dev/null +++ b/testutils/trace_context.go @@ -0,0 +1,49 @@ +package testutils + +import ( + "github.com/opentracing/api-golang/opentracing" +) + +// SimpleTraceContextSource is a dummy implementation of TraceContextSource. +type SimpleTraceContextSource struct{} + +// NewRootTraceContext imlements NewRootTraceContext of opentracing.TraceContextSource. +func (source *SimpleTraceContextSource) NewRootTraceContext() opentracing.TraceContext { + return nil +} + +// MarshalTraceContextBinary imlements MarshalTraceContextBinary of opentracing.TraceContextSource. +func (source *SimpleTraceContextSource) MarshalTraceContextBinary( + tc opentracing.TraceContext, +) ( + traceContextID []byte, + traceTags []byte, +) { + panic("Not implemented") +} + +// MarshalTraceContextStringMap imlements MarshalTraceContextStringMap of opentracing.TraceContextSource. +func (source *SimpleTraceContextSource) MarshalTraceContextStringMap( + tc opentracing.TraceContext, +) ( + traceContextID map[string]string, + traceTags map[string]string, +) { + panic("Not implemented") +} + +// UnmarshalTraceContextBinary imlements UnmarshalTraceContextBinary of opentracing.TraceContextSource. +func (source *SimpleTraceContextSource) UnmarshalTraceContextBinary( + traceContextID []byte, + traceTags []byte, +) (opentracing.TraceContext, error) { + panic("Not implemented") +} + +// UnmarshalTraceContextStringMap imlements UnmarshalTraceContextStringMap of opentracing.TraceContextSource. +func (source *SimpleTraceContextSource) UnmarshalTraceContextStringMap( + traceContextID map[string]string, + traceTags map[string]string, +) (opentracing.TraceContext, error) { + panic("Not implemented") +} diff --git a/testutils/trace_context_test.go b/testutils/trace_context_test.go new file mode 100644 index 0000000..dc0cc0a --- /dev/null +++ b/testutils/trace_context_test.go @@ -0,0 +1,8 @@ +package testutils + +import "testing" + +func TestSimpleTraceContextSource(t *testing.T) { + ctxSrc := &SimpleTraceContextSource{} + ctxSrc.NewRootTraceContext() +} From 8c6cc6392d564ee6b9a8f55cf8434886da48bbad Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Mon, 28 Dec 2015 17:08:14 -0500 Subject: [PATCH 3/4] Update SetTag comment to clarify support for arbitrary value types --- opentracing/span.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/opentracing/span.go b/opentracing/span.go index 254cbf8..4c810bf 100644 --- a/opentracing/span.go +++ b/opentracing/span.go @@ -9,8 +9,14 @@ type Span interface { // Creates and starts a child span. StartChild(operationName string) Span - // Adds a tag to the span. The `value` is immediately coerced into a string - // using fmt.Sprint(). + // Adds a tag to the span. + // + // Tag values can be of arbitrary types, however the treatment of complex + // types is dependent on the underlying tracing system implementation. + // It is expected that most tracing systems will handle primitive types + // like strings and numbers. If a tracing system cannot understand how + // to handle a particular value type, it may ignore the tag, but shall + // not panic. // // If there is a pre-existing tag set for `key`, it is overwritten. SetTag(key string, value interface{}) Span From 00ab68527cdd918827291735f99974965fc3b396 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Mon, 28 Dec 2015 21:53:42 -0500 Subject: [PATCH 4/4] Rename tags from rpc.* to peer.* --- opentracing/ext/tags.go | 23 ++++++++++++----------- opentracing/ext/tags_test.go | 26 +++++++++++++------------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/opentracing/ext/tags.go b/opentracing/ext/tags.go index 7eb3b6b..6e22a69 100644 --- a/opentracing/ext/tags.go +++ b/opentracing/ext/tags.go @@ -5,22 +5,23 @@ import ( ) var ( - // RPC tags as symmetrical, each can be emitted by either client-side of server-side + // PeerXXX tags can be emitted by either client-side of server-side to describe + // the other side/service in a peer-to-peer communications, like an RPC call. - // RPCService records the service name of the RPC peer - RPCService = &stringTag{"rpc.service"} + // PeerService records the service name of the peer + PeerService = &stringTag{"peer.service"} - // RPCHostname records the host name of the RPC peer - RPCHostname = &stringTag{"rpc.hostname"} + // PeerHostname records the host name of the peer + PeerHostname = &stringTag{"peer.hostname"} - // RPCHostIPv4 records IP v4 host address of the RPC peer - RPCHostIPv4 = &uint32Tag{"rpc.ipv4"} + // PeerHostIPv4 records IP v4 host address of the peer + PeerHostIPv4 = &uint32Tag{"peer.ipv4"} - // RPCHostIPv6 records IP v6 host address of the RPC peer - RPCHostIPv6 = &stringTag{"rpc.ipv6"} + // PeerHostIPv6 records IP v6 host address of the peer + PeerHostIPv6 = &stringTag{"peer.ipv6"} - // RPCPort records port number of the RPC peer - RPCPort = &uint16Tag{"rpc.port"} + // PeerPort records port number of the peer + PeerPort = &uint16Tag{"peer.port"} ) type stringTag struct { diff --git a/opentracing/ext/tags_test.go b/opentracing/ext/tags_test.go index c9c525d..275d86f 100644 --- a/opentracing/ext/tags_test.go +++ b/opentracing/ext/tags_test.go @@ -9,26 +9,26 @@ import ( "github.com/opentracing/api-golang/testutils/assert" ) -func TestRPCTags(t *testing.T) { - if ext.RPCService.Key != "rpc.service" { - t.Fatalf("Invalid RPCService.Key %v", ext.RPCService.Key) +func TestPeerTags(t *testing.T) { + if ext.PeerService.Key != "peer.service" { + t.Fatalf("Invalid PeerService.Key %v", ext.PeerService.Key) } recorder := testutils.NewInMemoryRecorder("test-process") tracer := standardtracer.New(recorder, &testutils.SimpleTraceContextSource{}) span := tracer.StartTrace("my-trace") - ext.RPCService.Add(span, "my-service") - ext.RPCHostname.Add(span, "my-hostname") - ext.RPCHostIPv4.Add(span, uint32(127<<24|1)) - ext.RPCHostIPv6.Add(span, "::") - ext.RPCPort.Add(span, uint16(8080)) + ext.PeerService.Add(span, "my-service") + ext.PeerHostname.Add(span, "my-hostname") + ext.PeerHostIPv4.Add(span, uint32(127<<24|1)) + ext.PeerHostIPv6.Add(span, "::") + ext.PeerPort.Add(span, uint16(8080)) span.Finish() if len(recorder.GetSpans()) != 1 { t.Fatal("Span not recorded") } rawSpan := recorder.GetSpans()[0] - assert.EqualValues(t, "my-service", rawSpan.Tags[ext.RPCService.Key]) - assert.EqualValues(t, "my-hostname", rawSpan.Tags[ext.RPCHostname.Key]) - assert.EqualValues(t, uint32(127<<24|1), rawSpan.Tags[ext.RPCHostIPv4.Key]) - assert.EqualValues(t, "::", rawSpan.Tags[ext.RPCHostIPv6.Key]) - assert.EqualValues(t, 8080, rawSpan.Tags[ext.RPCPort.Key]) + assert.EqualValues(t, "my-service", rawSpan.Tags[ext.PeerService.Key]) + assert.EqualValues(t, "my-hostname", rawSpan.Tags[ext.PeerHostname.Key]) + assert.EqualValues(t, uint32(127<<24|1), rawSpan.Tags[ext.PeerHostIPv4.Key]) + assert.EqualValues(t, "::", rawSpan.Tags[ext.PeerHostIPv6.Key]) + assert.EqualValues(t, 8080, rawSpan.Tags[ext.PeerPort.Key]) }