Enable golint & gofmt, resolve issues (#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)