Skip to content

Commit

Permalink
Enable Lint Rule: exported (jaegertracing#5525)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
-  Partial Fix for jaegertracing#5506

## Description of the changes
- Enabled exported in revive linter
- Converted struct `Config` to `ConfigurationV2`
- Converted struct `DependencyStoreParams` to `Params`
- Converted struct `SamplingStoreParams` to `Params`

## How was this change tested?
- `make lint` `make test`

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: FlamingSaint <raghuramkannan400@gmail.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
  • Loading branch information
FlamingSaint and yurishkuro authored Jun 4, 2024
1 parent 2438ec0 commit 7dea2db
Show file tree
Hide file tree
Showing 15 changed files with 37 additions and 70 deletions.
3 changes: 0 additions & 3 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,6 @@ linters-settings:
# this is idiocy, promotes less readable code. Don't enable.
- name: var-declaration
disabled: true
# enable after cleanup: warns of stutter names
- name: exported
disabled: true
# enable after cleanup
- name: redefines-builtin-id
disabled: true
Expand Down
2 changes: 1 addition & 1 deletion cmd/jaeger/internal/extension/jaegerstorage/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
memoryCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
badgerCfg "github.com/jaegertracing/jaeger/plugin/storage/badger"
"github.com/jaegertracing/jaeger/plugin/storage/cassandra"
grpcCfg "github.com/jaegertracing/jaeger/plugin/storage/grpc/config"
grpcCfg "github.com/jaegertracing/jaeger/plugin/storage/grpc"
)

// Config has the configuration for jaeger-query,
Expand Down
3 changes: 1 addition & 2 deletions cmd/jaeger/internal/extension/jaegerstorage/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"github.com/jaegertracing/jaeger/plugin/storage/cassandra"
"github.com/jaegertracing/jaeger/plugin/storage/es"
"github.com/jaegertracing/jaeger/plugin/storage/grpc"
grpcCfg "github.com/jaegertracing/jaeger/plugin/storage/grpc/config"
"github.com/jaegertracing/jaeger/plugin/storage/memory"
"github.com/jaegertracing/jaeger/storage"
)
Expand Down Expand Up @@ -116,7 +115,7 @@ func (s *storageExt) Start(ctx context.Context, host component.Host) error {
cfg: s.config.Badger,
builder: badger.NewFactoryWithConfig,
}
grpcStarter := &starter[grpcCfg.ConfigV2, *grpc.Factory]{
grpcStarter := &starter[grpc.ConfigV2, *grpc.Factory]{
ext: s,
storageKind: "grpc",
cfg: s.config.GRPC,
Expand Down
4 changes: 2 additions & 2 deletions plugin/storage/es/dependencystore/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type DependencyStore struct {
}

// DependencyStoreParams holds constructor parameters for NewDependencyStore
type DependencyStoreParams struct {
type Params struct {
Client func() es.Client
Logger *zap.Logger
IndexPrefix string
Expand All @@ -57,7 +57,7 @@ type DependencyStoreParams struct {
}

// NewDependencyStore returns a DependencyStore
func NewDependencyStore(p DependencyStoreParams) *DependencyStore {
func NewDependencyStore(p Params) *DependencyStore {
return &DependencyStore{
client: p.Client,
logger: p.Logger,
Expand Down
22 changes: 11 additions & 11 deletions plugin/storage/es/dependencystore/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func withDepStorage(indexPrefix, indexDateLayout string, maxDocCount int, fn fun
client: client,
logger: logger,
logBuffer: logBuffer,
storage: NewDependencyStore(DependencyStoreParams{
storage: NewDependencyStore(Params{
Client: func() es.Client { return client },
Logger: logger,
IndexPrefix: indexPrefix,
Expand All @@ -79,7 +79,7 @@ func TestNewSpanReaderIndexPrefix(t *testing.T) {
}
for _, testCase := range testCases {
client := &mocks.Client{}
r := NewDependencyStore(DependencyStoreParams{
r := NewDependencyStore(Params{
Client: func() es.Client { return client },
Logger: zap.NewNop(),
IndexPrefix: testCase.prefix,
Expand Down Expand Up @@ -216,40 +216,40 @@ func TestGetReadIndices(t *testing.T) {
testCases := []struct {
indices []string
lookback time.Duration
params DependencyStoreParams
params Params
}{
{
params: DependencyStoreParams{IndexPrefix: "", IndexDateLayout: "2006-01-02", UseReadWriteAliases: true},
params: Params{IndexPrefix: "", IndexDateLayout: "2006-01-02", UseReadWriteAliases: true},
lookback: 23 * time.Hour,
indices: []string{
dependencyIndex + "read",
},
},
{
params: DependencyStoreParams{IndexPrefix: "", IndexDateLayout: "2006-01-02"},
params: Params{IndexPrefix: "", IndexDateLayout: "2006-01-02"},
lookback: 23 * time.Hour,
indices: []string{
dependencyIndex + fixedTime.Format("2006-01-02"),
dependencyIndex + fixedTime.Add(-23*time.Hour).Format("2006-01-02"),
},
},
{
params: DependencyStoreParams{IndexPrefix: "", IndexDateLayout: "2006-01-02"},
params: Params{IndexPrefix: "", IndexDateLayout: "2006-01-02"},
lookback: 13 * time.Hour,
indices: []string{
dependencyIndex + fixedTime.UTC().Format("2006-01-02"),
dependencyIndex + fixedTime.Add(-13*time.Hour).Format("2006-01-02"),
},
},
{
params: DependencyStoreParams{IndexPrefix: "foo:", IndexDateLayout: "2006-01-02"},
params: Params{IndexPrefix: "foo:", IndexDateLayout: "2006-01-02"},
lookback: 1 * time.Hour,
indices: []string{
"foo:" + indexPrefixSeparator + dependencyIndex + fixedTime.Format("2006-01-02"),
},
},
{
params: DependencyStoreParams{IndexPrefix: "foo-", IndexDateLayout: "2006-01-02"},
params: Params{IndexPrefix: "foo-", IndexDateLayout: "2006-01-02"},
lookback: 0,
indices: []string{
"foo-" + indexPrefixSeparator + dependencyIndex + fixedTime.Format("2006-01-02"),
Expand All @@ -267,14 +267,14 @@ func TestGetWriteIndex(t *testing.T) {
testCases := []struct {
writeIndex string
lookback time.Duration
params DependencyStoreParams
params Params
}{
{
params: DependencyStoreParams{IndexPrefix: "", IndexDateLayout: "2006-01-02", UseReadWriteAliases: true},
params: Params{IndexPrefix: "", IndexDateLayout: "2006-01-02", UseReadWriteAliases: true},
writeIndex: dependencyIndex + "write",
},
{
params: DependencyStoreParams{IndexPrefix: "", IndexDateLayout: "2006-01-02", UseReadWriteAliases: false},
params: Params{IndexPrefix: "", IndexDateLayout: "2006-01-02", UseReadWriteAliases: false},
writeIndex: dependencyIndex + fixedTime.Format("2006-01-02"),
},
}
Expand Down
4 changes: 2 additions & 2 deletions plugin/storage/es/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ func createSpanWriter(
}

func (f *Factory) CreateSamplingStore(maxBuckets int) (samplingstore.Store, error) {
params := esSampleStore.SamplingStoreParams{
params := esSampleStore.Params{
Client: f.getPrimaryClient,
Logger: f.logger,
IndexPrefix: f.primaryConfig.IndexPrefix,
Expand Down Expand Up @@ -341,7 +341,7 @@ func createDependencyReader(
cfg *config.Configuration,
logger *zap.Logger,
) (dependencystore.Reader, error) {
reader := esDepStore.NewDependencyStore(esDepStore.DependencyStoreParams{
reader := esDepStore.NewDependencyStore(esDepStore.Params{
Client: clientFn,
Logger: logger,
IndexPrefix: cfg.IndexPrefix,
Expand Down
6 changes: 3 additions & 3 deletions plugin/storage/es/samplingstore/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ type SamplingStore struct {
lookback time.Duration
}

type SamplingStoreParams struct {
type Params struct {
Client func() es.Client
Logger *zap.Logger
IndexPrefix string
Expand All @@ -55,7 +55,7 @@ type SamplingStoreParams struct {
MaxDocCount int
}

func NewSamplingStore(p SamplingStoreParams) *SamplingStore {
func NewSamplingStore(p Params) *SamplingStore {
return &SamplingStore{
client: p.Client,
logger: p.Logger,
Expand Down Expand Up @@ -195,7 +195,7 @@ func getReadIndices(indexName, indexDateLayout string, startTime time.Time, endT
return indices
}

func (p *SamplingStoreParams) PrefixedIndexName() string {
func (p *Params) PrefixedIndexName() string {
if p.IndexPrefix != "" {
return p.IndexPrefix + indexPrefixSeparator + samplingIndex
}
Expand Down
4 changes: 2 additions & 2 deletions plugin/storage/es/samplingstore/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func withEsSampling(indexPrefix, indexDateLayout string, maxDocCount int, fn fun
client: client,
logger: logger,
logBuffer: logBuffer,
storage: NewSamplingStore(SamplingStoreParams{
storage: NewSamplingStore(Params{
Client: func() es.Client { return client },
Logger: logger,
IndexPrefix: indexPrefix,
Expand Down Expand Up @@ -81,7 +81,7 @@ func TestNewIndexPrefix(t *testing.T) {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
client := &mocks.Client{}
r := NewSamplingStore(SamplingStoreParams{
r := NewSamplingStore(Params{
Client: func() es.Client { return client },
Logger: zap.NewNop(),
IndexPrefix: test.prefix,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package config
package grpc

import (
"context"
Expand Down
25 changes: 0 additions & 25 deletions plugin/storage/grpc/config/empty_test.go

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2024 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package config
package grpc

import (
"errors"
Expand Down
9 changes: 4 additions & 5 deletions plugin/storage/grpc/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (

"github.com/jaegertracing/jaeger/pkg/metrics"
"github.com/jaegertracing/jaeger/plugin"
"github.com/jaegertracing/jaeger/plugin/storage/grpc/config"
"github.com/jaegertracing/jaeger/storage"
"github.com/jaegertracing/jaeger/storage/dependencystore"
"github.com/jaegertracing/jaeger/storage/spanstore"
Expand All @@ -46,10 +45,10 @@ type Factory struct {
logger *zap.Logger
tracerProvider trace.TracerProvider

configV1 config.Configuration
configV2 *config.ConfigV2
configV1 Configuration
configV2 *ConfigV2

services *config.ClientPluginServices
services *ClientPluginServices
}

// NewFactory creates a new Factory.
Expand All @@ -59,7 +58,7 @@ func NewFactory() *Factory {

// NewFactoryWithConfig is used from jaeger(v2).
func NewFactoryWithConfig(
cfg config.ConfigV2,
cfg ConfigV2,
metricsFactory metrics.Factory,
logger *zap.Logger,
) (*Factory, error) {
Expand Down
9 changes: 4 additions & 5 deletions plugin/storage/grpc/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import (

"github.com/jaegertracing/jaeger/pkg/config"
"github.com/jaegertracing/jaeger/pkg/metrics"
grpcConfig "github.com/jaegertracing/jaeger/plugin/storage/grpc/config"
"github.com/jaegertracing/jaeger/plugin/storage/grpc/mocks"
"github.com/jaegertracing/jaeger/plugin/storage/grpc/shared"
"github.com/jaegertracing/jaeger/storage"
Expand Down Expand Up @@ -74,8 +73,8 @@ func (s *store) StreamingSpanWriter() spanstore.Writer {
return s.writer
}

func makeMockServices() *grpcConfig.ClientPluginServices {
return &grpcConfig.ClientPluginServices{
func makeMockServices() *ClientPluginServices {
return &ClientPluginServices{
PluginServices: shared.PluginServices{
Store: &store{
writer: new(spanStoreMocks.Writer),
Expand Down Expand Up @@ -110,7 +109,7 @@ func makeFactory(t *testing.T) *Factory {
}

func TestNewFactoryError(t *testing.T) {
cfg := &grpcConfig.ConfigV2{
cfg := &ConfigV2{
ClientConfig: configgrpc.ClientConfig{
// non-empty Auth is currently not supported
Auth: &configauth.Authentication{},
Expand Down Expand Up @@ -161,7 +160,7 @@ func TestGRPCStorageFactoryWithConfig(t *testing.T) {
}()
defer s.Stop()

cfg := grpcConfig.ConfigV2{
cfg := ConfigV2{
ClientConfig: configgrpc.ClientConfig{
Endpoint: lis.Addr().String(),
},
Expand Down
3 changes: 1 addition & 2 deletions plugin/storage/grpc/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (

"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/plugin/storage/grpc/config"
)

const (
Expand All @@ -47,7 +46,7 @@ func v1AddFlags(flagSet *flag.FlagSet) {
flagSet.Duration(remoteConnectionTimeout, defaultConnectionTimeout, "The remote storage gRPC server connection timeout")
}

func v1InitFromViper(cfg *config.Configuration, v *viper.Viper) error {
func v1InitFromViper(cfg *Configuration, v *viper.Viper) error {
cfg.RemoteServerAddr = v.GetString(remoteServer)
var err error
cfg.RemoteTLS, err = tlsFlagsConfig().InitFromViper(v)
Expand Down
9 changes: 4 additions & 5 deletions plugin/storage/grpc/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (

"github.com/jaegertracing/jaeger/pkg/config"
"github.com/jaegertracing/jaeger/pkg/tenancy"
grpccfg "github.com/jaegertracing/jaeger/plugin/storage/grpc/config"
)

func TestOptionsWithFlags(t *testing.T) {
Expand All @@ -33,7 +32,7 @@ func TestOptionsWithFlags(t *testing.T) {
"--multi-tenancy.header=x-scope-orgid",
})
require.NoError(t, err)
var cfg grpccfg.Configuration
var cfg Configuration
require.NoError(t, v1InitFromViper(&cfg, v))

assert.Equal(t, "foo:12345", cfg.RemoteServerAddr)
Expand All @@ -49,7 +48,7 @@ func TestRemoteOptionsWithFlags(t *testing.T) {
"--grpc-storage.connection-timeout=60s",
})
require.NoError(t, err)
var cfg grpccfg.Configuration
var cfg Configuration
require.NoError(t, v1InitFromViper(&cfg, v))

assert.Equal(t, "localhost:2001", cfg.RemoteServerAddr)
Expand All @@ -65,7 +64,7 @@ func TestRemoteOptionsNoTLSWithFlags(t *testing.T) {
"--grpc-storage.connection-timeout=60s",
})
require.NoError(t, err)
var cfg grpccfg.Configuration
var cfg Configuration
require.NoError(t, v1InitFromViper(&cfg, v))

assert.Equal(t, "localhost:2001", cfg.RemoteServerAddr)
Expand All @@ -80,7 +79,7 @@ func TestFailedTLSFlags(t *testing.T) {
"--grpc-storage.tls.cert=blah", // invalid unless tls.enabled=true
})
require.NoError(t, err)
var cfg grpccfg.Configuration
var cfg Configuration
err = v1InitFromViper(&cfg, v)
require.Error(t, err)
assert.Contains(t, err.Error(), "failed to parse gRPC storage TLS options")
Expand Down

0 comments on commit 7dea2db

Please sign in to comment.