Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add static check and fix all errors #218

Merged
merged 4 commits into from
Aug 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ GOOS=$(shell go env GOOS)
ADDLICENCESE= addlicense
MISSPELL=misspell -error
MISSPELL_CORRECTION=misspell -w
STATICCHECK=staticcheck

GIT_SHA=$(shell git rev-parse --short HEAD)
BUILD_INFO_IMPORT_PATH=github.com/open-telemetry/opentelemetry-service/internal/version
Expand All @@ -38,10 +39,10 @@ all-pkgs:
all-srcs:
@echo $(ALL_SRC) | tr ' ' '\n' | sort

.DEFAULT_GOAL := addlicense-fmt-vet-lint-goimports-misspell-test
.DEFAULT_GOAL := addlicense-fmt-vet-lint-goimports-misspell-staticcheck-test

.PHONY: addlicense-fmt-vet-lint-goimports-misspell-test
addlicense-fmt-vet-lint-goimports-misspell-test: addlicense fmt vet lint goimports misspell test
.PHONY: addlicense-fmt-vet-lint-goimports-misspell-staticcheck-test
addlicense-fmt-vet-lint-goimports-misspell-staticcheck-test: addlicense fmt vet lint goimports misspell staticcheck test

.PHONY: e2e-test
e2e-test: otelsvc
Expand All @@ -52,7 +53,7 @@ test:
$(GOTEST) $(GOTEST_OPT) $(ALL_PKGS)

.PHONY: travis-ci
travis-ci: fmt vet lint goimports misspell test-with-cover otelsvc
travis-ci: fmt vet lint goimports misspell staticcheck test-with-cover otelsvc
$(MAKE) -C testbed install-tools
$(MAKE) -C testbed runtests

Expand Down Expand Up @@ -117,6 +118,10 @@ misspell:
misspell-correction:
$(MISSPELL_CORRECTION) $(ALL_SRC_AND_DOC)

.PHONY: staticcheck
staticcheck:
$(STATICCHECK) ./...

.PHONY: vet
vet:
@$(GOVET) ./...
Expand All @@ -128,7 +133,8 @@ install-tools:
github.com/google/addlicense \
golang.org/x/lint/golint \
golang.org/x/tools/cmd/goimports \
github.com/client9/misspell/cmd/misspell
github.com/client9/misspell/cmd/misspell \
honnef.co/go/tools/cmd/staticcheck

.PHONY: otelsvc
otelsvc:
Expand Down
8 changes: 4 additions & 4 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func loadReceivers(v *viper.Viper, factories map[string]receiver.Factory) (confi
keyMap := v.GetStringMap(receiversKeyName)

// Prepare resulting map
receivers := make(configmodels.Receivers, 0)
receivers := make(configmodels.Receivers)

// Iterate over input map and create a config for each.
for key := range keyMap {
Expand Down Expand Up @@ -242,7 +242,7 @@ func loadExporters(v *viper.Viper, factories map[string]exporter.Factory) (confi
keyMap := v.GetStringMap(exportersKeyName)

// Prepare resulting map
exporters := make(configmodels.Exporters, 0)
exporters := make(configmodels.Exporters)

// Iterate over exporters and create a config for each.
for key := range keyMap {
Expand Down Expand Up @@ -299,7 +299,7 @@ func loadProcessors(v *viper.Viper, factories map[string]processor.Factory) (con
keyMap := v.GetStringMap(processorsKeyName)

// Prepare resulting map.
processors := make(configmodels.Processors, 0)
processors := make(configmodels.Processors)

// Iterate over processors and create a config for each.
for key := range keyMap {
Expand Down Expand Up @@ -356,7 +356,7 @@ func loadPipelines(v *viper.Viper) (configmodels.Pipelines, error) {
keyMap := v.GetStringMap(pipelinesKeyName)

// Prepare resulting map.
pipelines := make(configmodels.Pipelines, 0)
pipelines := make(configmodels.Pipelines)

// Iterate over input map and create a config for each.
for key := range keyMap {
Expand Down
2 changes: 1 addition & 1 deletion exporter/exporterhelper/metricshelper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func newPushMetricsData(droppedSpans int, retError error) PushMetricsData {
}

func generateMetricsTraffic(t *testing.T, te exporter.MetricsExporter, numRequests int, wantError error) {
td := consumerdata.MetricsData{Metrics: make([]*metricspb.Metric, 1, 1)}
td := consumerdata.MetricsData{Metrics: make([]*metricspb.Metric, 1)}
ctx, span := trace.StartSpan(context.Background(), fakeParentSpanName, trace.WithSampler(trace.AlwaysSample()))
defer span.End()
for i := 0; i < numRequests; i++ {
Expand Down
4 changes: 2 additions & 2 deletions exporter/exporterhelper/tracehelper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func checkRecordedMetricsForTraceExporter(t *testing.T, te exporter.TraceExporte
doneFn := observabilitytest.SetupRecordedMetricsTest()
defer doneFn()

spans := make([]*tracepb.Span, 2, 2)
spans := make([]*tracepb.Span, 2)
td := consumerdata.TraceData{Spans: spans}
ctx := observability.ContextWithReceiverName(context.Background(), fakeReceiverName)
const numBatches = 7
Expand All @@ -151,7 +151,7 @@ func checkRecordedMetricsForTraceExporter(t *testing.T, te exporter.TraceExporte
}

func generateTraceTraffic(t *testing.T, te exporter.TraceExporter, numRequests int, wantError error) {
td := consumerdata.TraceData{Spans: make([]*tracepb.Span, 1, 1)}
td := consumerdata.TraceData{Spans: make([]*tracepb.Span, 1)}
ctx, span := trace.StartSpan(context.Background(), fakeParentSpanName, trace.WithSampler(trace.AlwaysSample()))
defer span.End()
for i := 0; i < numRequests; i++ {
Expand Down
8 changes: 4 additions & 4 deletions exporter/exportertest/nop_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestNopTraceExporter_NoErrors(t *testing.T) {
if err := nte.ConsumeTraceData(context.Background(), td); err != nil {
t.Fatalf("Wanted nil got error")
}
if "nop_trace" != nte.TraceExportFormat() {
if nte.TraceExportFormat() != "nop_trace" {
t.Fatalf("Wanted nop_trace got %s", nte.TraceExportFormat())
}
}
Expand All @@ -45,7 +45,7 @@ func TestNopTraceExporter_WithErrors(t *testing.T) {
if got := nte.ConsumeTraceData(context.Background(), td); got != want {
t.Fatalf("Want %v Got %v", want, got)
}
if "nop_trace" != nte.TraceExportFormat() {
if nte.TraceExportFormat() != "nop_trace" {
t.Fatalf("Wanted nop_trace got %s", nte.TraceExportFormat())
}
}
Expand All @@ -58,7 +58,7 @@ func TestNopMetricsExporter_NoErrors(t *testing.T) {
if err := nme.ConsumeMetricsData(context.Background(), md); err != nil {
t.Fatalf("Wanted nil got error")
}
if "nop_metrics" != nme.MetricsExportFormat() {
if nme.MetricsExportFormat() != "nop_metrics" {
t.Fatalf("Wanted nop_metrics got %s", nme.MetricsExportFormat())
}
}
Expand All @@ -72,7 +72,7 @@ func TestNopMetricsExporter_WithErrors(t *testing.T) {
if got := nme.ConsumeMetricsData(context.Background(), md); got != want {
t.Fatalf("Want %v Got %v", want, got)
}
if "nop_metrics" != nme.MetricsExportFormat() {
if nme.MetricsExportFormat() != "nop_metrics" {
t.Fatalf("Wanted nop_metrics got %s", nme.MetricsExportFormat())
}
}
4 changes: 2 additions & 2 deletions exporter/exportertest/sink_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestSinkTraceExporter(t *testing.T) {
if !reflect.DeepEqual(got, want) {
t.Errorf("Mismatches responses\nGot:\n\t%v\nWant:\n\t%v\n", got, want)
}
if "sink_trace" != sink.TraceExportFormat() {
if sink.TraceExportFormat() != "sink_trace" {
t.Errorf("Wanted sink_trace got %s", sink.TraceExportFormat())
}
}
Expand All @@ -60,7 +60,7 @@ func TestSinkMetricsExporter(t *testing.T) {
if !reflect.DeepEqual(got, want) {
t.Errorf("Mismatches responses\nGot:\n\t%v\nWant:\n\t%v\n", got, want)
}
if "sink_metrics" != sink.MetricsExportFormat() {
if sink.MetricsExportFormat() != "sink_metrics" {
t.Errorf("Wanted sink_metrics got %s", sink.MetricsExportFormat())
}
}
2 changes: 1 addition & 1 deletion exporter/jaegerexporter/jaeger_thrift_http_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (s *JaegerThriftHTTPSender) ConsumeTraceData(ctx context.Context, td consum
io.Copy(ioutil.Discard, resp.Body)
resp.Body.Close()
if resp.StatusCode >= http.StatusBadRequest {
return fmt.Errorf("Jaeger Thirft HTTP sender error: %d", resp.StatusCode)
return fmt.Errorf("jaeger Thirft HTTP sender error: %d", resp.StatusCode)
}
return nil
}
Expand Down
4 changes: 2 additions & 2 deletions exporter/loggingexporter/logging_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestLoggingTraceExporterNoErrors(t *testing.T) {
if err := lte.ConsumeTraceData(context.Background(), td); err != nil {
t.Fatalf("Wanted nil got %v", err)
}
if "logging_trace" != lte.TraceExportFormat() {
if lte.TraceExportFormat() != "logging_trace" {
t.Errorf("Wanted logging_trace got %v", lte.TraceExportFormat())
}
}
Expand All @@ -50,7 +50,7 @@ func TestLoggingMetricsExporterNoErrors(t *testing.T) {
if err := lme.ConsumeMetricsData(context.Background(), md); err != nil {
t.Fatalf("Wanted nil got %v", err)
}
if "logging_metrics" != lme.MetricsExportFormat() {
if lme.MetricsExportFormat() != "logging_metrics" {
t.Errorf("Wanted logging_metrics got %v", lme.MetricsExportFormat())
}
}
1 change: 0 additions & 1 deletion exporter/opencensusexporter/opencensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ type opencensusConfig struct {
}

type ocagentExporter struct {
counter uint32
exporters chan *ocagent.Exporter
}

Expand Down
1 change: 0 additions & 1 deletion exporter/prometheusexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ func TestCreateTraceExporter(t *testing.T) {
}

func TestCreateMetricsExporter(t *testing.T) {
const defaultTestEndPoint = "127.0.0.1:55678"
tests := []struct {
name string
config Config
Expand Down
2 changes: 1 addition & 1 deletion exporter/zipkinexporter/zipkin.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func ZipkinExportersFromViper(v *viper.Viper) (tps []consumer.TraceConsumer, mps
}
zle, err := newZipkinExporter(endpoint, serviceName, uploadPeriod)
if err != nil {
return nil, nil, nil, fmt.Errorf("Cannot configure Zipkin exporter: %v", err)
return nil, nil, nil, fmt.Errorf("cannot configure Zipkin exporter: %v", err)
}
tps = append(tps, zle)
doneFns = append(doneFns, zle.stop)
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,5 @@ require (
google.golang.org/api v0.5.0
google.golang.org/grpc v1.21.0
gopkg.in/yaml.v2 v2.2.2
honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a
)
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,7 @@ gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
honnef.co/go/tools v0.0.0-20180728063816-88497007e858/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a h1:/8zB6iBfHCl1qAnEAWwGPNrUvapuy6CPla1VM0k8hQw=
honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
k8s.io/api v0.0.0-20181213150558-05914d821849 h1:WZFcFPXmLR7g5CxQNmjWv0mg8qulJLxDghbzS4pQtzY=
k8s.io/api v0.0.0-20181213150558-05914d821849/go.mod h1:iuAfoD4hCxJ8Onx9kaTIt30j7jUFS00AXQi6QMi99vA=
Expand Down
3 changes: 1 addition & 2 deletions internal/collector/processor/idbatcher/id_batcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ type batcher struct {
cbMutex sync.Mutex
currentBatch Batch

numBatches uint64
newBatchesInitialCapacity uint64
stopchan chan bool
stopped bool
Expand Down Expand Up @@ -120,7 +119,7 @@ func (b *batcher) AddToCurrentBatch(id ID) {
}

func (b *batcher) CloseCurrentAndTakeFirstBatch() (Batch, bool) {
for readBatch := range b.batches {
if readBatch, ok := <-b.batches; ok {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was it intended to change the for by if?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, otherwise either test or static check would fail. The updated code is equivalent to the previous one.

if !b.stopped {
nextBatch := make(Batch, 0, b.newBatchesInitialCapacity)
b.cbMutex.Lock()
Expand Down
8 changes: 5 additions & 3 deletions internal/collector/processor/idbatcher/id_batcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,19 @@ func concurrencyTest(t *testing.T, numBatches, newBatchesInitialCapacity, batchC
var got Batch
go func() {
var completedDequeues uint64
outer:
for {
select {
case <-ticker.C:
g, _ := batcher.CloseCurrentAndTakeFirstBatch()
completedDequeues++
if completedDequeues <= numBatches && len(g) != 0 {
t.Fatal("Some of the first batches were not empty")
t.Error("Some of the first batches were not empty")
return
}
got = append(got, g...)
case <-stopTicker:
break
break outer
}
}
}()
Expand Down Expand Up @@ -154,7 +156,7 @@ func concurrencyTest(t *testing.T, numBatches, newBatchesInitialCapacity, batchC
}

func generateSequentialIds(numIds uint64) [][]byte {
ids := make([][]byte, numIds, numIds)
ids := make([][]byte, numIds)
for i := uint64(0); i < numIds; i++ {
ids[i] = tracetranslator.UInt64ToByteTraceID(0, i)
}
Expand Down
1 change: 0 additions & 1 deletion internal/collector/processor/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ func MetricTagKeys(level telemetry.Level) []tag.Key {
fallthrough
case telemetry.Basic:
tagKeys = append(tagKeys, TagExporterNameKey)
break
default:
return nil
}
Expand Down
1 change: 1 addition & 0 deletions internal/tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,5 @@ import (
_ "github.com/jstemmer/go-junit-report"
_ "golang.org/x/lint/golint"
_ "golang.org/x/tools/cmd/goimports"
_ "honnef.co/go/tools/cmd/staticcheck"
)
6 changes: 3 additions & 3 deletions observability/observabilitytest/observabilitytest.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func checkValueForView(vName string, wantTags []tag.Tag, value int64) error {

rows, err := view.RetrieveData(vName)
if err != nil {
return fmt.Errorf("Error retrieving view data for view Name %s", vName)
return fmt.Errorf("error retrieving view data for view Name %s", vName)
}

for _, row := range rows {
Expand All @@ -99,13 +99,13 @@ func checkValueForView(vName string, wantTags []tag.Tag, value int64) error {
if reflect.DeepEqual(wantTags, row.Tags) {
sum := row.Data.(*view.SumData)
if float64(value) != sum.Value {
return fmt.Errorf("Different recorded value: want %v got %v", float64(value), sum.Value)
return fmt.Errorf("different recorded value: want %v got %v", float64(value), sum.Value)
}
// We found the result
return nil
}
}
return fmt.Errorf("Could not find wantTags: %s in rows %v", wantTags, rows)
return fmt.Errorf("could not find wantTags: %s in rows %v", wantTags, rows)
}

func wantsTagsForExporterView(receiverName string, exporterTagName string) []tag.Tag {
Expand Down
2 changes: 1 addition & 1 deletion processor/addattributesprocessor/addattributesprocessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
)

var (
errUnsupportedType = errors.New("Unsupported type")
errUnsupportedType = errors.New("unsupported type")
)

type addattributesprocessor struct {
Expand Down
4 changes: 2 additions & 2 deletions processor/attributekeyprocessor/attributekeyprocessor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,11 @@ func Test_attributekeyprocessor_ConsumeTraceData(t *testing.T) {
},
},
td: consumerdata.TraceData{
Spans: make([]*tracepb.Span, 1, 1),
Spans: make([]*tracepb.Span, 1),
},
want: []consumerdata.TraceData{
{
Spans: make([]*tracepb.Span, 1, 1),
Spans: make([]*tracepb.Span, 1),
},
},
},
Expand Down
2 changes: 2 additions & 0 deletions processor/attributekeyprocessor/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (

func TestLoadConfig(t *testing.T) {
receivers, _, exporters, err := config.ExampleComponents()
require.NoError(t, err)
processors, err := processor.Build(&Factory{})
require.NotNil(t, processors)
require.NoError(t, err)
Expand Down Expand Up @@ -76,6 +77,7 @@ func TestLoadConfig(t *testing.T) {

func TestLoadConfigEmpty(t *testing.T) {
receivers, _, exporters, err := config.ExampleComponents()
require.NoError(t, err)
processors, err := processor.Build(&Factory{})
require.NotNil(t, processors)
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion processor/attributekeyprocessor/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (f *Factory) CreateDefaultConfig() configmodels.Processor {
TypeVal: typeStr,
NameVal: typeStr,
},
Keys: make(map[string]NewKeyProperties, 0),
Keys: make(map[string]NewKeyProperties),
}
}

Expand Down
2 changes: 0 additions & 2 deletions processor/nodebatcher/node_batcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ type batcher struct {
numTickers int
tickTime time.Duration
timeout time.Duration

bucketMu sync.RWMutex
}

var _ consumer.TraceConsumer = (*batcher)(nil)
Expand Down
1 change: 1 addition & 0 deletions processor/probabilisticsampler/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func TestLoadConfig(t *testing.T) {

func TestLoadConfigEmpty(t *testing.T) {
receivers, _, exporters, err := config.ExampleComponents()
require.NoError(t, err)
processors, err := processor.Build(&Factory{})
require.NotNil(t, processors)
require.NoError(t, err)
Expand Down
Loading