Repository: open-telemetry/opentelemetry-go
Pull request #2213 (2021-08-30 17:18:34)

Refactor SDK span creation and implementation
The span creation and configuration process is split across the tracer Start method and the startSpanInternal function, each living in different files. This adds confusion when developing. It requires the developer to remember certain parts of the configuration happen in one place or the other. Additionally, when it is determined that a span is not recorded during its creation it still is implemented by a full functioning `Span` implementation. This adds unneeded memory overhead. These changes unify the creation and configuration of a new span and add a new `nonRecordingSpan` type.

Unification of the span creation in a newSpan method on the tracer inherently associates the tracer with the span creation and centralizes all the configuration needed for a new span. This creation path is split in the newSpan method. One for a non-recording and another for a recording span.

# Benchmarks

Benchmarks performed in the `otel/sdk/trace` package.

## Before

```sh
$ go test -bench=.
go.opentelemetry.io/otel/sdk/trace.recordStackTrace(...)
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/trace
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
BenchmarkAttributesMapToKeyValue-8     	  368532	      3135 ns/op
BenchmarkStartEndSpan/AlwaysSample-8   	 1372064	       872.5 ns/op	     848 B/op	       9 allocs/op
BenchmarkStartEndSpan/NeverSample-8    	 1588966	       741.9 ns/op	     752 B/op	       8 allocs/op
BenchmarkSpanWithAttributes_4/AlwaysSample-8         	  714387	      1647 ns/op	    1584 B/op	      17 allocs/op
BenchmarkSpanWithAttributes_4/NeverSample-8          	 1367698	       877.7 ns/op	     944 B/op	       9 allocs/op
BenchmarkSpanWithAttributes_8/AlwaysSample-8         	  551848	      2054 ns/op	    2112 B/op	      23 allocs/op
BenchmarkSpanWithAttributes_8/NeverSample-8          	 1257816	       953.5 ns/op	    1136 B/op	       9 allocs/op
BenchmarkSpanWithAttributes_all/AlwaysSample-8       	  557142	      1965 ns/op	    1936 B/op	      21 allocs/op
BenchmarkSpanWithAttributes_all/NeverSample-8        	 1216846	       931.6 ns/op	    1072 B/op	       9 allocs/op
BenchmarkSpanWithAttributes_all_2x/AlwaysSample-8    	  323812	      3457 ns/op	    3236 B/op	      32 allocs/op
BenchmarkSpanWithAttributes_all_2x/NeverSample-8     	 1000000	      1177 ns/op	    1392 B/op	       9 allocs/op
BenchmarkTraceID_DotString-8                         	14290009	        80.33 ns/op
BenchmarkSpanID_DotString-8                          	19520773	        59.88 ns/op
PASS
```

## After

```sh
$ go test -bench=.
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/trace
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
BenchmarkAttributesMapToKeyValue-8     	  382208	      2932 ns/op
BenchmarkStartEndSpan/AlwaysSample-8   	 1284544	       911.1 ns/op	     848 B/op	       9 allocs/op
BenchmarkStartEndSpan/NeverSample-8    	 3394278	       341.1 ns/op	     224 B/op	       3 allocs/op
BenchmarkSpanWithAttributes_4/AlwaysSample-8         	  690414	      1576 ns/op	    1584 B/op	      17 allocs/op
BenchmarkSpanWithAttributes_4/NeverSample-8          	 2494444	       474.5 ns/op	     416 B/op	       4 allocs/op
BenchmarkSpanWithAttributes_8/AlwaysSample-8         	  495074	      2243 ns/op	    2112 B/op	      23 allocs/op
BenchmarkSpanWithAttributes_8/NeverSample-8          	 2070771	       569.0 ns/op	     608 B/op	       4 allocs/op
BenchmarkSpanWithAttributes_all/AlwaysSample-8       	  550532	      2135 ns/op	    1936 B/op	      21 allocs/op
BenchmarkSpanWithAttributes_all/NeverSample-8        	 2279611	       539.2 ns/op	     544 B/op	       4 allocs/op
BenchmarkSpanWithAttributes_all_2x/AlwaysSample-8    	  334038	      3591 ns/op	    3236 B/op	      32 allocs/op
BenchmarkSpanWithAttributes_all_2x/NeverSample-8     	 1617702	       742.9 ns/op	     864 B/op	       4 allocs/op
BenchmarkTraceID_DotString-8                         	13296470	        80.64 ns/op
BenchmarkSpanID_DotString-8                          	19072332	        62.09 ns/op
PASS
```

codecov[bot]: # [Codecov](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/2213?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) Report
> Merging [#2213](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/2213?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) (e859989) into [main](https://codecov.io/gh/open-telemetry/opentelemetry-go/commit/3b05ba021c0c779283854a01ade4f547436b0b84?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) (3b05ba0) will **decrease** coverage by `0.1%`.
> The diff coverage is `90.1%`.

[![Impacted file tree graph](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/2213/graphs/tree.svg?width=650&height=150&src=pr&token=8efTmh4kvf&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry)](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/2213?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry)

```diff
@@           Coverage Diff           @@
##            main   #2213     +/-   ##
=======================================
- Coverage   72.9%   72.7%   -0.2%     
=======================================
  Files        176     176             
  Lines      12161   12177     +16     
=======================================
- Hits        8870    8862      -8     
- Misses      3053    3072     +19     
- Partials     238     243      +5     
```


| [Impacted Files](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/2213?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) | Coverage Δ | |
|---|---|---|
| [sdk/trace/span.go](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/2213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry#diff-c2RrL3RyYWNlL3NwYW4uZ28=) | `81.1% <75.4%> (-9.0%)` | :arrow_down: |
| [sdk/trace/tracer.go](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/2213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry#diff-c2RrL3RyYWNlL3RyYWNlci5nbw==) | `100.0% <100.0%> (+6.8%)` | :arrow_up: |
| [...s/otlp/otlptrace/internal/connection/connection.go](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/2213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry#diff-ZXhwb3J0ZXJzL290bHAvb3RscHRyYWNlL2ludGVybmFsL2Nvbm5lY3Rpb24vY29ubmVjdGlvbi5nbw==) | `14.8% <0.0%> (-1.6%)` | :arrow_down: |

jmacd: (This is a nice optimization!)

Patch:
diff --git a/sdk/trace/span.go b/sdk/trace/span.go
index 635044f0..cda16444 100644
--- a/sdk/trace/span.go
+++ b/sdk/trace/span.go
@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"reflect"
 	"runtime"
+	rt "runtime/trace"
 	"sync"
 	"time"
 
@@ -98,9 +99,9 @@ type ReadWriteSpan interface {
 	ReadOnlySpan
 }
 
-// span is an implementation of the OpenTelemetry Span API representing the
-// individual component of a trace.
-type span struct {
+// recordingSpan is an implementation of the OpenTelemetry Span API
+// representing the individual component of a trace that is sampled.
+type recordingSpan struct {
 	// mu protects the contents of this span.
 	mu sync.Mutex
 
@@ -157,10 +158,11 @@ type span struct {
 	spanLimits SpanLimits
 }
 
-var _ trace.Span = &span{}
+var _ ReadWriteSpan = (*recordingSpan)(nil)
+var _ runtimeTracer = (*recordingSpan)(nil)
 
 // SpanContext returns the SpanContext of this span.
-func (s *span) SpanContext() trace.SpanContext {
+func (s *recordingSpan) SpanContext() trace.SpanContext {
 	if s == nil {
 		return trace.SpanContext{}
 	}
@@ -169,21 +171,21 @@ func (s *span) SpanContext() trace.SpanContext {
 
 // IsRecording returns if this span is being recorded. If this span has ended
 // this will return false.
-func (s *span) IsRecording() bool {
+func (s *recordingSpan) IsRecording() bool {
 	if s == nil {
 		return false
 	}
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	return !s.startTime.IsZero() && s.endTime.IsZero()
+	return s.endTime.IsZero()
 }
 
 // SetStatus sets the status of the Span in the form of a code and a
 // description, overriding previous values set. The description is only
 // included in the set status when the code is for an error. If this span is
 // not being recorded than this method does nothing.
-func (s *span) SetStatus(code codes.Code, description string) {
+func (s *recordingSpan) SetStatus(code codes.Code, description string) {
 	if !s.IsRecording() {
 		return
 	}
@@ -204,7 +206,7 @@ func (s *span) SetStatus(code codes.Code, description string) {
 // will be overwritten with the value contained in attributes.
 //
 // If this span is not being recorded than this method does nothing.
-func (s *span) SetAttributes(attributes ...attribute.KeyValue) {
+func (s *recordingSpan) SetAttributes(attributes ...attribute.KeyValue) {
 	if !s.IsRecording() {
 		return
 	}
@@ -219,7 +221,7 @@ func (s *span) SetAttributes(attributes ...attribute.KeyValue) {
 //
 // If this method is called while panicking an error event is added to the
 // Span before ending it and the panic is continued.
-func (s *span) End(options ...trace.SpanEndOption) {
+func (s *recordingSpan) End(options ...trace.SpanEndOption) {
 	// Do not start by checking if the span is being recorded which requires
 	// acquiring a lock. Make a minimal check that the span is not nil.
 	if s == nil {
@@ -284,7 +286,7 @@ func (s *span) End(options ...trace.SpanEndOption) {
 // SetStatus is required if the Status of the Span should be set to Error, this method
 // does not change the Span status. If this span is not being recorded or err is nil
 // than this method does nothing.
-func (s *span) RecordError(err error, opts ...trace.EventOption) {
+func (s *recordingSpan) RecordError(err error, opts ...trace.EventOption) {
 	if s == nil || err == nil || !s.IsRecording() {
 		return
 	}
@@ -322,14 +324,14 @@ func recordStackTrace() string {
 
 // AddEvent adds an event with the provided name and options. If this span is
 // not being recorded than this method does nothing.
-func (s *span) AddEvent(name string, o ...trace.EventOption) {
+func (s *recordingSpan) AddEvent(name string, o ...trace.EventOption) {
 	if !s.IsRecording() {
 		return
 	}
 	s.addEvent(name, o...)
 }
 
-func (s *span) addEvent(name string, o ...trace.EventOption) {
+func (s *recordingSpan) addEvent(name string, o ...trace.EventOption) {
 	c := trace.NewEventConfig(o...)
 
 	// Discard over limited attributes
@@ -351,7 +353,7 @@ func (s *span) addEvent(name string, o ...trace.EventOption) {
 
 // SetName sets the name of this span. If this span is not being recorded than
 // this method does nothing.
-func (s *span) SetName(name string) {
+func (s *recordingSpan) SetName(name string) {
 	if !s.IsRecording() {
 		return
 	}
@@ -362,28 +364,28 @@ func (s *span) SetName(name string) {
 }
 
 // Name returns the name of this span.
-func (s *span) Name() string {
+func (s *recordingSpan) Name() string {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	return s.name
 }
 
 // Name returns the SpanContext of this span's parent span.
-func (s *span) Parent() trace.SpanContext {
+func (s *recordingSpan) Parent() trace.SpanContext {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	return s.parent
 }
 
 // SpanKind returns the SpanKind of this span.
-func (s *span) SpanKind() trace.SpanKind {
+func (s *recordingSpan) SpanKind() trace.SpanKind {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	return s.spanKind
 }
 
 // StartTime returns the time this span started.
-func (s *span) StartTime() time.Time {
+func (s *recordingSpan) StartTime() time.Time {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	return s.startTime
@@ -391,14 +393,14 @@ func (s *span) StartTime() time.Time {
 
 // EndTime returns the time this span ended. For spans that have not yet
 // ended, the returned value will be the zero value of time.Time.
-func (s *span) EndTime() time.Time {
+func (s *recordingSpan) EndTime() time.Time {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	return s.endTime
 }
 
 // Attributes returns the attributes of this span.
-func (s *span) Attributes() []attribute.KeyValue {
+func (s *recordingSpan) Attributes() []attribute.KeyValue {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	if s.attributes.evictList.Len() == 0 {
@@ -408,7 +410,7 @@ func (s *span) Attributes() []attribute.KeyValue {
 }
 
 // Links returns the links of this span.
-func (s *span) Links() []Link {
+func (s *recordingSpan) Links() []Link {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	if len(s.links.queue) == 0 {
@@ -418,7 +420,7 @@ func (s *span) Links() []Link {
 }
 
 // Events returns the events of this span.
-func (s *span) Events() []Event {
+func (s *recordingSpan) Events() []Event {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	if len(s.events.queue) == 0 {
@@ -428,7 +430,7 @@ func (s *span) Events() []Event {
 }
 
 // Status returns the status of this span.
-func (s *span) Status() Status {
+func (s *recordingSpan) Status() Status {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	return s.status
@@ -436,7 +438,7 @@ func (s *span) Status() Status {
 
 // InstrumentationLibrary returns the instrumentation.Library associated with
 // the Tracer that created this span.
-func (s *span) InstrumentationLibrary() instrumentation.Library {
+func (s *recordingSpan) InstrumentationLibrary() instrumentation.Library {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	return s.instrumentationLibrary
@@ -444,13 +446,13 @@ func (s *span) InstrumentationLibrary() instrumentation.Library {
 
 // Resource returns the Resource associated with the Tracer that created this
 // span.
-func (s *span) Resource() *resource.Resource {
+func (s *recordingSpan) Resource() *resource.Resource {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	return s.resource
 }
 
-func (s *span) addLink(link trace.Link) {
+func (s *recordingSpan) addLink(link trace.Link) {
 	if !s.IsRecording() {
 		return
 	}
@@ -470,7 +472,7 @@ func (s *span) addLink(link trace.Link) {
 
 // DroppedAttributes returns the number of attributes dropped by the span
 // due to limits being reached.
-func (s *span) DroppedAttributes() int {
+func (s *recordingSpan) DroppedAttributes() int {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	return s.attributes.droppedCount
@@ -478,7 +480,7 @@ func (s *span) DroppedAttributes() int {
 
 // DroppedLinks returns the number of links dropped by the span due to limits
 // being reached.
-func (s *span) DroppedLinks() int {
+func (s *recordingSpan) DroppedLinks() int {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	return s.links.droppedCount
@@ -486,7 +488,7 @@ func (s *span) DroppedLinks() int {
 
 // DroppedEvents returns the number of events dropped by the span due to
 // limits being reached.
-func (s *span) DroppedEvents() int {
+func (s *recordingSpan) DroppedEvents() int {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	return s.events.droppedCount
@@ -494,7 +496,7 @@ func (s *span) DroppedEvents() int {
 
 // ChildSpanCount returns the count of spans that consider the span a
 // direct parent.
-func (s *span) ChildSpanCount() int {
+func (s *recordingSpan) ChildSpanCount() int {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	return s.childSpanCount
@@ -502,12 +504,12 @@ func (s *span) ChildSpanCount() int {
 
 // TracerProvider returns a trace.TracerProvider that can be used to generate
 // additional Spans on the same telemetry pipeline as the current Span.
-func (s *span) TracerProvider() trace.TracerProvider {
+func (s *recordingSpan) TracerProvider() trace.TracerProvider {
 	return s.tracer.provider
 }
 
 // snapshot creates a read-only copy of the current state of the span.
-func (s *span) snapshot() ReadOnlySpan {
+func (s *recordingSpan) snapshot() ReadOnlySpan {
 	var sd snapshot
 	s.mu.Lock()
 	defer s.mu.Unlock()
@@ -538,7 +540,7 @@ func (s *span) snapshot() ReadOnlySpan {
 	return &sd
 }
 
-func (s *span) interfaceArrayToLinksArray() []Link {
+func (s *recordingSpan) interfaceArrayToLinksArray() []Link {
 	linkArr := make([]Link, 0)
 	for _, value := range s.links.queue {
 		linkArr = append(linkArr, value.(Link))
@@ -546,7 +548,7 @@ func (s *span) interfaceArrayToLinksArray() []Link {
 	return linkArr
 }
 
-func (s *span) interfaceArrayToEventArray() []Event {
+func (s *recordingSpan) interfaceArrayToEventArray() []Event {
 	eventArr := make([]Event, 0)
 	for _, value := range s.events.queue {
 		eventArr = append(eventArr, value.(Event))
@@ -554,7 +556,7 @@ func (s *span) interfaceArrayToEventArray() []Event {
 	return eventArr
 }
 
-func (s *span) copyToCappedAttributes(attributes ...attribute.KeyValue) {
+func (s *recordingSpan) copyToCappedAttributes(attributes ...attribute.KeyValue) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	for _, a := range attributes {
@@ -566,7 +568,7 @@ func (s *span) copyToCappedAttributes(attributes ...attribute.KeyValue) {
 	}
 }
 
-func (s *span) addChild() {
+func (s *recordingSpan) addChild() {
 	if !s.IsRecording() {
 		return
 	}
@@ -575,81 +577,65 @@ func (s *span) addChild() {
 	s.mu.Unlock()
 }
 
-func (*span) private() {}
+func (*recordingSpan) private() {}
 
-func startSpanInternal(ctx context.Context, tr *tracer, name string, o *trace.SpanConfig) *span {
-	span := &span{}
+// runtimeTrace starts a "runtime/trace".Task for the span and returns a
+// context containing the task.
+func (s *recordingSpan) runtimeTrace(ctx context.Context) context.Context {
+	if !rt.IsEnabled() {
+		// Avoid additional overhead if runtime/trace is not enabled.
+		return ctx
+	}
+	nctx, task := rt.NewTask(ctx, s.name)
 
-	provider := tr.provider
+	s.mu.Lock()
+	s.executionTracerTaskEnd = task.End
+	s.mu.Unlock()
 
-	// If told explicitly to make this a new root use a zero value SpanContext
-	// as a parent which contains an invalid trace ID and is not remote.
-	var psc trace.SpanContext
-	if o.NewRoot() {
-		ctx = trace.ContextWithSpanContext(ctx, psc)
-	} else {
-		psc = trace.SpanContextFromContext(ctx)
-	}
+	return nctx
+}
 
-	// If there is a valid parent trace ID, use it to ensure the continuity of
-	// the trace. Always generate a new span ID so other components can rely
-	// on a unique span ID, even if the Span is non-recording.
-	var tid trace.TraceID
-	var sid trace.SpanID
-	if !psc.TraceID().IsValid() {
-		tid, sid = provider.idGenerator.NewIDs(ctx)
-	} else {
-		tid = psc.TraceID()
-		sid = provider.idGenerator.NewSpanID(ctx, tid)
-	}
+// nonRecordingSpan is a minimal implementation of the OpenTelemetry Span API
+// that wraps a SpanContext. It performs no operations other than to return
+// the wrapped SpanContext or TracerProvider that created it.
+type nonRecordingSpan struct {
+	// tracer is the SDK tracer that created this span.
+	tracer *tracer
+	sc     trace.SpanContext
+}
 
-	spanLimits := provider.spanLimits
-	span.attributes = newAttributesMap(spanLimits.AttributeCountLimit)
-	span.events = newEvictedQueue(spanLimits.EventCountLimit)
-	span.links = newEvictedQueue(spanLimits.LinkCountLimit)
-	span.spanLimits = spanLimits
-
-	samplingResult := provider.sampler.ShouldSample(SamplingParameters{
-		ParentContext: ctx,
-		TraceID:       tid,
-		Name:          name,
-		Kind:          o.SpanKind(),
-		Attributes:    o.Attributes(),
-		Links:         o.Links(),
-	})
+var _ trace.Span = nonRecordingSpan{}
 
-	scc := trace.SpanContextConfig{
-		TraceID:    tid,
-		SpanID:     sid,
-		TraceState: samplingResult.Tracestate,
-	}
-	if isSampled(samplingResult) {
-		scc.TraceFlags = psc.TraceFlags() | trace.FlagsSampled
-	} else {
-		scc.TraceFlags = psc.TraceFlags() &^ trace.FlagsSampled
-	}
-	span.spanContext = trace.NewSpanContext(scc)
+// SpanContext returns the wrapped SpanContext.
+func (s nonRecordingSpan) SpanContext() trace.SpanContext { return s.sc }
 
-	if !isRecording(samplingResult) {
-		return span
-	}
+// IsRecording always returns false.
+func (nonRecordingSpan) IsRecording() bool { return false }
 
-	startTime := o.Timestamp()
-	if startTime.IsZero() {
-		startTime = time.Now()
-	}
-	span.startTime = startTime
+// SetStatus does nothing.
+func (nonRecordingSpan) SetStatus(codes.Code, string) {}
 
-	span.spanKind = trace.ValidateSpanKind(o.SpanKind())
-	span.name = name
-	span.parent = psc
-	span.resource = provider.resource
-	span.instrumentationLibrary = tr.instrumentationLibrary
+// SetError does nothing.
+func (nonRecordingSpan) SetError(bool) {}
 
-	span.SetAttributes(samplingResult.Attributes...)
+// SetAttributes does nothing.
+func (nonRecordingSpan) SetAttributes(...attribute.KeyValue) {}
 
-	return span
-}
+// End does nothing.
+func (nonRecordingSpan) End(...trace.SpanEndOption) {}
+
+// RecordError does nothing.
+func (nonRecordingSpan) RecordError(error, ...trace.EventOption) {}
+
+// AddEvent does nothing.
+func (nonRecordingSpan) AddEvent(string, ...trace.EventOption) {}
+
+// SetName does nothing.
+func (nonRecordingSpan) SetName(string) {}
+
+// TracerProvider returns the trace.TracerProvider that provided the Tracer
+// that created this span.
+func (s nonRecordingSpan) TracerProvider() trace.TracerProvider { return s.tracer.provider }
 
 func isRecording(s SamplingResult) bool {
 	return s.Decision == RecordOnly || s.Decision == RecordAndSample
diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go
index 621ca825..afd4b5b5 100644
--- a/sdk/trace/tracer.go
+++ b/sdk/trace/tracer.go
@@ -16,7 +16,7 @@ package trace // import "go.opentelemetry.io/otel/sdk/trace"
 
 import (
 	"context"
-	rt "runtime/trace"
+	"time"
 
 	"go.opentelemetry.io/otel/trace"
 
@@ -34,42 +34,120 @@ var _ trace.Tracer = &tracer{}
 //
 // The Span is created with the provided name and as a child of any existing
 // span context found in the passed context. The created Span will be
-// configured appropriately by any SpanOption passed. Any Timestamp option
-// passed will be used as the start time of the Span's life-cycle.
+// configured appropriately by any SpanOption passed.
 func (tr *tracer) Start(ctx context.Context, name string, options ...trace.SpanStartOption) (context.Context, trace.Span) {
 	config := trace.NewSpanStartConfig(options...)
 
 	// For local spans created by this SDK, track child span count.
 	if p := trace.SpanFromContext(ctx); p != nil {
-		if sdkSpan, ok := p.(*span); ok {
+		if sdkSpan, ok := p.(*recordingSpan); ok {
 			sdkSpan.addChild()
 		}
 	}
 
-	span := startSpanInternal(ctx, tr, name, &config)
-	for _, l := range config.Links() {
-		span.addLink(l)
-	}
-	span.SetAttributes(config.Attributes()...)
-
-	span.tracer = tr
-
-	if span.IsRecording() {
+	s := tr.newSpan(ctx, name, &config)
+	if rw, ok := s.(ReadWriteSpan); ok && s.IsRecording() {
 		sps, _ := tr.provider.spanProcessors.Load().(spanProcessorStates)
 		for _, sp := range sps {
-			sp.sp.OnStart(ctx, span)
+			sp.sp.OnStart(ctx, rw)
 		}
 	}
+	if rtt, ok := s.(runtimeTracer); ok {
+		ctx = rtt.runtimeTrace(ctx)
+	}
 
-	ctx, span.executionTracerTaskEnd = func(ctx context.Context) (context.Context, func()) {
-		if !rt.IsEnabled() {
-			// Avoid additional overhead if
-			// runtime/trace is not enabled.
-			return ctx, func() {}
-		}
-		nctx, task := rt.NewTask(ctx, name)
-		return nctx, task.End
-	}(ctx)
+	return trace.ContextWithSpan(ctx, s), s
+}
+
+type runtimeTracer interface {
+	// runtimeTrace starts a "runtime/trace".Task for the span and
+	// returns a context containing the task.
+	runtimeTrace(ctx context.Context) context.Context
+}
+
+// newSpan returns a new configured span.
+func (tr *tracer) newSpan(ctx context.Context, name string, config *trace.SpanConfig) trace.Span {
+	// If told explicitly to make this a new root use a zero value SpanContext
+	// as a parent which contains an invalid trace ID and is not remote.
+	var psc trace.SpanContext
+	if config.NewRoot() {
+		ctx = trace.ContextWithSpanContext(ctx, psc)
+	} else {
+		psc = trace.SpanContextFromContext(ctx)
+	}
+
+	// If there is a valid parent trace ID, use it to ensure the continuity of
+	// the trace. Always generate a new span ID so other components can rely
+	// on a unique span ID, even if the Span is non-recording.
+	var tid trace.TraceID
+	var sid trace.SpanID
+	if !psc.TraceID().IsValid() {
+		tid, sid = tr.provider.idGenerator.NewIDs(ctx)
+	} else {
+		tid = psc.TraceID()
+		sid = tr.provider.idGenerator.NewSpanID(ctx, tid)
+	}
+
+	samplingResult := tr.provider.sampler.ShouldSample(SamplingParameters{
+		ParentContext: ctx,
+		TraceID:       tid,
+		Name:          name,
+		Kind:          config.SpanKind(),
+		Attributes:    config.Attributes(),
+		Links:         config.Links(),
+	})
+
+	scc := trace.SpanContextConfig{
+		TraceID:    tid,
+		SpanID:     sid,
+		TraceState: samplingResult.Tracestate,
+	}
+	if isSampled(samplingResult) {
+		scc.TraceFlags = psc.TraceFlags() | trace.FlagsSampled
+	} else {
+		scc.TraceFlags = psc.TraceFlags() &^ trace.FlagsSampled
+	}
+	sc := trace.NewSpanContext(scc)
+
+	if !isRecording(samplingResult) {
+		return tr.newNonRecordingSpan(sc)
+	}
+	return tr.newRecordingSpan(psc, sc, name, samplingResult, config)
+}
+
+// newRecordingSpan returns a new configured recordingSpan.
+func (tr *tracer) newRecordingSpan(psc, sc trace.SpanContext, name string, sr SamplingResult, config *trace.SpanConfig) *recordingSpan {
+	startTime := config.Timestamp()
+	if startTime.IsZero() {
+		startTime = time.Now()
+	}
+
+	s := &recordingSpan{
+		parent:                 psc,
+		spanContext:            sc,
+		spanKind:               trace.ValidateSpanKind(config.SpanKind()),
+		name:                   name,
+		startTime:              startTime,
+		attributes:             newAttributesMap(tr.provider.spanLimits.AttributeCountLimit),
+		events:                 newEvictedQueue(tr.provider.spanLimits.EventCountLimit),
+		links:                  newEvictedQueue(tr.provider.spanLimits.LinkCountLimit),
+		tracer:                 tr,
+		spanLimits:             tr.provider.spanLimits,
+		resource:               tr.provider.resource,
+		instrumentationLibrary: tr.instrumentationLibrary,
+	}
+
+	for _, l := range config.Links() {
+		s.addLink(l)
+	}
+
+	s.SetAttributes(sr.Attributes...)
+	s.SetAttributes(config.Attributes()...)
+
+	return s
+}
 
-	return trace.ContextWithSpan(ctx, span), span
+// newNonRecordingSpan returns a new configured nonRecordingSpan.
+func (tr *tracer) newNonRecordingSpan(sc trace.SpanContext) nonRecordingSpan {
+	return nonRecordingSpan{tracer: tr, sc: sc}
 }


Test patch:
diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go
index 8dcc7b5f..c4ab8443 100644
--- a/sdk/trace/trace_test.go
+++ b/sdk/trace/trace_test.go
@@ -195,7 +195,7 @@ func TestSetName(t *testing.T) {
 		},
 	} {
 		sp := startNamedSpan(tp, "SetName", tt.name)
-		if sdkspan, ok := sp.(*span); ok {
+		if sdkspan, ok := sp.(*recordingSpan); ok {
 			if sdkspan.Name() != tt.name {
 				t.Errorf("%d: invalid name at span creation, expected %v, got %v", idx, tt.name, sdkspan.Name())
 			}
@@ -203,7 +203,7 @@ func TestSetName(t *testing.T) {
 			t.Errorf("%d: unable to coerce span to SDK span, is type %T", idx, sp)
 		}
 		sp.SetName(tt.newName)
-		if sdkspan, ok := sp.(*span); ok {
+		if sdkspan, ok := sp.(*recordingSpan); ok {
 			if sdkspan.Name() != tt.newName {
 				t.Errorf("%d: span name not changed, expected %v, got %v", idx, tt.newName, sdkspan.Name())
 			}
@@ -806,7 +806,7 @@ func cmpDiff(x, y interface{}) string {
 // checkChild is test utility function that tests that c has fields set appropriately,
 // given that it is a child span of p.
 func checkChild(t *testing.T, p trace.SpanContext, apiSpan trace.Span) error {
-	s := apiSpan.(*span)
+	s := apiSpan.(*recordingSpan)
 	if s == nil {
 		return fmt.Errorf("got nil child span, want non-nil")
 	}
@@ -1016,58 +1016,39 @@ func TestChildSpanCount(t *testing.T) {
 }
 
 func TestNilSpanEnd(t *testing.T) {
-	var span *span
+	var span *recordingSpan
 	span.End()
 }
 
-func TestExecutionTracerTaskEnd(t *testing.T) {
-	var n uint64
+func TestNonRecordingSpanDoesNotTrackRuntimeTracerTask(t *testing.T) {
 	tp := NewTracerProvider(WithSampler(NeverSample()))
-	tr := tp.Tracer("Execution Tracer Task End")
+	tr := tp.Tracer("TestNonRecordingSpanDoesNotTrackRuntimeTracerTask")
+
+	_, apiSpan := tr.Start(context.Background(), "foo")
+	if _, ok := apiSpan.(runtimeTracer); ok {
+		t.Fatalf("non recording span implements runtime trace task tracking")
+	}
+}
+
+func TestRecordingSpanRuntimeTracerTaskEnd(t *testing.T) {
+	tp := NewTracerProvider(WithSampler(AlwaysSample()))
+	tr := tp.Tracer("TestRecordingSpanRuntimeTracerTaskEnd")
 
+	var n uint64
 	executionTracerTaskEnd := func() {
 		atomic.AddUint64(&n, 1)
 	}
-
-	var spans []*span
 	_, apiSpan := tr.Start(context.Background(), "foo")
-	s := apiSpan.(*span)
-
-	s.executionTracerTaskEnd = executionTracerTaskEnd
-	spans = append(spans, s) // never sample
-
-	tID, _ := trace.TraceIDFromHex("0102030405060708090a0b0c0d0e0f")
-	sID, _ := trace.SpanIDFromHex("0001020304050607")
-	ctx := context.Background()
-
-	ctx = trace.ContextWithRemoteSpanContext(ctx,
-		trace.NewSpanContext(trace.SpanContextConfig{
-			TraceID:    tID,
-			SpanID:     sID,
-			TraceFlags: 0,
-		}),
-	)
-	_, apiSpan = tr.Start(
-		ctx,
-		"foo",
-	)
-	s = apiSpan.(*span)
-	s.executionTracerTaskEnd = executionTracerTaskEnd
-	spans = append(spans, s) // parent not sampled
+	s, ok := apiSpan.(*recordingSpan)
+	if !ok {
+		t.Fatal("recording span not returned from always sampled Tracer")
+	}
 
-	tp.sampler = AlwaysSample()
-	_, apiSpan = tr.Start(context.Background(), "foo")
-	s = apiSpan.(*span)
 	s.executionTracerTaskEnd = executionTracerTaskEnd
-	spans = append(spans, s) // always sample
+	s.End()
 
-	for _, span := range spans {
-		span.End()
-	}
-	// Only one span should be sampled meaning only one execution of
-	// executionTracerTaskEnd.
-	if got, want := n, uint64(1); got != want {
-		t.Fatalf("Execution tracer task ended for %v spans; want %v", got, want)
+	if n != 1 {
+		t.Error("recording span did not end runtime trace task")
 	}
 }
 
@@ -1202,8 +1183,8 @@ func TestRecordErrorWithStackTrace(t *testing.T) {
 	assert.Equal(t, got.events[0].Attributes[1].Value.AsString(), want.events[0].Attributes[1].Value.AsString())
 	gotStackTraceFunctionName := strings.Split(got.events[0].Attributes[2].Value.AsString(), "\n")
 
-	assert.True(t, strings.HasPrefix(gotStackTraceFunctionName[1], "go.opentelemetry.io/otel/sdk/trace.recordStackTrace"))
-	assert.True(t, strings.HasPrefix(gotStackTraceFunctionName[3], "go.opentelemetry.io/otel/sdk/trace.(*span).RecordError"))
+	assert.Truef(t, strings.HasPrefix(gotStackTraceFunctionName[1], "go.opentelemetry.io/otel/sdk/trace.recordStackTrace"), "%q not prefixed with go.opentelemetry.io/otel/sdk/trace.recordStackTrace", gotStackTraceFunctionName[1])
+	assert.Truef(t, strings.HasPrefix(gotStackTraceFunctionName[3], "go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).RecordError"), "%q not prefixed with go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).RecordError", gotStackTraceFunctionName[3])
 }
 
 func TestRecordErrorNil(t *testing.T) {
@@ -1434,9 +1415,8 @@ func TestSpanCapturesPanicWithStackTrace(t *testing.T) {
 	assert.Equal(t, spans[0].Events()[0].Attributes[1].Value.AsString(), "error message")
 
 	gotStackTraceFunctionName := strings.Split(spans[0].Events()[0].Attributes[2].Value.AsString(), "\n")
-	fmt.Println(strings.Split(gotStackTraceFunctionName[1], "(0x")[0])
-	assert.True(t, strings.HasPrefix(gotStackTraceFunctionName[1], "go.opentelemetry.io/otel/sdk/trace.recordStackTrace"))
-	assert.True(t, strings.HasPrefix(gotStackTraceFunctionName[3], "go.opentelemetry.io/otel/sdk/trace.(*span).End"))
+	assert.Truef(t, strings.HasPrefix(gotStackTraceFunctionName[1], "go.opentelemetry.io/otel/sdk/trace.recordStackTrace"), "%q not prefixed with go.opentelemetry.io/otel/sdk/trace.recordStackTrace", gotStackTraceFunctionName[1])
+	assert.Truef(t, strings.HasPrefix(gotStackTraceFunctionName[3], "go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End"), "%q not prefixed with go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End", gotStackTraceFunctionName[3])
 }
 
 func TestReadOnlySpan(t *testing.T) {
@@ -1498,9 +1478,9 @@ func TestReadOnlySpan(t *testing.T) {
 
 	// Verify snapshot() returns snapshots that are independent from the
 	// original span and from one another.
-	d1 := s.(*span).snapshot()
+	d1 := s.(*recordingSpan).snapshot()
 	s.AddEvent("baz")
-	d2 := s.(*span).snapshot()
+	d2 := s.(*recordingSpan).snapshot()
 	for _, e := range d1.Events() {
 		if e.Name == "baz" {
 			t.Errorf("Didn't expect to find 'baz' event")