Enable golint & gofmt, resolve issues (open-telemetry#214)

* Add golint to linters and resolve issues.

I decided to remove constructors for some of the propagation types
because the constructors can be reduced to either using the zero value
or a single, non optional member.

* Enable gofmt and commit fixes

.golangci.yml

@@ -7,6 +7,21 @@ linters:
   enable:
     - misspell
     - goimports
+    - golint
+    - gofmt
+
+issues:
+  exclude-rules:
+    # helpers in tests often (rightfully) pass a *testing.T as their first argument
+    - path: _test\.go
+      text: "context.Context should be the first parameter of a function"
+      linters:
+        - golint
+    # Yes, they are, but it's okay in a test
+    - path: _test\.go
+      text: "exported func.*returns unexported type.*which can be annoying to use"
+      linters:
+        - golint
 
 linters-settings:
   misspell:

api/metric/api_test.go

@@ -661,6 +661,7 @@ func TestObserver(t *testing.T) {
 }
 
 func checkBatches(t *testing.T, ctx context.Context, labels LabelSet, meter *mockMeter, descriptor *Descriptor) {
+	t.Helper()
 	if len(meter.measurementBatches) != 3 {
 		t.Errorf("Expected 3 recorded measurement batches, got %d", len(meter.measurementBatches))
 	}

api/trace/always_sampler.go

@@ -47,6 +47,6 @@ func (as alwaysSampleSampler) Description() string {
 
 var _ Sampler = alwaysSampleSampler{}
 
-func AlwaysSampleSampler() alwaysSampleSampler {
+func AlwaysSampleSampler() Sampler {
 	return alwaysSampleSampler{}
 }

api/trace/never_sampler.go

@@ -47,6 +47,6 @@ func (ns neverSampleSampler) Description() string {
 
 var _ Sampler = neverSampleSampler{}
 
-func NeverSampleSampler() neverSampleSampler {
+func NeverSampleSampler() Sampler {
 	return neverSampleSampler{}
 }

experimental/bridge/opentracing/mix_test.go

@@ -250,15 +250,15 @@ type coin3Value struct{}
 func newContextIntactTest() *contextIntactTest {
 	return &contextIntactTest{
 		contextKeyValues: []internal.MockContextKeyValue{
-			internal.MockContextKeyValue{
+			{
 				Key:   coin1Key{},
 				Value: coin1Value{},
 			},
-			internal.MockContextKeyValue{
+			{
 				Key:   coin2Key{},
 				Value: coin2Value{},
 			},
-			internal.MockContextKeyValue{
+			{
 				Key:   coin3Key{},
 				Value: coin3Value{},
 			},

experimental/streaming/exporter/exporter.go

@@ -51,6 +51,7 @@ type Observer interface {
 }
 
 //go:generate stringer -type=EventType
+// nolint:golint
 const (
 	INVALID EventType = iota
 	START_SPAN

experimental/streaming/sdk/span_test.go

@@ -153,8 +153,8 @@ func measurementCompare(m1, m2 metric.Measurement) bool {
 
 func diffEvents(t *testing.T, got, want []exporter.Event, extraIgnoredFields ...string) bool {
 	ignoredPaths := map[string]struct{}{
-		"Sequence": struct{}{},
-		"Context":  struct{}{},
+		"Sequence": {},
+		"Context":  {},
 	}
 	for _, field := range extraIgnoredFields {
 		ignoredPaths[field] = struct{}{}

experimental/streaming/sdk/trace.go

@@ -24,8 +24,8 @@ import (
 	"go.opentelemetry.io/experimental/streaming/exporter"
 )
 
+// TODO These should move somewhere in the api, right?
 var (
-	// TODO These should move somewhere in the api, right?
 	ErrorKey   = key.New("error")
 	SpanIDKey  = key.New("span_id")
 	TraceIDKey = key.New("trace_id")

exporter/trace/jaeger/jaeger_test.go

@@ -61,7 +61,7 @@ func Test_spanDataToThrift(t *testing.T) {
 				StartTime: now,
 				EndTime:   now,
 				Links: []apitrace.Link{
-					apitrace.Link{
+					{
 						SpanContext: core.SpanContext{
 							TraceID: linkTraceID,
 							SpanID:  linkSpanID,
@@ -96,7 +96,7 @@ func Test_spanDataToThrift(t *testing.T) {
 					{Key: "status.message", VType: gen.TagType_STRING, VStr: &statusMessage},
 				},
 				References: []*gen.SpanRef{
-					&gen.SpanRef{
+					{
 						RefType:     gen.SpanRefType_CHILD_OF,
 						TraceIdLow:  int64(linkTraceID.Low),
 						TraceIdHigh: int64(linkTraceID.High),

exporter/trace/jaeger/uploader.go

@@ -25,7 +25,7 @@ type EndpointOption func() (batchUploader, error)
 func WithAgentEndpoint(agentEndpoint string) func() (batchUploader, error) {
 	return func() (batchUploader, error) {
 		if agentEndpoint == "" {
-			return nil, errors.New("agentEndpoint must not be empty.")
+			return nil, errors.New("agentEndpoint must not be empty")
 		}
 
 		client, err := newAgentClientUDP(agentEndpoint, udpPacketMaxLength)
@@ -42,7 +42,7 @@ func WithAgentEndpoint(agentEndpoint string) func() (batchUploader, error) {
 func WithCollectorEndpoint(collectorEndpoint string, options ...CollectorEndpointOption) func() (batchUploader, error) {
 	return func() (batchUploader, error) {
 		if collectorEndpoint == "" {
-			return nil, errors.New("collectorEndpoint must not be empty.")
+			return nil, errors.New("collectorEndpoint must not be empty")
 		}
 
 		o := &CollectorEndpointOptions{}

internal/trace/mock_tracer.go

@@ -30,9 +30,9 @@ type MockTracer struct {
 	// Sampled specifies if the new span should be sampled or not.
 	Sampled bool
 
-	// StartSpanId is used to initialize spanId. It is incremented by one
+	// StartSpanID is used to initialize spanId. It is incremented by one
 	// every time a new span is created.
-	StartSpanId *uint64
+	StartSpanID *uint64
 }
 
 var _ apitrace.Tracer = (*MockTracer)(nil)
@@ -81,7 +81,7 @@ func (mt *MockTracer) Start(ctx context.Context, name string, o ...apitrace.Span
 	} else {
 		sc = opts.Reference.SpanContext
 	}
-	sc.SpanID = atomic.AddUint64(mt.StartSpanId, 1)
+	sc.SpanID = atomic.AddUint64(mt.StartSpanID, 1)
 	span = &MockSpan{
 		sc:     sc,
 		tracer: mt,

plugin/httptrace/httptrace.go

@@ -31,7 +31,7 @@ var (
 	HostKey = key.New("http.host")
 	URLKey  = key.New("http.url")
 
-	propagator = propagation.HttpTraceContextPropagator()
+	propagator = propagation.HTTPTraceContextPropagator{}
 )
 
 // Returns the Attributes, Context Entries, and SpanContext that were encoded by Inject.

plugin/othttp/server.go

@@ -117,7 +117,7 @@ func NewHandler(handler http.Handler, operation string, opts ...HandlerOption) h
 	h := HTTPHandler{handler: handler}
 	defaultOpts := []HandlerOption{
 		WithTracer(trace.GlobalTracer()),
-		WithPropagator(prop.HttpTraceContextPropagator()),
+		WithPropagator(prop.HTTPTraceContextPropagator{}),
 	}
 
 	for _, opt := range append(defaultOpts, opts...) {

plugin/othttp/server_test.go

@@ -29,7 +29,7 @@ func TestBasics(t *testing.T) {
 	rr := httptest.NewRecorder()
 
 	var id uint64
-	tracer := mocktrace.MockTracer{StartSpanId: &id}
+	tracer := mocktrace.MockTracer{StartSpanID: &id}
 
 	h := NewHandler(
 		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

propagation/binary_propagator.go

@@ -27,7 +27,7 @@ var _ apipropagation.BinaryFormatPropagator = binaryPropagator{}
 
 // BinaryPropagator creates a new propagator. The propagator implements
 // ToBytes and FromBytes method to transform SpanContext to/from byte array.
-func BinaryPropagator() binaryPropagator {
+func BinaryPropagator() apipropagation.BinaryFormatPropagator {
 	return binaryPropagator{}
 }
 

propagation/http_b3_propagator.go

@@ -36,16 +36,7 @@ const (
 	B3ParentSpanIDHeader = "X-B3-ParentSpanId"
 )
 
-type httpB3Propagator struct {
-	singleHeader bool
-}
-
-var _ apipropagation.TextFormatPropagator = httpB3Propagator{}
-
-var hexStr32ByteRegex = regexp.MustCompile("^[a-f0-9]{32}$")
-var hexStr16ByteRegex = regexp.MustCompile("^[a-f0-9]{16}$")
-
-// HttpB3Propagator creates a new text format propagator that facilitates core.SpanContext
+// HTTPB3Propagator that facilitates core.SpanContext
 // propagation using B3 Headers.
 // This propagator supports both version of B3 headers,
 //  1. Single Header :
@@ -57,18 +48,21 @@ var hexStr16ByteRegex = regexp.MustCompile("^[a-f0-9]{16}$")
 //    X-B3-Sampled: {SamplingState}
 //    X-B3-Flags: {DebugFlag}
 //
-// If singleHeader is set to true then X-B3 header is used to inject and extract. Otherwise,
+// If SingleHeader is set to true then X-B3 header is used to inject and extract. Otherwise,
 // separate headers are used to inject and extract.
-func HttpB3Propagator(singleHeader bool) httpB3Propagator {
-	// [TODO](rghetia): should it automatically look for both versions? which one to pick if
-	// both are present? What if one is valid and other one is not.
-	return httpB3Propagator{singleHeader}
+type HTTPB3Propagator struct {
+	SingleHeader bool
 }
 
-func (b3 httpB3Propagator) Inject(ctx context.Context, supplier apipropagation.Supplier) {
+var _ apipropagation.TextFormatPropagator = HTTPB3Propagator{}
+
+var hexStr32ByteRegex = regexp.MustCompile("^[a-f0-9]{32}$")
+var hexStr16ByteRegex = regexp.MustCompile("^[a-f0-9]{16}$")
+
+func (b3 HTTPB3Propagator) Inject(ctx context.Context, supplier apipropagation.Supplier) {
 	sc := trace.CurrentSpan(ctx).SpanContext()
 	if sc.IsValid() {
-		if b3.singleHeader {
+		if b3.SingleHeader {
 			sampled := sc.TraceFlags & core.TraceFlagsSampled
 			supplier.Set(B3SingleHeader,
 				fmt.Sprintf("%.16x%.16x-%.16x-%.1d", sc.TraceID.High, sc.TraceID.Low, sc.SpanID, sampled))
@@ -90,21 +84,21 @@ func (b3 httpB3Propagator) Inject(ctx context.Context, supplier apipropagation.S
 }
 
 // Extract retrieves B3 Headers from the supplier
-func (b3 httpB3Propagator) Extract(ctx context.Context, supplier apipropagation.Supplier) core.SpanContext {
-	if b3.singleHeader {
+func (b3 HTTPB3Propagator) Extract(ctx context.Context, supplier apipropagation.Supplier) core.SpanContext {
+	if b3.SingleHeader {
 		return b3.extractSingleHeader(supplier)
 	}
 	return b3.extract(supplier)
 }
 
-func (b3 httpB3Propagator) GetAllKeys() []string {
-	if b3.singleHeader {
+func (b3 HTTPB3Propagator) GetAllKeys() []string {
+	if b3.SingleHeader {
 		return []string{B3SingleHeader}
 	}
 	return []string{B3TraceIDHeader, B3SpanIDHeader, B3SampledHeader}
 }
 
-func (b3 httpB3Propagator) extract(supplier apipropagation.Supplier) core.SpanContext {
+func (b3 HTTPB3Propagator) extract(supplier apipropagation.Supplier) core.SpanContext {
 	tid, ok := b3.extractTraceID(supplier.Get(B3TraceIDHeader))
 	if !ok {
 		return core.EmptySpanContext()
@@ -139,7 +133,7 @@ func (b3 httpB3Propagator) extract(supplier apipropagation.Supplier) core.SpanCo
 	return sc
 }
 
-func (b3 httpB3Propagator) extractSingleHeader(supplier apipropagation.Supplier) core.SpanContext {
+func (b3 HTTPB3Propagator) extractSingleHeader(supplier apipropagation.Supplier) core.SpanContext {
 	h := supplier.Get(B3SingleHeader)
 	if h == "" || h == "0" {
 		core.EmptySpanContext()
@@ -153,30 +147,30 @@ func (b3 httpB3Propagator) extractSingleHeader(supplier apipropagation.Supplier)
 
 	if l < 2 {
 		return core.EmptySpanContext()
-	} else {
-		var ok bool
-		sc.TraceID, ok = b3.extractTraceID(parts[0])
+	}
+
+	var ok bool
+	sc.TraceID, ok = b3.extractTraceID(parts[0])
+	if !ok {
+		return core.EmptySpanContext()
+	}
+
+	sc.SpanID, ok = b3.extractSpanID(parts[1])
+	if !ok {
+		return core.EmptySpanContext()
+	}
+
+	if l > 2 {
+		sc.TraceFlags, ok = b3.extractSampledState(parts[2])
 		if !ok {
 			return core.EmptySpanContext()
 		}
-
-		sc.SpanID, ok = b3.extractSpanID(parts[1])
+	}
+	if l == 4 {
+		_, ok = b3.extractSpanID(parts[3])
 		if !ok {
 			return core.EmptySpanContext()
 		}
-
-		if l > 2 {
-			sc.TraceFlags, ok = b3.extractSampledState(parts[2])
-			if !ok {
-				return core.EmptySpanContext()
-			}
-		}
-		if l == 4 {
-			_, ok = b3.extractSpanID(parts[3])
-			if !ok {
-				return core.EmptySpanContext()
-			}
-		}
 	}
 
 	if !sc.IsValid() {
@@ -187,20 +181,20 @@ func (b3 httpB3Propagator) extractSingleHeader(supplier apipropagation.Supplier)
 }
 
 // extractTraceID parses the value of the X-B3-TraceId b3Header.
-func (b3 httpB3Propagator) extractTraceID(tid string) (traceID core.TraceID, ok bool) {
+func (b3 HTTPB3Propagator) extractTraceID(tid string) (traceID core.TraceID, ok bool) {
 	if hexStr32ByteRegex.MatchString(tid) {
 		traceID.High, _ = strconv.ParseUint(tid[0:(16)], 16, 64)
 		traceID.Low, _ = strconv.ParseUint(tid[(16):32], 16, 64)
 		ok = true
-	} else if b3.singleHeader && hexStr16ByteRegex.MatchString(tid) {
+	} else if b3.SingleHeader && hexStr16ByteRegex.MatchString(tid) {
 		traceID.Low, _ = strconv.ParseUint(tid[:16], 16, 64)
 		ok = true
 	}
 	return traceID, ok
 }
 
 // extractSpanID parses the value of the X-B3-SpanId or X-B3-ParentSpanId headers.
-func (b3 httpB3Propagator) extractSpanID(sid string) (spanID uint64, ok bool) {
+func (b3 HTTPB3Propagator) extractSpanID(sid string) (spanID uint64, ok bool) {
 	if hexStr16ByteRegex.MatchString(sid) {
 		spanID, _ = strconv.ParseUint(sid, 16, 64)
 		ok = true
@@ -209,26 +203,26 @@ func (b3 httpB3Propagator) extractSpanID(sid string) (spanID uint64, ok bool) {
 }
 
 // extractSampledState parses the value of the X-B3-Sampled b3Header.
-func (b3 httpB3Propagator) extractSampledState(sampled string) (flag byte, ok bool) {
+func (b3 HTTPB3Propagator) extractSampledState(sampled string) (flag byte, ok bool) {
 	switch sampled {
 	case "", "0":
 		return 0, true
 	case "1":
 		return core.TraceFlagsSampled, true
 	case "true":
-		if !b3.singleHeader {
+		if !b3.SingleHeader {
 			return core.TraceFlagsSampled, true
 		}
 	case "d":
-		if b3.singleHeader {
+		if b3.SingleHeader {
 			return core.TraceFlagsSampled, true
 		}
 	}
 	return 0, false
 }
 
 // extracDebugFlag parses the value of the X-B3-Sampled b3Header.
-func (b3 httpB3Propagator) extracDebugFlag(debug string) (flag byte, ok bool) {
+func (b3 HTTPB3Propagator) extracDebugFlag(debug string) (flag byte, ok bool) {
 	switch debug {
 	case "", "0":
 		return 0, true

propagation/http_b3_propagator_benchmark_test.go

@@ -54,7 +54,7 @@ func BenchmarkExtractB3(b *testing.B) {
 	trace.SetGlobalTracer(&mocktrace.MockTracer{})
 
 	for _, tg := range testGroup {
-		propagator := propagation.HttpB3Propagator(tg.singleHeader)
+		propagator := propagation.HTTPB3Propagator{tg.singleHeader}
 		for _, tt := range tg.tests {
 			traceBenchmark(tg.name+"/"+tt.name, b, func(b *testing.B) {
 				ctx := context.Background()
@@ -93,13 +93,13 @@ func BenchmarkInjectB3(b *testing.B) {
 
 	mockTracer := &mocktrace.MockTracer{
 		Sampled:     false,
-		StartSpanId: &id,
+		StartSpanID: &id,
 	}
 	trace.SetGlobalTracer(mockTracer)
 
 	for _, tg := range testGroup {
 		id = 0
-		propagator := propagation.HttpB3Propagator(tg.singleHeader)
+		propagator := propagation.HTTPB3Propagator{tg.singleHeader}
 		for _, tt := range tg.tests {
 			traceBenchmark(tg.name+"/"+tt.name, b, func(b *testing.B) {
 				req, _ := http.NewRequest("GET", "http://example.com", nil)