diff --git a/.golangci.yml b/.golangci.yml index 68e3b6abe49..bb394382330 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -204,9 +204,6 @@ linters-settings: # maybe enable, needs invesitgation of the impact - name: get-return disabled: true - # enable after cleanup - - name: early-return - disabled: true # investigate, could be real bugs. But didn't recent Go version changed loop variables semantics? - name: range-val-address disabled: true @@ -223,9 +220,6 @@ linters-settings: - name: nested-structs disabled: true # enable after cleanup - - name: indent-error-flow - disabled: true - # enable after cleanup - name: unexported-return disabled: true # looks useful, but requires refactoring: "calls to log.Fatal only in main() or init() functions" diff --git a/cmd/agent/app/reporter/grpc/flags.go b/cmd/agent/app/reporter/grpc/flags.go index 72289e4b2fd..9ea504be4e2 100644 --- a/cmd/agent/app/reporter/grpc/flags.go +++ b/cmd/agent/app/reporter/grpc/flags.go @@ -51,11 +51,11 @@ func (b *ConnBuilder) InitFromViper(v *viper.Viper) (*ConnBuilder, error) { b.CollectorHostPorts = strings.Split(hostPorts, ",") } b.MaxRetry = uint(v.GetInt(retry)) - if tls, err := tlsFlagsConfig.InitFromViper(v); err == nil { - b.TLS = tls - } else { + tls, err := tlsFlagsConfig.InitFromViper(v) + if err != nil { return b, fmt.Errorf("failed to process TLS options: %w", err) } + b.TLS = tls b.DiscoveryMinPeers = v.GetInt(discoveryMinPeers) return b, nil } diff --git a/cmd/collector/app/flags/flags.go b/cmd/collector/app/flags/flags.go index 6c5e5a2babd..64a36b66e3b 100644 --- a/cmd/collector/app/flags/flags.go +++ b/cmd/collector/app/flags/flags.go @@ -239,11 +239,11 @@ func (opts *HTTPOptions) initFromViper(v *viper.Viper, logger *zap.Logger, cfg s opts.IdleTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPIdleTimeout) opts.ReadTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadTimeout) opts.ReadHeaderTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadHeaderTimeout) - if tlsOpts, err := cfg.tls.InitFromViper(v); err == nil { - opts.TLS = tlsOpts - } else { + tlsOpts, err := cfg.tls.InitFromViper(v) + if err != nil { return fmt.Errorf("failed to parse HTTP TLS options: %w", err) } + opts.TLS = tlsOpts return nil } @@ -252,11 +252,11 @@ func (opts *GRPCOptions) initFromViper(v *viper.Viper, logger *zap.Logger, cfg s opts.MaxReceiveMessageLength = v.GetInt(cfg.prefix + "." + flagSuffixGRPCMaxReceiveMessageLength) opts.MaxConnectionAge = v.GetDuration(cfg.prefix + "." + flagSuffixGRPCMaxConnectionAge) opts.MaxConnectionAgeGrace = v.GetDuration(cfg.prefix + "." + flagSuffixGRPCMaxConnectionAgeGrace) - if tlsOpts, err := cfg.tls.InitFromViper(v); err == nil { - opts.TLS = tlsOpts - } else { + tlsOpts, err := cfg.tls.InitFromViper(v) + if err != nil { return fmt.Errorf("failed to parse gRPC TLS options: %w", err) } + opts.TLS = tlsOpts opts.Tenancy = tenancy.InitFromViper(v) return nil @@ -289,11 +289,11 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) cOpts.Zipkin.KeepAlive = v.GetBool(flagZipkinKeepAliveEnabled) cOpts.Zipkin.HTTPHostPort = ports.FormatHostPort(v.GetString(flagZipkinHTTPHostPort)) - if tlsZipkin, err := tlsZipkinFlagsConfig.InitFromViper(v); err == nil { - cOpts.Zipkin.TLS = tlsZipkin - } else { + tlsZipkin, err := tlsZipkinFlagsConfig.InitFromViper(v) + if err != nil { return cOpts, fmt.Errorf("failed to parse Zipkin TLS options: %w", err) } + cOpts.Zipkin.TLS = tlsZipkin cOpts.Zipkin.CORS = corsZipkinFlags.InitFromViper(v) return cOpts, nil diff --git a/cmd/collector/app/span_processor_test.go b/cmd/collector/app/span_processor_test.go index 48550d331eb..ed1b6bde619 100644 --- a/cmd/collector/app/span_processor_test.go +++ b/cmd/collector/app/span_processor_test.go @@ -607,11 +607,10 @@ func TestStartDynQueueSizeUpdater(t *testing.T) { // we wait up to 50 milliseconds for i := 0; i < 5; i++ { - if p.queue.Capacity() == 100 { - time.Sleep(10 * time.Millisecond) - } else { + if p.queue.Capacity() != 100 { break } + time.Sleep(10 * time.Millisecond) } assert.EqualValues(t, 104857, p.queue.Capacity()) diff --git a/cmd/es-rollover/app/init/action.go b/cmd/es-rollover/app/init/action.go index d0b982125bc..dd4b2ef3fb5 100644 --- a/cmd/es-rollover/app/init/action.go +++ b/cmd/es-rollover/app/init/action.go @@ -62,17 +62,16 @@ func (c Action) Do() error { return err } if c.Config.UseILM { - if version >= ilmVersionSupport { - policyExist, err := c.ILMClient.Exists(c.Config.ILMPolicyName) - if err != nil { - return err - } - if !policyExist { - return fmt.Errorf("ILM policy %s doesn't exist in Elasticsearch. Please create it and re-run init", c.Config.ILMPolicyName) - } - } else { + if version < ilmVersionSupport { return fmt.Errorf("ILM is supported only for ES version 7+") } + policyExist, err := c.ILMClient.Exists(c.Config.ILMPolicyName) + if err != nil { + return err + } + if !policyExist { + return fmt.Errorf("ILM policy %s doesn't exist in Elasticsearch. Please create it and re-run init", c.Config.ILMPolicyName) + } } rolloverIndices := app.RolloverIndices(c.Config.Archive, c.Config.SkipDependencies, c.Config.AdaptiveSampling, c.Config.IndexPrefix) for _, indexName := range rolloverIndices { diff --git a/cmd/ingester/app/consumer/offset/concurrent_list.go b/cmd/ingester/app/consumer/offset/concurrent_list.go index decd0c8e4b2..0bf8c1cfdb0 100644 --- a/cmd/ingester/app/consumer/offset/concurrent_list.go +++ b/cmd/ingester/app/consumer/offset/concurrent_list.go @@ -76,11 +76,10 @@ func getHighestContiguous(offsets []int64) int64 { highestContiguous := minOffset for { - if _, ok := offsetSet[highestContiguous+1]; ok { - highestContiguous++ - } else { + if _, ok := offsetSet[highestContiguous+1]; !ok { break } + highestContiguous++ } return highestContiguous diff --git a/cmd/internal/flags/service.go b/cmd/internal/flags/service.go index 622a960a4e2..88ae89166c5 100644 --- a/cmd/internal/flags/service.go +++ b/cmd/internal/flags/service.go @@ -84,16 +84,16 @@ func (s *Service) Start(v *viper.Viper) error { sFlags := new(SharedFlags).InitFromViper(v) newProdConfig := zap.NewProductionConfig() newProdConfig.Sampling = nil - if logger, err := sFlags.NewLogger(newProdConfig); err == nil { - s.Logger = logger - grpcZap.ReplaceGrpcLoggerV2(logger.WithOptions( - // grpclog is not consistent with the depth of call tree before it's dispatched to zap, - // but Skip(2) still shows grpclog as caller, while Skip(3) shows actual grpc packages. - zap.AddCallerSkip(3), - )) - } else { + logger, err := sFlags.NewLogger(newProdConfig) + if err != nil { return fmt.Errorf("cannot create logger: %w", err) } + s.Logger = logger + grpcZap.ReplaceGrpcLoggerV2(logger.WithOptions( + // grpclog is not consistent with the depth of call tree before it's dispatched to zap, + // but Skip(2) still shows grpclog as caller, while Skip(3) shows actual grpc packages. + zap.AddCallerSkip(3), + )) metricsBuilder := new(metricsbuilder.Builder).InitFromViper(v) metricsFactory, err := metricsBuilder.CreateMetricsFactory("") diff --git a/cmd/jaeger/internal/extension/jaegerstorage/config.go b/cmd/jaeger/internal/extension/jaegerstorage/config.go index 41b7333df24..88b43f9f56a 100644 --- a/cmd/jaeger/internal/extension/jaegerstorage/config.go +++ b/cmd/jaeger/internal/extension/jaegerstorage/config.go @@ -36,7 +36,6 @@ func (cfg *Config) Validate() error { emptyCfg := createDefaultConfig().(*Config) if reflect.DeepEqual(*cfg, *emptyCfg) { return fmt.Errorf("%s: no storage type present in config", ID) - } else { - return nil } + return nil } diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index 7304b5f7d09..c2b7d823900 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -122,16 +122,16 @@ func AddFlags(flagSet *flag.FlagSet) { func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*QueryOptions, error) { qOpts.HTTPHostPort = v.GetString(queryHTTPHostPort) qOpts.GRPCHostPort = v.GetString(queryGRPCHostPort) - if tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v); err == nil { - qOpts.TLSGRPC = tlsGrpc - } else { + tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v) + if err != nil { return qOpts, fmt.Errorf("failed to process gRPC TLS options: %w", err) } - if tlsHTTP, err := tlsHTTPFlagsConfig.InitFromViper(v); err == nil { - qOpts.TLSHTTP = tlsHTTP - } else { + qOpts.TLSGRPC = tlsGrpc + tlsHTTP, err := tlsHTTPFlagsConfig.InitFromViper(v) + if err != nil { return qOpts, fmt.Errorf("failed to process HTTP TLS options: %w", err) } + qOpts.TLSHTTP = tlsHTTP qOpts.BasePath = v.GetString(queryBasePath) qOpts.StaticAssets.Path = v.GetString(queryStaticFiles) qOpts.StaticAssets.LogAccess = v.GetBool(queryLogStaticAssetsAccess) diff --git a/cmd/query/app/query_parser.go b/cmd/query/app/query_parser.go index 48b1ad9b146..4e781977085 100644 --- a/cmd/query/app/query_parser.go +++ b/cmd/query/app/query_parser.go @@ -165,11 +165,11 @@ func (p *queryParser) parseTraceQueryParams(r *http.Request) (*traceQueryParamet var traceIDs []model.TraceID for _, id := range r.Form[traceIDParam] { - if traceID, err := model.TraceIDFromString(id); err == nil { - traceIDs = append(traceIDs, traceID) - } else { + traceID, err := model.TraceIDFromString(id) + if err != nil { return nil, fmt.Errorf("cannot parse traceID param: %w", err) } + traceIDs = append(traceIDs, traceID) } traceQuery := &traceQueryParameters{ @@ -360,11 +360,11 @@ func parseSpanKinds(r *http.Request, paramName string, defaultSpanKinds []string func mapSpanKindsToOpenTelemetry(spanKinds []string) ([]string, error) { otelSpanKinds := make([]string, len(spanKinds)) for i, spanKind := range spanKinds { - if v, ok := jaegerToOtelSpanKind[spanKind]; ok { - otelSpanKinds[i] = v - } else { + v, ok := jaegerToOtelSpanKind[spanKind] + if !ok { return otelSpanKinds, fmt.Errorf("unsupported span kind: '%s'", spanKind) } + otelSpanKinds[i] = v } return otelSpanKinds, nil } @@ -385,11 +385,10 @@ func (*queryParser) parseTags(simpleTags []string, jsonTags []string) (map[strin retMe := make(map[string]string) for _, tag := range simpleTags { keyAndValue := strings.Split(tag, ":") - if l := len(keyAndValue); l > 1 { - retMe[keyAndValue[0]] = strings.Join(keyAndValue[1:], ":") - } else { + if l := len(keyAndValue); l <= 1 { return nil, fmt.Errorf("malformed 'tag' parameter, expecting key:value, received: %s", tag) } + retMe[keyAndValue[0]] = strings.Join(keyAndValue[1:], ":") } for _, tags := range jsonTags { var fromJSON map[string]string diff --git a/cmd/remote-storage/app/flags.go b/cmd/remote-storage/app/flags.go index c30426bc19e..8e7cebc2f1d 100644 --- a/cmd/remote-storage/app/flags.go +++ b/cmd/remote-storage/app/flags.go @@ -54,11 +54,11 @@ func AddFlags(flagSet *flag.FlagSet) { // InitFromViper initializes Options with properties from CLI flags. func (o *Options) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Options, error) { o.GRPCHostPort = v.GetString(flagGRPCHostPort) - if tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v); err == nil { - o.TLSGRPC = tlsGrpc - } else { + tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v) + if err != nil { return o, fmt.Errorf("failed to process gRPC TLS options: %w", err) } + o.TLSGRPC = tlsGrpc o.Tenancy = tenancy.InitFromViper(v) return o, nil } diff --git a/pkg/es/client/index_client.go b/pkg/es/client/index_client.go index 99e4255c6d0..e98acba8c99 100644 --- a/pkg/es/client/index_client.go +++ b/pkg/es/client/index_client.go @@ -244,12 +244,10 @@ func (i IndicesClient) version() (uint, error) { // CreateTemplate an ES index template func (i IndicesClient) CreateTemplate(template, name string) error { endpointFmt := "_template/%s" - if v, err := i.version(); err == nil { - if v >= 8 { - endpointFmt = "_index_template/%s" - } - } else { + if v, err := i.version(); err != nil { return err + } else if v >= 8 { + endpointFmt = "_index_template/%s" } _, err := i.request(elasticRequest{ endpoint: fmt.Sprintf(endpointFmt, name), diff --git a/pkg/fswatcher/fswatcher.go b/pkg/fswatcher/fswatcher.go index d3c929d1d9b..fa7ef7f8ca5 100644 --- a/pkg/fswatcher/fswatcher.go +++ b/pkg/fswatcher/fswatcher.go @@ -86,11 +86,11 @@ func (w *FSWatcher) setupWatchedPaths(filepaths []string) error { if p == "" { continue } - if h, err := hashFile(p); err == nil { - w.fileHashContentMap[p] = h - } else { + h, err := hashFile(p) + if err != nil { return err } + w.fileHashContentMap[p] = h dir := path.Dir(p) if _, ok := uniqueDirs[dir]; !ok { if err := w.watcher.Add(dir); err != nil { diff --git a/pkg/queue/bounded_queue_test.go b/pkg/queue/bounded_queue_test.go index 36c508b2872..b398d486425 100644 --- a/pkg/queue/bounded_queue_test.go +++ b/pkg/queue/bounded_queue_test.go @@ -80,11 +80,10 @@ func helper(t *testing.T, startConsumers func(q *BoundedQueue, consumerFn func(i q.StartLengthReporting(time.Millisecond, gauge) for i := 0; i < 1000; i++ { _, g := mFact.Snapshot() - if g["size"] == 0 { - time.Sleep(time.Millisecond) - } else { + if g["size"] != 0 { break } + time.Sleep(time.Millisecond) } c, g := mFact.Snapshot() diff --git a/plugin/storage/cassandra/factory.go b/plugin/storage/cassandra/factory.go index 19c48f971ec..ae082e99cc5 100644 --- a/plugin/storage/cassandra/factory.go +++ b/plugin/storage/cassandra/factory.go @@ -155,11 +155,11 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) f.primarySession = primarySession if f.archiveConfig != nil { - if archiveSession, err := f.archiveConfig.NewSession(logger); err == nil { - f.archiveSession = archiveSession - } else { + archiveSession, err := f.archiveConfig.NewSession(logger) + if err != nil { return err } + f.archiveSession = archiveSession } else { logger.Info("Cassandra archive storage configuration is empty, skipping") } diff --git a/plugin/storage/grpc/shared/grpc_handler.go b/plugin/storage/grpc/shared/grpc_handler.go index 9b235786a50..fd76ac7f1d3 100644 --- a/plugin/storage/grpc/shared/grpc_handler.go +++ b/plugin/storage/grpc/shared/grpc_handler.go @@ -156,9 +156,8 @@ func (s *GRPCHandler) Close(ctx context.Context, r *storage_v1.CloseWriterReques } return &storage_v1.CloseWriterResponse{}, nil - } else { - return nil, status.Error(codes.Unimplemented, "span writer does not support graceful shutdown") } + return nil, status.Error(codes.Unimplemented, "span writer does not support graceful shutdown") } // GetTrace takes a traceID and streams a Trace associated with that traceID