Skip to content

Commit

Permalink
Enable Lint Rules: struct-tag & unexported-return (jaegertracing#5533)
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 struct-tag & unexported-return in revive linter
- Only use tags on exported fields
- Converted exported function to have an exported return type

## 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 <[email protected]>
  • Loading branch information
FlamingSaint authored Jun 5, 2024
1 parent 6b95711 commit 14197fe
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 31 deletions.
6 changes: 0 additions & 6 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,6 @@ linters-settings:
# investigate, could be real bugs. But didn't recent Go version changed loop variables semantics?
- name: range-val-address
disabled: true
# enable after cleanup: "tag on not-exported field"
- name: struct-tag
disabled: true
# this is idiocy, promotes less readable code. Don't enable.
- name: var-declaration
disabled: true
Expand All @@ -219,9 +216,6 @@ linters-settings:
# "no nested structs are allowed" - don't enable, doesn't make sense
- name: nested-structs
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"
- name: deep-exit
disabled: true
Expand Down
2 changes: 1 addition & 1 deletion pkg/config/tlscfg/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type Options struct {
MaxVersion string `mapstructure:"max_version"`
SkipHostVerify bool `mapstructure:"skip_host_verify"`
ReloadInterval time.Duration `mapstructure:"reload_interval"`
certWatcher *certWatcher `mapstructure:"-"`
certWatcher *certWatcher
}

var systemCertPool = x509.SystemCertPool // to allow overriding in unit test
Expand Down
44 changes: 22 additions & 22 deletions plugin/storage/grpc/shared/grpc_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,16 @@ import (
const BearerTokenKey = "bearer.token"

var (
_ StoragePlugin = (*grpcClient)(nil)
_ ArchiveStoragePlugin = (*grpcClient)(nil)
_ PluginCapabilities = (*grpcClient)(nil)
_ StoragePlugin = (*GRPCClient)(nil)
_ ArchiveStoragePlugin = (*GRPCClient)(nil)
_ PluginCapabilities = (*GRPCClient)(nil)

// upgradeContext composites several steps of upgrading context
upgradeContext = composeContextUpgradeFuncs(upgradeContextWithBearerToken)
)

// grpcClient implements shared.StoragePlugin and reads/writes spans and dependencies
type grpcClient struct {
// GRPCClient implements shared.StoragePlugin and reads/writes spans and dependencies
type GRPCClient struct {
readerClient storage_v1.SpanReaderPluginClient
writerClient storage_v1.SpanWriterPluginClient
archiveReaderClient storage_v1.ArchiveSpanReaderPluginClient
Expand All @@ -57,8 +57,8 @@ type grpcClient struct {
streamWriterClient storage_v1.StreamingSpanWriterPluginClient
}

func NewGRPCClient(c *grpc.ClientConn) *grpcClient {
return &grpcClient{
func NewGRPCClient(c *grpc.ClientConn) *GRPCClient {
return &GRPCClient{
readerClient: storage_v1.NewSpanReaderPluginClient(c),
writerClient: storage_v1.NewSpanWriterPluginClient(c),
archiveReaderClient: storage_v1.NewArchiveSpanReaderPluginClient(c),
Expand Down Expand Up @@ -100,34 +100,34 @@ func upgradeContextWithBearerToken(ctx context.Context) context.Context {
}

// DependencyReader implements shared.StoragePlugin.
func (c *grpcClient) DependencyReader() dependencystore.Reader {
func (c *GRPCClient) DependencyReader() dependencystore.Reader {
return c
}

// SpanReader implements shared.StoragePlugin.
func (c *grpcClient) SpanReader() spanstore.Reader {
func (c *GRPCClient) SpanReader() spanstore.Reader {
return c
}

// SpanWriter implements shared.StoragePlugin.
func (c *grpcClient) SpanWriter() spanstore.Writer {
func (c *GRPCClient) SpanWriter() spanstore.Writer {
return c
}

func (c *grpcClient) StreamingSpanWriter() spanstore.Writer {
func (c *GRPCClient) StreamingSpanWriter() spanstore.Writer {
return newStreamingSpanWriter(c.streamWriterClient)
}

func (c *grpcClient) ArchiveSpanReader() spanstore.Reader {
func (c *GRPCClient) ArchiveSpanReader() spanstore.Reader {
return &archiveReader{client: c.archiveReaderClient}
}

func (c *grpcClient) ArchiveSpanWriter() spanstore.Writer {
func (c *GRPCClient) ArchiveSpanWriter() spanstore.Writer {
return &archiveWriter{client: c.archiveWriterClient}
}

// GetTrace takes a traceID and returns a Trace associated with that traceID
func (c *grpcClient) GetTrace(ctx context.Context, traceID model.TraceID) (*model.Trace, error) {
func (c *GRPCClient) GetTrace(ctx context.Context, traceID model.TraceID) (*model.Trace, error) {
stream, err := c.readerClient.GetTrace(upgradeContext(ctx), &storage_v1.GetTraceRequest{
TraceID: traceID,
})
Expand All @@ -142,7 +142,7 @@ func (c *grpcClient) GetTrace(ctx context.Context, traceID model.TraceID) (*mode
}

// GetServices returns a list of all known services
func (c *grpcClient) GetServices(ctx context.Context) ([]string, error) {
func (c *GRPCClient) GetServices(ctx context.Context) ([]string, error) {
resp, err := c.readerClient.GetServices(upgradeContext(ctx), &storage_v1.GetServicesRequest{})
if err != nil {
return nil, fmt.Errorf("plugin error: %w", err)
Expand All @@ -152,7 +152,7 @@ func (c *grpcClient) GetServices(ctx context.Context) ([]string, error) {
}

// GetOperations returns the operations of a given service
func (c *grpcClient) GetOperations(
func (c *GRPCClient) GetOperations(
ctx context.Context,
query spanstore.OperationQueryParameters,
) ([]spanstore.Operation, error) {
Expand Down Expand Up @@ -183,7 +183,7 @@ func (c *grpcClient) GetOperations(
}

// FindTraces retrieves traces that match the traceQuery
func (c *grpcClient) FindTraces(ctx context.Context, query *spanstore.TraceQueryParameters) ([]*model.Trace, error) {
func (c *GRPCClient) FindTraces(ctx context.Context, query *spanstore.TraceQueryParameters) ([]*model.Trace, error) {
stream, err := c.readerClient.FindTraces(upgradeContext(ctx), &storage_v1.FindTracesRequest{
Query: &storage_v1.TraceQueryParameters{
ServiceName: query.ServiceName,
Expand Down Expand Up @@ -221,7 +221,7 @@ func (c *grpcClient) FindTraces(ctx context.Context, query *spanstore.TraceQuery
}

// FindTraceIDs retrieves traceIDs that match the traceQuery
func (c *grpcClient) FindTraceIDs(ctx context.Context, query *spanstore.TraceQueryParameters) ([]model.TraceID, error) {
func (c *GRPCClient) FindTraceIDs(ctx context.Context, query *spanstore.TraceQueryParameters) ([]model.TraceID, error) {
resp, err := c.readerClient.FindTraceIDs(upgradeContext(ctx), &storage_v1.FindTraceIDsRequest{
Query: &storage_v1.TraceQueryParameters{
ServiceName: query.ServiceName,
Expand All @@ -242,7 +242,7 @@ func (c *grpcClient) FindTraceIDs(ctx context.Context, query *spanstore.TraceQue
}

// WriteSpan saves the span
func (c *grpcClient) WriteSpan(ctx context.Context, span *model.Span) error {
func (c *GRPCClient) WriteSpan(ctx context.Context, span *model.Span) error {
_, err := c.writerClient.WriteSpan(ctx, &storage_v1.WriteSpanRequest{
Span: span,
})
Expand All @@ -253,7 +253,7 @@ func (c *grpcClient) WriteSpan(ctx context.Context, span *model.Span) error {
return nil
}

func (c *grpcClient) Close() error {
func (c *GRPCClient) Close() error {
_, err := c.writerClient.Close(context.Background(), &storage_v1.CloseWriterRequest{})
if err != nil && status.Code(err) != codes.Unimplemented {
return fmt.Errorf("plugin error: %w", err)
Expand All @@ -263,7 +263,7 @@ func (c *grpcClient) Close() error {
}

// GetDependencies returns all interservice dependencies
func (c *grpcClient) GetDependencies(ctx context.Context, endTs time.Time, lookback time.Duration) ([]model.DependencyLink, error) {
func (c *GRPCClient) GetDependencies(ctx context.Context, endTs time.Time, lookback time.Duration) ([]model.DependencyLink, error) {
resp, err := c.depsReaderClient.GetDependencies(ctx, &storage_v1.GetDependenciesRequest{
EndTime: endTs,
StartTime: endTs.Add(-lookback),
Expand All @@ -275,7 +275,7 @@ func (c *grpcClient) GetDependencies(ctx context.Context, endTs time.Time, lookb
return resp.Dependencies, nil
}

func (c *grpcClient) Capabilities() (*Capabilities, error) {
func (c *GRPCClient) Capabilities() (*Capabilities, error) {
capabilities, err := c.capabilitiesClient.Capabilities(context.Background(), &storage_v1.CapabilitiesRequest{})
if status.Code(err) == codes.Unimplemented {
return &Capabilities{}, nil
Expand Down
4 changes: 2 additions & 2 deletions plugin/storage/grpc/shared/grpc_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ var (
)

type grpcClientTest struct {
client *grpcClient
client *GRPCClient
spanReader *grpcMocks.SpanReaderPluginClient
spanWriter *grpcMocks.SpanWriterPluginClient
archiveReader *grpcMocks.ArchiveSpanReaderPluginClient
Expand All @@ -93,7 +93,7 @@ func withGRPCClient(fn func(r *grpcClientTest)) {
capabilities := new(grpcMocks.PluginCapabilitiesClient)

r := &grpcClientTest{
client: &grpcClient{
client: &GRPCClient{
readerClient: spanReader,
writerClient: spanWriter,
archiveReaderClient: archiveReader,
Expand Down

0 comments on commit 14197fe

Please sign in to comment.