Adding locks where context is accessed

This commit addresses data race issues where span.context is accessed without locks in methods which can be called concurrently with method setBaggageItem which modifies context

Per issue jaegertracing#526

Signed-off-by: Keerthana Selvakumar <keerukeerthana8@gmail.com>

span.go

@@ -85,11 +85,11 @@ func NewTag(key string, value interface{}) Tag {
 func (s *Span) SetOperationName(operationName string) opentracing.Span {
 	s.Lock()
 	s.operationName = operationName
-	s.Unlock()
 	if !s.isSamplingFinalized() {
 		decision := s.tracer.sampler.OnSetOperationName(s, operationName)
-		s.applySamplingDecision(decision, true)
+		s.applySamplingDecision(decision)
 	}
+	s.Unlock()
 	s.observer.OnSetOperationName(operationName)
 	return s
 }
@@ -101,18 +101,16 @@ func (s *Span) SetTag(key string, value interface{}) opentracing.Span {
 
 func (s *Span) setTagInternal(key string, value interface{}, lock bool) opentracing.Span {
 	s.observer.OnSetTag(key, value)
+	s.Lock()
+	defer s.Unlock()
 	if key == string(ext.SamplingPriority) && !setSamplingPriority(s, value) {
 		return s
 	}
 	if !s.isSamplingFinalized() {
 		decision := s.tracer.sampler.OnSetTag(s, key, value)
-		s.applySamplingDecision(decision, lock)
+		s.applySamplingDecision(decision)
 	}
 	if s.isWriteable() {
-		if lock {
-			s.Lock()
-			defer s.Unlock()
-		}
 		s.appendTagNoLocking(key, value)
 	}
 	return s
@@ -340,13 +338,11 @@ func (s *Span) FinishWithOptions(options opentracing.FinishOptions) {
 	s.observer.OnFinish(options)
 	s.Lock()
 	s.duration = options.FinishTime.Sub(s.startTime)
-	s.Unlock()
 	if !s.isSamplingFinalized() {
 		decision := s.tracer.sampler.OnFinishSpan(s)
-		s.applySamplingDecision(decision, true)
+		s.applySamplingDecision(decision)
 	}
 	if s.context.IsSampled() {
-		s.Lock()
 		s.fixLogsIfDropped()
 		if len(options.LogRecords) > 0 || len(options.BulkLogData) > 0 {
 			// Note: bulk logs are not subject to maxLogsPerSpan limit
@@ -357,8 +353,8 @@ func (s *Span) FinishWithOptions(options opentracing.FinishOptions) {
 				s.logs = append(s.logs, ld.ToLogRecord())
 			}
 		}
-		s.Unlock()
 	}
+	s.Unlock()
 	// call reportSpan even for non-sampled traces, to return span to the pool
 	// and update metrics counter
 	s.tracer.reportSpan(s)
@@ -425,17 +421,13 @@ func (s *Span) serviceName() string {
 	return s.tracer.serviceName
 }
 
-func (s *Span) applySamplingDecision(decision SamplingDecision, lock bool) {
+func (s *Span) applySamplingDecision(decision SamplingDecision) {
 	if !decision.Retryable {
 		s.context.samplingState.setFinal()
 	}
 	if decision.Sample {
 		s.context.samplingState.setSampled()
 		if len(decision.Tags) > 0 {
-			if lock {
-				s.Lock()
-				defer s.Unlock()
-			}
 			for _, tag := range decision.Tags {
 				s.appendTagNoLocking(tag.key, tag.value)
 			}

span_test.go

@@ -363,7 +363,7 @@ func TestSpan_References(t *testing.T) {
 }
 
 func TestSpanContextRaces(t *testing.T) {
-	t.Skip("Skipped: test will panic with -race, see https://github.com/jaegertracing/jaeger-client-go/issues/526")
+	//t.Skip("Skipped: test will panic with -race, see https://github.com/jaegertracing/jaeger-client-go/issues/526")
 	tracer, closer := NewTracer("test", NewConstSampler(true), NewNullReporter())
 	defer closer.Close()
 

tracer.go

@@ -322,7 +322,7 @@ func (t *Tracer) startSpanWithOptions(
 
 	if !sp.isSamplingFinalized() {
 		decision := t.sampler.OnCreateSpan(sp)
-		sp.applySamplingDecision(decision, false)
+		sp.applySamplingDecision(decision)
 	}
 	sp.observer = t.observer.OnStartSpan(sp, operationName, options)