Skip to content

Commit

Permalink
Add golangci-lint and fix problems (#25)
Browse files Browse the repository at this point in the history
Added golangci-lint yaml and lint workflow. Fixed all the findings.
  • Loading branch information
lucacome authored Nov 7, 2024
1 parent d08aa2b commit d814d2a
Show file tree
Hide file tree
Showing 14 changed files with 256 additions and 77 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,4 @@ jobs:
go-version: stable

- name: Run Unit Tests
run: go test -v ./...
run: go test ./... -race -shuffle=on -v
15 changes: 8 additions & 7 deletions .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,21 @@ name: "CodeQL"

on:
push:
branches: [ "master" ]
branches:
- master
pull_request:
branches: [ "master" ]
branches:
- master
schedule:
- cron: '41 15 * * 6'

concurrency:
group: ${{ github.ref_name }}-codeql
cancel-in-progress: true

jobs:
analyze:
name: Analyze (${{ matrix.language }})
# Runner size impacts CodeQL analysis time. To learn more, please see:
# - https://gh.io/recommended-hardware-resources-for-running-codeql
# - https://gh.io/supported-runners-and-hardware-resources
# - https://gh.io/using-larger-runners (GitHub.com only)
# Consider using larger runners or machines with greater resources for possible analysis time improvements.
runs-on: ubuntu-24.04
permissions:
# required for all workflows
Expand Down
45 changes: 45 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
name: Lint

on:
push:
branches:
- master
pull_request:
branches:
- master

defaults:
run:
shell: bash

concurrency:
group: ${{ github.ref_name }}-lint
cancel-in-progress: true

jobs:
lint:
name: Go Lint
runs-on: ubuntu-24.04
steps:
- name: Checkout Repository
uses: actions/checkout@v4

- name: Setup Golang Environment
uses: actions/setup-go@v5
with:
go-version: stable

- name: Lint Go
uses: golangci/golangci-lint-action@v6

actionlint:
name: Actionlint
runs-on: ubuntu-24.04
steps:
- name: Checkout Repository
uses: actions/checkout@v4

- name: Lint Actions
uses: reviewdog/action-actionlint@v1
with:
actionlint_flags: -shellcheck ""
91 changes: 91 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
linters-settings:
misspell:
locale: US
revive:
ignore-generated-header: true
rules:
- name: blank-imports
- name: context-as-argument
- name: context-keys-type
- name: dot-imports
- name: empty-block
- name: error-naming
- name: error-return
- name: error-strings
- name: errorf
- name: exported
- name: increment-decrement
- name: indent-error-flow
- name: package-comments
- name: range
- name: receiver-naming
- name: redefines-builtin-id
- name: superfluous-else
- name: time-naming
- name: unexported-return
- name: unreachable-code
- name: unused-parameter
- name: var-declaration
- name: var-naming
govet:
enable-all: true
linters:
enable:
- asasalint
- asciicheck
- bidichk
# - contextcheck
- copyloopvar
- dupword
- durationcheck
- errcheck
- errchkjson
- errname
- errorlint
- fatcontext
- forcetypeassert
- gocheckcompilerdirectives
- gochecksumtype
- gocritic
- godot
- gofmt
- gofumpt
- goimports
- gosec
- gosimple
- gosmopolitan
- govet
- ineffassign
- intrange
- makezero
- misspell
- musttag
- nilerr
- noctx
- nolintlint
- paralleltest
- perfsprint
- prealloc
- predeclared
- reassign
- revive
- staticcheck
- stylecheck
- tagalign
- tenv
- thelper
- tparallel
- typecheck
- unconvert
- unparam
- unused
- usestdlibvars
- wastedassign
- whitespace
# - wrapcheck
disable-all: true
issues:
max-issues-per-linter: 0
max-same-issues: 0
run:
timeout: 5m
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ systems in Go.

## Installation

```
```shell
go get github.com/opentracing-contrib/go-grpc
```

Expand Down Expand Up @@ -54,4 +54,3 @@ s := grpc.NewServer(

// All future RPC activity involving `s` will be automatically traced.
```

21 changes: 11 additions & 10 deletions client.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package otgrpc

import (
"context"
"errors"
"io"
"runtime"
"sync/atomic"

opentracing "github.com/opentracing/opentracing-go"
"context"
"github.com/opentracing/opentracing-go/ext"
"github.com/opentracing/opentracing-go/log"
"google.golang.org/grpc"
Expand All @@ -18,10 +19,10 @@ import (
//
// For example:
//
// conn, err := grpc.Dial(
// address,
// ..., // (existing DialOptions)
// grpc.WithUnaryInterceptor(otgrpc.OpenTracingClientInterceptor(tracer)))
// conn, err := grpc.Dial(
// address,
// ..., // (existing DialOptions)
// grpc.WithUnaryInterceptor(otgrpc.OpenTracingClientInterceptor(tracer)))
//
// All gRPC client spans will inject the OpenTracing SpanContext into the gRPC
// metadata; they will also look in the context.Context for an active
Expand Down Expand Up @@ -80,10 +81,10 @@ func OpenTracingClientInterceptor(tracer opentracing.Tracer, optFuncs ...Option)
//
// For example:
//
// conn, err := grpc.Dial(
// address,
// ..., // (existing DialOptions)
// grpc.WithStreamInterceptor(otgrpc.OpenTracingStreamClientInterceptor(tracer)))
// conn, err := grpc.Dial(
// address,
// ..., // (existing DialOptions)
// grpc.WithStreamInterceptor(otgrpc.OpenTracingStreamClientInterceptor(tracer)))
//
// All gRPC client spans will inject the OpenTracing SpanContext into the gRPC
// metadata; they will also look in the context.Context for an active
Expand Down Expand Up @@ -206,7 +207,7 @@ func (cs *openTracingClientStream) SendMsg(m interface{}) error {

func (cs *openTracingClientStream) RecvMsg(m interface{}) error {
err := cs.ClientStream.RecvMsg(m)
if err == io.EOF {
if errors.Is(err, io.EOF) {
cs.finishFunc(nil)
return err
} else if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const (
ServerError Class = "5xx"
)

// ErrorClass returns the class of the given error
// ErrorClass returns the class of the given error.
func ErrorClass(err error) Class {
if s, ok := status.FromError(err); ok {
switch s.Code() {
Expand Down
1 change: 1 addition & 0 deletions errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const (
)

func TestSpanTags(t *testing.T) {
t.Parallel()
tracer := mocktracer.New()
for code := firstCode; code <= lastCode; code++ {
// Client error
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ go 1.21.0

require (
github.com/golang/protobuf v1.5.4
github.com/grpc-ecosystem/grpc-opentracing v0.0.0-20180507213350-8e809c8a8645
github.com/opentracing/opentracing-go v1.2.0
github.com/stretchr/testify v1.9.0
google.golang.org/grpc v1.67.1
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek
github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/grpc-ecosystem/grpc-opentracing v0.0.0-20180507213350-8e809c8a8645 h1:MJG/KsmcqMwFAkh8mTnAwhyKoB+sTAnY4CACC110tbU=
github.com/grpc-ecosystem/grpc-opentracing v0.0.0-20180507213350-8e809c8a8645/go.mod h1:6iZfnjpejD4L/4DwD7NryNaJyCQdzwWwH2MWhCA90Kw=
github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+1B0VhjKrZUs=
github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
Expand Down
7 changes: 3 additions & 4 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type SpanInclusionFunc func(
method string,
req, resp interface{}) bool

// IncludingSpans binds a IncludeSpanFunc to the options
// IncludingSpans binds a IncludeSpanFunc to the options.
func IncludingSpans(inclusionFunc SpanInclusionFunc) Option {
return func(o *options) {
o.inclusionFunc = inclusionFunc
Expand Down Expand Up @@ -60,10 +60,9 @@ func SpanDecorator(decorator SpanDecoratorFunc) Option {
// scale well as production use dictates other configuration and tuning
// parameters.
type options struct {
logPayloads bool
decorator SpanDecoratorFunc
// May be nil.
decorator SpanDecoratorFunc
inclusionFunc SpanInclusionFunc
logPayloads bool
}

// newOptions returns the default options.
Expand Down
41 changes: 21 additions & 20 deletions server.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package otgrpc

import (
opentracing "github.com/opentracing/opentracing-go"
"context"

opentracing "github.com/opentracing/opentracing-go"
"github.com/opentracing/opentracing-go/ext"
"github.com/opentracing/opentracing-go/log"
"google.golang.org/grpc"
Expand All @@ -14,9 +15,9 @@ import (
//
// For example:
//
// s := grpc.NewServer(
// ..., // (existing ServerOptions)
// grpc.UnaryInterceptor(otgrpc.OpenTracingServerInterceptor(tracer)))
// s := grpc.NewServer(
// ..., // (existing ServerOptions)
// grpc.UnaryInterceptor(otgrpc.OpenTracingServerInterceptor(tracer)))
//
// All gRPC server spans will look for an OpenTracing SpanContext in the gRPC
// metadata; if found, the server span will act as the ChildOf that RPC
Expand All @@ -33,12 +34,12 @@ func OpenTracingServerInterceptor(tracer opentracing.Tracer, optFuncs ...Option)
info *grpc.UnaryServerInfo,
handler grpc.UnaryHandler,
) (resp interface{}, err error) {
spanContext, err := extractSpanContext(ctx, tracer)
if err != nil && err != opentracing.ErrSpanContextNotFound {
// TODO: establish some sort of error reporting mechanism here. We
// don't know where to put such an error and must rely on Tracer
// implementations to do something appropriate for the time being.
}
spanContext, _ := extractSpanContext(ctx, tracer)
// if err != nil && !errors.Is(err, opentracing.ErrSpanContextNotFound) {
// // TODO: establish some sort of error reporting mechanism here. We
// // don't know where to put such an error and must rely on Tracer
// // implementations to do something appropriate for the time being.
// }
if otgrpcOpts.inclusionFunc != nil &&
!otgrpcOpts.inclusionFunc(spanContext, info.FullMethod, req, nil) {
return handler(ctx, req)
Expand Down Expand Up @@ -76,9 +77,9 @@ func OpenTracingServerInterceptor(tracer opentracing.Tracer, optFuncs ...Option)
//
// For example:
//
// s := grpc.NewServer(
// ..., // (existing ServerOptions)
// grpc.StreamInterceptor(otgrpc.OpenTracingStreamServerInterceptor(tracer)))
// s := grpc.NewServer(
// ..., // (existing ServerOptions)
// grpc.StreamInterceptor(otgrpc.OpenTracingStreamServerInterceptor(tracer)))
//
// All gRPC server spans will look for an OpenTracing SpanContext in the gRPC
// metadata; if found, the server span will act as the ChildOf that RPC
Expand All @@ -90,12 +91,12 @@ func OpenTracingStreamServerInterceptor(tracer opentracing.Tracer, optFuncs ...O
otgrpcOpts := newOptions()
otgrpcOpts.apply(optFuncs...)
return func(srv interface{}, ss grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error {
spanContext, err := extractSpanContext(ss.Context(), tracer)
if err != nil && err != opentracing.ErrSpanContextNotFound {
// TODO: establish some sort of error reporting mechanism here. We
// don't know where to put such an error and must rely on Tracer
// implementations to do something appropriate for the time being.
}
spanContext, _ := extractSpanContext(ss.Context(), tracer)
// if err != nil && !errors.Is(err, opentracing.ErrSpanContextNotFound) {
// // TODO: establish some sort of error reporting mechanism here. We
// // don't know where to put such an error and must rely on Tracer
// // implementations to do something appropriate for the time being.
// }
if otgrpcOpts.inclusionFunc != nil &&
!otgrpcOpts.inclusionFunc(spanContext, info.FullMethod, nil, nil) {
return handler(srv, ss)
Expand All @@ -111,7 +112,7 @@ func OpenTracingStreamServerInterceptor(tracer opentracing.Tracer, optFuncs ...O
ServerStream: ss,
ctx: opentracing.ContextWithSpan(ss.Context(), serverSpan),
}
err = handler(srv, ss)
err := handler(srv, ss)
if err != nil {
SetSpanTags(serverSpan, err, false)
serverSpan.LogFields(log.String("event", "error"), log.String("message", err.Error()))
Expand Down
6 changes: 2 additions & 4 deletions shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@ import (
"google.golang.org/grpc/metadata"
)

var (
// Morally a const:
gRPCComponentTag = opentracing.Tag{string(ext.Component), "gRPC"}
)
// Morally a const:.
var gRPCComponentTag = opentracing.Tag{Key: string(ext.Component), Value: "gRPC"}

// metadataReaderWriter satisfies both the opentracing.TextMapReader and
// opentracing.TextMapWriter interfaces.
Expand Down
Loading

0 comments on commit d814d2a

Please sign in to comment.