From d25fae7c103af230586892bedf2528796ccc037b Mon Sep 17 00:00:00 2001
From: Liz Fong-Jones <lizf@honeycomb.io>
Date: Mon, 4 Nov 2019 13:04:38 -0600
Subject: [PATCH] set attributes in tracer.Start() (#286)

* set attributes in tracer.Start()

* add tests

* remove no longer needed attribute additions
---
 plugin/httptrace/clienttrace.go |  4 ----
 sdk/trace/trace_test.go         | 37 ++++++++++++++++++++++++++++-----
 sdk/trace/tracer.go             |  1 +
 3 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/plugin/httptrace/clienttrace.go b/plugin/httptrace/clienttrace.go
index b1415c1f0..a6a32becb 100644
--- a/plugin/httptrace/clienttrace.go
+++ b/plugin/httptrace/clienttrace.go
@@ -79,10 +79,6 @@ func NewClientTrace(ctx context.Context) *httptrace.ClientTrace {
 
 func (ct *clientTracer) start(hook, spanName string, attrs ...core.KeyValue) {
 	_, sp := ct.tr.Start(ct.Context, spanName, trace.WithAttributes(attrs...), trace.WithSpanKind(trace.SpanKindClient))
-	// TODO(paivagustavo): remove this for loop when `trace.WithAttributes(attrs...)` works.
-	for _, attr := range attrs {
-		sp.SetAttribute(attr)
-	}
 	ct.mtx.Lock()
 	defer ct.mtx.Unlock()
 	if ct.root == nil {
diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go
index 916314dc8..a66b6cf9a 100644
--- a/sdk/trace/trace_test.go
+++ b/sdk/trace/trace_test.go
@@ -287,6 +287,33 @@ func TestStartSpanWithFollowsFrom(t *testing.T) {
 
 // TODO: [rghetia] Equivalent of SpanKind Test.
 
+func TestSetSpanAttributesOnStart(t *testing.T) {
+	te := &testExporter{}
+	tp, _ := NewProvider(WithSyncer(te))
+	span := startSpan(tp, "StartSpanAttribute", apitrace.WithAttributes(key.String("key1", "value1")))
+	got, err := endSpan(te, span)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	want := &export.SpanData{
+		SpanContext: core.SpanContext{
+			TraceID:    tid,
+			TraceFlags: 0x1,
+		},
+		ParentSpanID: sid,
+		Name:         "StartSpanAttribute/span0",
+		Attributes: []core.KeyValue{
+			key.String("key1", "value1"),
+		},
+		SpanKind:        "internal",
+		HasRemoteParent: true,
+	}
+	if diff := cmpDiff(got, want); diff != "" {
+		t.Errorf("SetSpanAttributesOnStart: -got +want %s", diff)
+	}
+}
+
 func TestSetSpanAttributes(t *testing.T) {
 	te := &testExporter{}
 	tp, _ := NewProvider(WithSyncer(te))
@@ -655,20 +682,20 @@ func checkChild(p core.SpanContext, apiSpan apitrace.Span) error {
 
 // startSpan starts a span with a name "span0". See startNamedSpan for
 // details.
-func startSpan(tp *Provider, trName string) apitrace.Span {
-	return startNamedSpan(tp, trName, "span0")
+func startSpan(tp *Provider, trName string, args ...apitrace.SpanOption) apitrace.Span {
+	return startNamedSpan(tp, trName, "span0", args...)
 }
 
 // startNamed Span is a test utility func that starts a span with a
 // passed name and with ChildOf option.  remote span context contains
 // TraceFlags with sampled bit set. This allows the span to be
 // automatically sampled.
-func startNamedSpan(tp *Provider, trName, name string) apitrace.Span {
+func startNamedSpan(tp *Provider, trName, name string, args ...apitrace.SpanOption) apitrace.Span {
+	args = append(args, apitrace.ChildOf(remoteSpanContext()), apitrace.WithRecord())
 	_, span := tp.GetTracer(trName).Start(
 		context.Background(),
 		name,
-		apitrace.ChildOf(remoteSpanContext()),
-		apitrace.WithRecord(),
+		args...,
 	)
 	return span
 }
diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go
index 03ceaf5d1..539cc20c1 100644
--- a/sdk/trace/tracer.go
+++ b/sdk/trace/tracer.go
@@ -63,6 +63,7 @@ func (tr *tracer) Start(ctx context.Context, name string, o ...apitrace.SpanOpti
 	for _, l := range opts.Links {
 		span.AddLink(l)
 	}
+	span.SetAttributes(opts.Attributes...)
 
 	span.tracer = tr