From 7966e63342509d2a9814f8cb31dafe733bf0fe54 Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Wed, 25 Sep 2019 19:30:02 +0200 Subject: [PATCH] More cleanups from #100 (#142) * Drop dead event type * Replace EventType from reader with one from exporter * Do not concat the strings for string buffer * Fill scope ID in events That required renaming a variable, because it had the same name as the type we wanted to assert later. * Do not fail quietly when span context is busted --- .../streaming/exporter/eventtype_string.go | 19 ++++++------ experimental/streaming/exporter/exporter.go | 1 - .../exporter/reader/format/format.go | 29 ++++++++++++------- .../streaming/exporter/reader/reader.go | 29 +++++-------------- .../streaming/exporter/spandata/spandata.go | 7 ++--- experimental/streaming/sdk/span.go | 1 + experimental/streaming/sdk/span_test.go | 9 ++++-- 7 files changed, 46 insertions(+), 49 deletions(-) diff --git a/experimental/streaming/exporter/eventtype_string.go b/experimental/streaming/exporter/eventtype_string.go index 7b31414f816..6fb67049acc 100644 --- a/experimental/streaming/exporter/eventtype_string.go +++ b/experimental/streaming/exporter/eventtype_string.go @@ -12,19 +12,18 @@ func _() { _ = x[START_SPAN-1] _ = x[FINISH_SPAN-2] _ = x[ADD_EVENT-3] - _ = x[ADD_EVENTF-4] - _ = x[NEW_SCOPE-5] - _ = x[NEW_MEASURE-6] - _ = x[NEW_METRIC-7] - _ = x[MODIFY_ATTR-8] - _ = x[RECORD_STATS-9] - _ = x[SET_STATUS-10] - _ = x[SET_NAME-11] + _ = x[NEW_SCOPE-4] + _ = x[NEW_MEASURE-5] + _ = x[NEW_METRIC-6] + _ = x[MODIFY_ATTR-7] + _ = x[RECORD_STATS-8] + _ = x[SET_STATUS-9] + _ = x[SET_NAME-10] } -const _EventType_name = "INVALIDSTART_SPANFINISH_SPANADD_EVENTADD_EVENTFNEW_SCOPENEW_MEASURENEW_METRICMODIFY_ATTRRECORD_STATSSET_STATUSSET_NAME" +const _EventType_name = "INVALIDSTART_SPANFINISH_SPANADD_EVENTNEW_SCOPENEW_MEASURENEW_METRICMODIFY_ATTRRECORD_STATSSET_STATUSSET_NAME" -var _EventType_index = [...]uint8{0, 7, 17, 28, 37, 47, 56, 67, 77, 88, 100, 110, 118} +var _EventType_index = [...]uint8{0, 7, 17, 28, 37, 46, 57, 67, 78, 90, 100, 108} func (i EventType) String() string { if i < 0 || i >= EventType(len(_EventType_index)-1) { diff --git a/experimental/streaming/exporter/exporter.go b/experimental/streaming/exporter/exporter.go index fa5311bde61..e6eda7c26ac 100644 --- a/experimental/streaming/exporter/exporter.go +++ b/experimental/streaming/exporter/exporter.go @@ -57,7 +57,6 @@ const ( START_SPAN FINISH_SPAN ADD_EVENT - ADD_EVENTF NEW_SCOPE NEW_MEASURE NEW_METRIC diff --git a/experimental/streaming/exporter/reader/format/format.go b/experimental/streaming/exporter/reader/format/format.go index c1de80482e6..e828ba872b4 100644 --- a/experimental/streaming/exporter/reader/format/format.go +++ b/experimental/streaming/exporter/reader/format/format.go @@ -20,6 +20,7 @@ import ( "go.opentelemetry.io/api/core" "go.opentelemetry.io/api/key" + "go.opentelemetry.io/experimental/streaming/exporter" "go.opentelemetry.io/experimental/streaming/exporter/reader" // TODO this should not be an SDK dependency; move conventional tags into the API. @@ -37,7 +38,10 @@ func AppendEvent(buf *strings.Builder, data reader.Event) { if skipIf && data.Attributes.HasValue(kv.Key) { return true } - buf.WriteString(" " + kv.Key.Name + "=" + kv.Value.Emit()) + buf.WriteString(" ") + buf.WriteString(kv.Key.Name) + buf.WriteString("=") + buf.WriteString(kv.Value.Emit()) return true } } @@ -46,7 +50,7 @@ func AppendEvent(buf *strings.Builder, data reader.Event) { buf.WriteString(" ") switch data.Type { - case reader.START_SPAN: + case exporter.START_SPAN: buf.WriteString("start ") buf.WriteString(data.Name) @@ -61,7 +65,7 @@ func AppendEvent(buf *strings.Builder, data reader.Event) { buf.WriteString(" >") } - case reader.FINISH_SPAN: + case exporter.FINISH_SPAN: buf.WriteString("finish ") buf.WriteString(data.Name) @@ -69,19 +73,23 @@ func AppendEvent(buf *strings.Builder, data reader.Event) { buf.WriteString(data.Duration.String()) buf.WriteString(")") - case reader.ADD_EVENT: + case exporter.ADD_EVENT: buf.WriteString("event: ") buf.WriteString(data.Message) buf.WriteString(" (") data.Attributes.Foreach(func(kv core.KeyValue) bool { - buf.WriteString(" " + kv.Key.Name + "=" + kv.Value.Emit()) + buf.WriteString(" ") + buf.WriteString(kv.Key.Name) + buf.WriteString("=") + buf.WriteString(kv.Value.Emit()) return true }) buf.WriteString(")") - case reader.MODIFY_ATTR: - buf.WriteString("modify attr") - case reader.RECORD_STATS: + case exporter.MODIFY_ATTR: + buf.WriteString("modify attr ") + buf.WriteString(data.Type.String()) + case exporter.RECORD_STATS: buf.WriteString("record") for _, s := range data.Stats { @@ -103,11 +111,12 @@ func AppendEvent(buf *strings.Builder, data reader.Event) { }) buf.WriteString("}") } - case reader.SET_STATUS: + + case exporter.SET_STATUS: buf.WriteString("set status ") buf.WriteString(data.Status.String()) - case reader.SET_NAME: + case exporter.SET_NAME: buf.WriteString("set name ") buf.WriteString(data.Name) diff --git a/experimental/streaming/exporter/reader/reader.go b/experimental/streaming/exporter/reader/reader.go index d583502569b..9b904f5a49c 100644 --- a/experimental/streaming/exporter/reader/reader.go +++ b/experimental/streaming/exporter/reader/reader.go @@ -31,10 +31,8 @@ type Reader interface { Read(Event) } -type EventType int - type Event struct { - Type EventType + Type exporter.EventType Time time.Time Sequence exporter.EventID SpanContext core.SpanContext @@ -94,17 +92,6 @@ type readerScope struct { attributes tag.Map } -const ( - INVALID EventType = iota - START_SPAN - FINISH_SPAN - ADD_EVENT - MODIFY_ATTR - RECORD_STATS - SET_STATUS - SET_NAME -) - // NewReaderObserver returns an implementation that computes the // necessary state needed by a reader to process events in memory. // Practically, this means tracking live metric handles and scope @@ -149,7 +136,7 @@ func (ro *readerObserver) orderedObserve(event exporter.Event) { span.readerScope.attributes = rattrs read.Name = span.name - read.Type = START_SPAN + read.Type = exporter.START_SPAN read.SpanContext = span.spanContext read.Attributes = rattrs @@ -177,7 +164,7 @@ func (ro *readerObserver) orderedObserve(event exporter.Event) { } read.Name = span.name - read.Type = FINISH_SPAN + read.Type = exporter.FINISH_SPAN read.Attributes = attrs read.Duration = event.Time.Sub(span.start) @@ -227,7 +214,7 @@ func (ro *readerObserver) orderedObserve(event exporter.Event) { return } - read.Type = MODIFY_ATTR + read.Type = exporter.MODIFY_ATTR read.Attributes = sc.attributes if span != nil { @@ -254,7 +241,7 @@ func (ro *readerObserver) orderedObserve(event exporter.Event) { return case exporter.ADD_EVENT: - read.Type = ADD_EVENT + read.Type = exporter.ADD_EVENT read.Message = event.String attrs, span := ro.readScope(event.Scope) @@ -266,7 +253,7 @@ func (ro *readerObserver) orderedObserve(event exporter.Event) { } case exporter.RECORD_STATS: - read.Type = RECORD_STATS + read.Type = exporter.RECORD_STATS _, span := ro.readScope(event.Scope) if span != nil { @@ -280,7 +267,7 @@ func (ro *readerObserver) orderedObserve(event exporter.Event) { } case exporter.SET_STATUS: - read.Type = SET_STATUS + read.Type = exporter.SET_STATUS read.Status = event.Status _, span := ro.readScope(event.Scope) if span != nil { @@ -289,7 +276,7 @@ func (ro *readerObserver) orderedObserve(event exporter.Event) { } case exporter.SET_NAME: - read.Type = SET_NAME + read.Type = exporter.SET_NAME read.Name = event.String default: diff --git a/experimental/streaming/exporter/spandata/spandata.go b/experimental/streaming/exporter/spandata/spandata.go index 79f6e620f4f..20e3ae45100 100644 --- a/experimental/streaming/exporter/spandata/spandata.go +++ b/experimental/streaming/exporter/spandata/spandata.go @@ -42,11 +42,10 @@ func NewReaderObserver(readers ...Reader) exporter.Observer { func (s *spanReader) Read(data reader.Event) { if !data.SpanContext.HasSpanID() { - // @@@ This is happening, somehow span context is busted. - return + panic("How is this?") } var span *Span - if data.Type == reader.START_SPAN { + if data.Type == exporter.START_SPAN { span = &Span{Events: make([]reader.Event, 0, 4)} s.spans[data.SpanContext] = span } else { @@ -59,7 +58,7 @@ func (s *spanReader) Read(data reader.Event) { span.Events = append(span.Events, data) - if data.Type == reader.FINISH_SPAN { + if data.Type == exporter.FINISH_SPAN { for _, r := range s.readers { r.Read(span) } diff --git a/experimental/streaming/sdk/span.go b/experimental/streaming/sdk/span.go index c6b5d4aaf3c..de51ab0c089 100644 --- a/experimental/streaming/sdk/span.go +++ b/experimental/streaming/sdk/span.go @@ -120,6 +120,7 @@ func (sp *span) addEventWithTime(ctx context.Context, timestamp time.Time, msg s sp.tracer.exporter.Record(exporter.Event{ Time: timestamp, Type: exporter.ADD_EVENT, + Scope: sp.ScopeID(), String: msg, Attributes: attrs, Context: ctx, diff --git a/experimental/streaming/sdk/span_test.go b/experimental/streaming/sdk/span_test.go index 722b5d02377..0509d64f495 100644 --- a/experimental/streaming/sdk/span_test.go +++ b/experimental/streaming/sdk/span_test.go @@ -35,14 +35,14 @@ func TestEvents(t *testing.T) { _ = New(obs).WithSpan(context.Background(), "test", func(ctx context.Context) error { type test1Type struct{} type test2Type struct{} - span := trace.CurrentSpan(ctx) + sp := trace.CurrentSpan(ctx) k1v1 := key.New("k1").String("v1") k2v2 := key.New("k2").String("v2") k3v3 := key.New("k3").String("v3") ctx1 := context.WithValue(ctx, test1Type{}, 42) - span.AddEvent(ctx1, "one two three", k1v1) + sp.AddEvent(ctx1, "one two three", k1v1) ctx2 := context.WithValue(ctx1, test2Type{}, "foo") - span.AddEvent(ctx2, "testing", k2v2, k3v3) + sp.AddEvent(ctx2, "testing", k2v2, k3v3) got := obs.Events(exporter.ADD_EVENT) for idx := range got { @@ -54,16 +54,19 @@ func TestEvents(t *testing.T) { if len(got) != 2 { t.Errorf("Expected two events, got %d", len(got)) } + sdkSpan := sp.(*span) want := []exporter.Event{ { Type: exporter.ADD_EVENT, String: "one two three", Attributes: []core.KeyValue{k1v1}, + Scope: sdkSpan.ScopeID(), }, { Type: exporter.ADD_EVENT, String: "testing", Attributes: []core.KeyValue{k2v2, k3v3}, + Scope: sdkSpan.ScopeID(), }, } if diffEvents(t, got, want) {