From eb9fe13a777b691cc1ecfd1a29bdbe6368fc0e9d Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Tue, 3 Dec 2019 06:58:55 +0100 Subject: [PATCH] Drop SetAttribute from Span (#361) fixes #302 --- api/testharness/harness.go | 3 --- api/trace/api.go | 1 - api/trace/current_test.go | 4 ---- api/trace/noop_span.go | 4 ---- bridge/opentracing/bridge.go | 2 +- bridge/opentracing/internal/mock.go | 12 +++------- example/basic/main.go | 2 +- example/namedtracer/foo/foo.go | 2 +- exporter/trace/stackdriver/trace.go | 2 +- internal/trace/mock_span.go | 4 ---- plugin/httptrace/clienttrace.go | 6 ++--- plugin/othttp/handler.go | 2 +- plugin/othttp/handler_example_test.go | 2 +- sdk/trace/benchmark_test.go | 33 --------------------------- sdk/trace/span.go | 7 ------ sdk/trace/trace_test.go | 12 ++++++---- 16 files changed, 19 insertions(+), 79 deletions(-) diff --git a/api/testharness/harness.go b/api/testharness/harness.go index 1edc3ac0c..d943d0aaf 100644 --- a/api/testharness/harness.go +++ b/api/testharness/harness.go @@ -310,9 +310,6 @@ func (h *Harness) testSpan(tracerFactory func() trace.Tracer) { "#SetName": func(span trace.Span) { span.SetName("new name") }, - "#SetAttribute": func(span trace.Span) { - span.SetAttribute(core.Key("key").String("value")) - }, "#SetAttributes": func(span trace.Span) { span.SetAttributes(core.Key("key1").String("value"), core.Key("key2").Int(123)) }, diff --git a/api/trace/api.go b/api/trace/api.go index 2f2ab8f96..53c2dc550 100644 --- a/api/trace/api.go +++ b/api/trace/api.go @@ -84,7 +84,6 @@ type Span interface { SetName(name string) // Set span attributes - SetAttribute(core.KeyValue) SetAttributes(...core.KeyValue) } diff --git a/api/trace/current_test.go b/api/trace/current_test.go index 9e8bcddde..76be78b03 100644 --- a/api/trace/current_test.go +++ b/api/trace/current_test.go @@ -79,10 +79,6 @@ func (mockSpan) SetName(name string) { func (mockSpan) SetError(v bool) { } -// SetAttribute does nothing. -func (mockSpan) SetAttribute(attribute core.KeyValue) { -} - // SetAttributes does nothing. func (mockSpan) SetAttributes(attributes ...core.KeyValue) { } diff --git a/api/trace/noop_span.go b/api/trace/noop_span.go index 0f616ecbb..59aa4177f 100644 --- a/api/trace/noop_span.go +++ b/api/trace/noop_span.go @@ -46,10 +46,6 @@ func (NoopSpan) SetStatus(status codes.Code) { func (NoopSpan) SetError(v bool) { } -// SetAttribute does nothing. -func (NoopSpan) SetAttribute(attribute core.KeyValue) { -} - // SetAttributes does nothing. func (NoopSpan) SetAttributes(attributes ...core.KeyValue) { } diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go index 034907bfd..992d8f086 100644 --- a/bridge/opentracing/bridge.go +++ b/bridge/opentracing/bridge.go @@ -131,7 +131,7 @@ func (s *bridgeSpan) SetTag(key string, value interface{}) ot.Span { s.otelSpan.SetStatus(status) } default: - s.otelSpan.SetAttribute(otTagToOtelCoreKeyValue(key, value)) + s.otelSpan.SetAttributes(otTagToOtelCoreKeyValue(key, value)) } return s } diff --git a/bridge/opentracing/internal/mock.go b/bridge/opentracing/internal/mock.go index 51fedfbb6..8aa4b71ef 100644 --- a/bridge/opentracing/internal/mock.go +++ b/bridge/opentracing/internal/mock.go @@ -225,21 +225,15 @@ func (s *MockSpan) IsRecording() bool { } func (s *MockSpan) SetStatus(status codes.Code) { - s.SetAttribute(NameKey.Uint32(uint32(status))) + s.SetAttributes(NameKey.Uint32(uint32(status))) } func (s *MockSpan) SetName(name string) { - s.SetAttribute(NameKey.String(name)) + s.SetAttributes(NameKey.String(name)) } func (s *MockSpan) SetError(v bool) { - s.SetAttribute(ErrorKey.Bool(v)) -} - -func (s *MockSpan) SetAttribute(attribute otelcore.KeyValue) { - s.applyUpdate(oteldctx.MapUpdate{ - SingleKV: attribute, - }) + s.SetAttributes(ErrorKey.Bool(v)) } func (s *MockSpan) SetAttributes(attributes ...otelcore.KeyValue) { diff --git a/example/basic/main.go b/example/basic/main.go index e5cad77a2..4d1c57de7 100644 --- a/example/basic/main.go +++ b/example/basic/main.go @@ -126,7 +126,7 @@ func main() { ctx, "Sub operation...", func(ctx context.Context) error { - trace.CurrentSpan(ctx).SetAttribute(lemonsKey.String("five")) + trace.CurrentSpan(ctx).SetAttributes(lemonsKey.String("five")) trace.CurrentSpan(ctx).AddEvent(ctx, "Sub span event") diff --git a/example/namedtracer/foo/foo.go b/example/namedtracer/foo/foo.go index 3dc32a8a1..0d9b5afef 100644 --- a/example/namedtracer/foo/foo.go +++ b/example/namedtracer/foo/foo.go @@ -37,7 +37,7 @@ func SubOperation(ctx context.Context) error { ctx, "Sub operation...", func(ctx context.Context) error { - trace.CurrentSpan(ctx).SetAttribute(lemonsKey.String("five")) + trace.CurrentSpan(ctx).SetAttributes(lemonsKey.String("five")) trace.CurrentSpan(ctx).AddEvent(ctx, "Sub span event") diff --git a/exporter/trace/stackdriver/trace.go b/exporter/trace/stackdriver/trace.go index 114a2bc66..9687d89da 100644 --- a/exporter/trace/stackdriver/trace.go +++ b/exporter/trace/stackdriver/trace.go @@ -83,7 +83,7 @@ func (e *traceExporter) uploadSpans(ctx context.Context, spans []*tracepb.Span) // "go.opentelemetry.io/otel/exporter/stackdriver.uploadSpans", // ) // defer span.End() - // span.SetAttribute(key.New("num_spans").Int64(int64(len(spans)))) + // span.SetAttributes(key.New("num_spans").Int64(int64(len(spans)))) err := e.client.BatchWriteSpans(ctx, &req) if err != nil { diff --git a/internal/trace/mock_span.go b/internal/trace/mock_span.go index 7cc63da0a..48a8fc638 100644 --- a/internal/trace/mock_span.go +++ b/internal/trace/mock_span.go @@ -54,10 +54,6 @@ func (ms *MockSpan) SetStatus(status codes.Code) { func (ms *MockSpan) SetError(v bool) { } -// SetAttribute does nothing. -func (ms *MockSpan) SetAttribute(attribute core.KeyValue) { -} - // SetAttributes does nothing. func (ms *MockSpan) SetAttributes(attributes ...core.KeyValue) { } diff --git a/plugin/httptrace/clienttrace.go b/plugin/httptrace/clienttrace.go index 962e240fe..433b0ef88 100644 --- a/plugin/httptrace/clienttrace.go +++ b/plugin/httptrace/clienttrace.go @@ -99,7 +99,7 @@ func (ct *clientTracer) end(hook string, err error, attrs ...core.KeyValue) { if span, ok := ct.activeHooks[hook]; ok { if err != nil { span.SetStatus(codes.Unknown) - span.SetAttribute(MessageKey.String(err.Error())) + span.SetAttributes(MessageKey.String(err.Error())) } span.SetAttributes(attrs...) span.End() @@ -164,7 +164,7 @@ func (ct *clientTracer) wroteHeaderField(k string, v []string) { if ct.span("http.headers") == nil { ct.start("http.headers", "http.headers") } - ct.root.SetAttribute(key.String("http."+strings.ToLower(k), sliceToString(v))) + ct.root.SetAttributes(key.String("http."+strings.ToLower(k), sliceToString(v))) } func (ct *clientTracer) wroteHeaders() { @@ -173,7 +173,7 @@ func (ct *clientTracer) wroteHeaders() { func (ct *clientTracer) wroteRequest(info httptrace.WroteRequestInfo) { if info.Err != nil { - ct.root.SetAttribute(MessageKey.String(info.Err.Error())) + ct.root.SetAttributes(MessageKey.String(info.Err.Error())) ct.root.SetStatus(codes.Unknown) } ct.end("http.send", info.Err) diff --git a/plugin/othttp/handler.go b/plugin/othttp/handler.go index 544e4e313..5dbba8ec7 100644 --- a/plugin/othttp/handler.go +++ b/plugin/othttp/handler.go @@ -223,7 +223,7 @@ func WithRouteTag(route string, h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { span := trace.CurrentSpan(r.Context()) //TODO: Why doesn't tag.Upsert work? - span.SetAttribute(RouteKey.String(route)) + span.SetAttributes(RouteKey.String(route)) h.ServeHTTP(w, r.WithContext(trace.SetCurrentSpan(r.Context(), span))) }) } diff --git a/plugin/othttp/handler_example_test.go b/plugin/othttp/handler_example_test.go index 6d8981a6a..a6846e262 100644 --- a/plugin/othttp/handler_example_test.go +++ b/plugin/othttp/handler_example_test.go @@ -65,7 +65,7 @@ func ExampleNewHandler() { case "": err = fmt.Errorf("expected /hello/:name in %q", s) default: - trace.CurrentSpan(ctx).SetAttribute(core.Key("name").String(pp[1])) + trace.CurrentSpan(ctx).SetAttributes(core.Key("name").String(pp[1])) } return pp[1], err } diff --git a/sdk/trace/benchmark_test.go b/sdk/trace/benchmark_test.go index 32fb8a701..8f4543e74 100644 --- a/sdk/trace/benchmark_test.go +++ b/sdk/trace/benchmark_test.go @@ -53,39 +53,6 @@ func BenchmarkSpanWithAttributes_4(b *testing.B) { }) } -func BenchmarkSpan_SetAttributes(b *testing.B) { - b.Run("SetAttribute", func(b *testing.B) { - traceBenchmark(b, "Benchmark Start With 4 Attributes", func(b *testing.B, t apitrace.Tracer) { - ctx := context.Background() - b.ResetTimer() - - for i := 0; i < b.N; i++ { - _, span := t.Start(ctx, "/foo") - span.SetAttribute(key.New("key1").Bool(false)) - span.SetAttribute(key.New("key2").String("hello")) - span.SetAttribute(key.New("key3").Uint64(123)) - span.SetAttribute(key.New("key4").Float64(123.456)) - span.End() - } - }) - }) - - b.Run("SetAttributes", func(b *testing.B) { - traceBenchmark(b, "Benchmark Start With 4 Attributes", func(b *testing.B, t apitrace.Tracer) { - ctx := context.Background() - b.ResetTimer() - for i := 0; i < b.N; i++ { - _, span := t.Start(ctx, "/foo") - span.SetAttributes(key.New("key1").Bool(false)) - span.SetAttributes(key.New("key2").String("hello")) - span.SetAttributes(key.New("key3").Uint64(123)) - span.SetAttributes(key.New("key4").Float64(123.456)) - span.End() - } - }) - }) -} - func BenchmarkSpanWithAttributes_8(b *testing.B) { traceBenchmark(b, "Benchmark Start With 8 Attributes", func(b *testing.B, t apitrace.Tracer) { ctx := context.Background() diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 906126366..94efc5963 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -84,13 +84,6 @@ func (s *span) SetStatus(status codes.Code) { s.mu.Unlock() } -func (s *span) SetAttribute(attribute core.KeyValue) { - if !s.IsRecording() { - return - } - s.copyToCappedAttributes(attribute) -} - func (s *span) SetAttributes(attributes ...core.KeyValue) { if !s.IsRecording() { return diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 8b3f68140..75077b90d 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -328,7 +328,7 @@ func TestSetSpanAttributes(t *testing.T) { te := &testExporter{} tp, _ := NewProvider(WithSyncer(te)) span := startSpan(tp, "SpanAttribute") - span.SetAttribute(key.New("key1").String("value1")) + span.SetAttributes(key.New("key1").String("value1")) got, err := endSpan(te, span) if err != nil { t.Fatal(err) @@ -358,10 +358,12 @@ func TestSetSpanAttributesOverLimit(t *testing.T) { tp, _ := NewProvider(WithConfig(cfg), WithSyncer(te)) span := startSpan(tp, "SpanAttributesOverLimit") - span.SetAttribute(key.Bool("key1", true)) - span.SetAttribute(key.String("key2", "value2")) - span.SetAttribute(key.Bool("key1", false)) // Replace key1. - span.SetAttribute(key.Int64("key4", 4)) // Remove key2 and add key4 + span.SetAttributes( + key.Bool("key1", true), + key.String("key2", "value2"), + key.Bool("key1", false), // Replace key1. + key.Int64("key4", 4), // Remove key2 and add key4 + ) got, err := endSpan(te, span) if err != nil { t.Fatal(err)