diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 00000000..f7a9ca1d --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -0,0 +1,25 @@ +name: golangci-lint +on: + push: + branches: + - main + pull_request: + +permissions: + contents: read + checks: write + +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: '1.22' + cache: false + - name: golangci-lint + uses: golangci/golangci-lint-action@v4 + with: + version: v1.54 \ No newline at end of file diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 719ea546..c0290b65 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -4,7 +4,7 @@ on: pull_request: null push: branches: - - master + - main jobs: license-check: @@ -42,8 +42,3 @@ jobs: run: go vet ./... - name: Run tests run: make test - - uses: dominikh/staticcheck-action@v1.2.0 - if: matrix.goversion == '1.21' - with: - install-go: false - cache-key: ${{ matrix.goversion }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ff718d62..9cdd218b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -53,7 +53,7 @@ Run all tests with `make test`. We enforce the following in CI: -* [`staticcheck ./...`](https://staticcheck.dev/docs/) +* [`golangci-lint run`](https://golangci-lint.run/) * [`go vet ./...`](https://pkg.go.dev/cmd/vet) * [`gofmt`](https://pkg.go.dev/cmd/gofmt) diff --git a/internal/hdrhist/encode.go b/internal/hdrhist/encode.go index 06b05c9a..bc56cec0 100644 --- a/internal/hdrhist/encode.go +++ b/internal/hdrhist/encode.go @@ -11,6 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. + package hdrhist import ( @@ -27,10 +28,13 @@ func EncodeCompressed(h *Hist) ([]byte, error) { var buf bytes.Buffer b64w := base64.NewEncoder(base64.StdEncoding, &buf) if err := encodeCompressed(h, b64w, h.Max()); err != nil { - b64w.Close() + _ = b64w.Close() return nil, errors.Wrap(err, "unable to encode histogram") } - b64w.Close() // DO NOT defer this close, otherwise that could prevent bytes from getting flushed + // DO NOT defer this close, otherwise that could prevent bytes from getting flushed + if err := b64w.Close(); err != nil { + return nil, err + } return buf.Bytes(), nil } @@ -39,15 +43,22 @@ func encodeCompressed(h *Hist, w io.Writer, histMax int64) error { var buf bytes.Buffer var cookie int32 = compressedEncodingCookie - binary.Write(&buf, binary.BigEndian, cookie) + err := binary.Write(&buf, binary.BigEndian, cookie) + if err != nil { + return err + } buf.WriteString("\x00\x00\x00\x00") preCompressed := buf.Len() zw, _ := zlib.NewWriterLevel(&buf, zlib.BestCompression) - encodeInto(h, zw, histMax) // won't error, not io device - zw.Close() + if err = encodeInto(h, zw, histMax); err != nil { + return err + } + if err = zw.Close(); err != nil { + return err + } binary.BigEndian.PutUint32(buf.Bytes()[4:], uint32(buf.Len()-preCompressed)) - _, err := buf.WriteTo(w) + _, err = buf.WriteTo(w) return errors.Wrap(err, "unable to write compressed hist") } @@ -57,13 +68,21 @@ func encodeInto(h *Hist, w io.Writer, max int64) error { importantLen := h.b.countsIndex(max) + 1 var buf bytes.Buffer var cookie int32 = encodingCookie - binary.Write(&buf, binary.BigEndian, cookie) + if err := binary.Write(&buf, binary.BigEndian, cookie); err != nil { + return err + } buf.WriteString("\x00\x00\x00\x00") // length will go here buf.WriteString("\x00\x00\x00\x00") // normalizing index offset cfg := h.Config() - binary.Write(&buf, binary.BigEndian, int32(cfg.SigFigs)) - binary.Write(&buf, binary.BigEndian, int64(cfg.LowestDiscernible)) - binary.Write(&buf, binary.BigEndian, int64(cfg.HighestTrackable)) + if err := binary.Write(&buf, binary.BigEndian, cfg.SigFigs); err != nil { + return err + } + if err := binary.Write(&buf, binary.BigEndian, cfg.LowestDiscernible); err != nil { + return err + } + if err := binary.Write(&buf, binary.BigEndian, cfg.HighestTrackable); err != nil { + return err + } // int to double conversion ratio buf.WriteString("\x3f\xf0\x00\x00\x00\x00\x00\x00") payloadStart := buf.Len() diff --git a/internal/hdrhist/hist.go b/internal/hdrhist/hist.go index 23e26163..eee68915 100644 --- a/internal/hdrhist/hist.go +++ b/internal/hdrhist/hist.go @@ -191,11 +191,7 @@ func (b *buckets) valueFor(i int) int64 { func (b *buckets) sizeOfEquivalentValueRange(v int64) int64 { bi := bucketIndex(v, b.subMask, b.leadZeroCountBase) - sbi := subBucketIndex(v, bi, b.unitMag) t := bi - if sbi >= int(b.subCount) { - bi++ - } nextDist := int64(1) << uint64(int64(b.unitMag)+int64(t)) return nextDist } diff --git a/internal/hdrhist/log_writer.go b/internal/hdrhist/log_writer.go index 1617b5f6..3234b95d 100644 --- a/internal/hdrhist/log_writer.go +++ b/internal/hdrhist/log_writer.go @@ -92,8 +92,12 @@ func (l *LogWriter) writeHist(h *Hist, start time.Time, end time.Time) error { float64(end.Sub(start)/1e6)/1e3, float64(max)/MaxValueUnitRatio) b64w := base64.NewEncoder(base64.StdEncoding, &l.buf) - encodeCompressed(h, b64w, max) // not writing to disk yet, won't fail - b64w.Close() + if err := encodeCompressed(h, b64w, max); err != nil { + return err + } + if err := b64w.Close(); err != nil { + return err + } l.buf.WriteString("\n") _, err := l.buf.WriteTo(l.w) return errors.Wrap(err, "unable to write hist") diff --git a/internal/host/sys_linux.go b/internal/host/sys_linux.go index 90fb154e..420f07c9 100644 --- a/internal/host/sys_linux.go +++ b/internal/host/sys_linux.go @@ -68,7 +68,7 @@ func initDistro() (distro string) { distro = utils.GetStrByKeyword(SUSE, "PRETTY_NAME") if distro != "" { // TODO - //lint:ignore SA1024 This is untested so until we have coverage I'm not inclined to fix this + //nolint:staticcheck // This is untested so until we have coverage I'm not inclined to fix this distro = strings.TrimLeft(distro, "PRETTY_NAME=") distro = strings.Trim(distro, "\"") return distro diff --git a/internal/log/logging.go b/internal/log/logging.go index 3abfc270..a92e031a 100644 --- a/internal/log/logging.go +++ b/internal/log/logging.go @@ -193,7 +193,7 @@ func logIt(level LogLevel, msg string, args []interface{}) { buffer.WriteString(pre) - s := msg + var s string if msg == "" { s = fmt.Sprint(args...) } else { diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index f343a410..d7aa06cb 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -985,7 +985,6 @@ func BuildServerlessMessage(span HTTPSpanMessage, rcs map[string]*RateCounts, ra } if ttTraced != 0 { bbuf.AppendString(strconv.Itoa(i), "Triggered") - i++ } bbuf.AppendFinishObject(start) @@ -1038,7 +1037,7 @@ func RecordSpan(span sdktrace.ReadOnlySpan, isAppoptics bool) { Method: method, } - var tagsList []map[string]string = nil + var tagsList []map[string]string var metricName string if !isAppoptics { tagsList = []map[string]string{swoTags} diff --git a/internal/metrics/metrics_linux_test.go b/internal/metrics/metrics_linux_test.go index ad4b0224..8b560a08 100644 --- a/internal/metrics/metrics_linux_test.go +++ b/internal/metrics/metrics_linux_test.go @@ -18,6 +18,7 @@ package metrics import ( "github.com/solarwinds/apm-go/internal/bson" "github.com/solarwinds/apm-go/internal/utils" + "github.com/stretchr/testify/require" "strings" "syscall" "testing" @@ -29,7 +30,8 @@ func TestAppendUname(t *testing.T) { bbuf := bson.NewBuffer() appendUname(bbuf) bbuf.Finish() - m := bsonToMap(bbuf) + m, err := bsonToMap(bbuf) + require.NoError(t, err) var sysname, release string diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index 2ad1a1ab..8e5492b1 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -24,6 +24,7 @@ import ( "github.com/solarwinds/apm-go/internal/log" "github.com/solarwinds/apm-go/internal/swotel/semconv" "github.com/solarwinds/apm-go/internal/testutils" + "github.com/stretchr/testify/require" sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/trace" "math" @@ -38,10 +39,12 @@ import ( mbson "gopkg.in/mgo.v2/bson" ) -func bsonToMap(bbuf *bson.Buffer) map[string]interface{} { +func bsonToMap(bbuf *bson.Buffer) (map[string]interface{}, error) { m := make(map[string]interface{}) - mbson.Unmarshal(bbuf.GetBuf(), m) - return m + if err := mbson.Unmarshal(bbuf.GetBuf(), m); err != nil { + return nil, err + } + return m, nil } func round(val float64, roundOn float64, places int) (newVal float64) { @@ -62,7 +65,8 @@ func TestAppendIPAddresses(t *testing.T) { bbuf := bson.NewBuffer() appendIPAddresses(bbuf) bbuf.Finish() - m := bsonToMap(bbuf) + m, err := bsonToMap(bbuf) + require.NoError(t, err) ifaces, _ := host.FilteredIfaces() var addresses []string @@ -94,7 +98,8 @@ func TestAppendMACAddresses(t *testing.T) { bbuf := bson.NewBuffer() appendMACAddresses(bbuf, host.CurrentID().MAC()) bbuf.Finish() - m := bsonToMap(bbuf) + m, err := bsonToMap(bbuf) + require.NoError(t, err) ifaces, _ := host.FilteredIfaces() var macs []string @@ -131,7 +136,8 @@ func TestAddMetricsValue(t *testing.T) { addMetricsValue(bbuf, &index, "name4", float64(444.44)) addMetricsValue(bbuf, &index, "name5", "hello") bbuf.Finish() - m := bsonToMap(bbuf) + m, err := bsonToMap(bbuf) + require.NoError(t, err) assert.NotZero(t, m["0"]) m2 := m["0"].(map[string]interface{}) @@ -223,8 +229,10 @@ func TestRecordMeasurement(t *testing.T) { t1 := make(map[string]string) t1["t1"] = "tag1" t1["t2"] = "tag2" - me.recordWithSoloTags("name1", t1, 111.11, 1, false) - me.recordWithSoloTags("name1", t1, 222, 1, false) + err := me.recordWithSoloTags("name1", t1, 111.11, 1, false) + require.NoError(t, err) + err = me.recordWithSoloTags("name1", t1, 222, 1, false) + require.NoError(t, err) assert.NotNil(t, me.m["name1&false&t1:tag1&t2:tag2&"]) m := me.m["name1&false&t1:tag1&t2:tag2&"] assert.Equal(t, "tag1", m.Tags["t1"]) @@ -235,7 +243,8 @@ func TestRecordMeasurement(t *testing.T) { t2 := make(map[string]string) t2["t3"] = "tag3" - me.recordWithSoloTags("name2", t2, 123.456, 3, true) + err = me.recordWithSoloTags("name2", t2, 123.456, 3, true) + require.NoError(t, err) assert.NotNil(t, me.m["name2&true&t3:tag3&"]) m = me.m["name2&true&t3:tag3&"] assert.Equal(t, "tag3", m.Tags["t3"]) @@ -305,7 +314,8 @@ func TestAddMeasurementToBSON(t *testing.T) { addMeasurementToBSON(bbuf, &index, measurement1) addMeasurementToBSON(bbuf, &index, measurement2) bbuf.Finish() - m := bsonToMap(bbuf) + m, err := bsonToMap(bbuf) + require.NoError(t, err) assert.NotZero(t, m["0"]) m1 := m["0"].(map[string]interface{}) @@ -365,7 +375,8 @@ func TestAddHistogramToBSON(t *testing.T) { addHistogramToBSON(bbuf, &index, h1) addHistogramToBSON(bbuf, &index, h2) bbuf.Finish() - m := bsonToMap(bbuf) + m, err := bsonToMap(bbuf) + require.NoError(t, err) assert.NotZero(t, m["0"]) m1 := m["0"].(map[string]interface{}) @@ -392,7 +403,8 @@ func TestGenerateMetricsMessage(t *testing.T) { RCRegular: {10, 2, 5, 5, 1}, RCRelaxedTriggerTrace: {3, 0, 1, 2, 0}, RCStrictTriggerTrace: {4, 0, 3, 1, 0}}, true)) - m := bsonToMap(bbuf) + m, err := bsonToMap(bbuf) + require.NoError(t, err) _, ok := m["Hostname"] assert.False(t, ok) @@ -477,8 +489,9 @@ func TestGenerateMetricsMessage(t *testing.T) { } } - m = bsonToMap(bson.WithBuf(BuildBuiltinMetricsMessage(testMetrics, &EventQueueStats{}, + m, err = bsonToMap(bson.WithBuf(BuildBuiltinMetricsMessage(testMetrics, &EventQueueStats{}, map[string]*RateCounts{RCRegular: {}, RCRelaxedTriggerTrace: {}, RCStrictTriggerTrace: {}}, true))) + require.NoError(t, err) assert.NotNil(t, m["TransactionNameOverflow"]) assert.True(t, m["TransactionNameOverflow"].(bool)) diff --git a/internal/reporter/log_writer.go b/internal/reporter/log_writer.go index 97b0fdf2..d381b7de 100644 --- a/internal/reporter/log_writer.go +++ b/internal/reporter/log_writer.go @@ -132,7 +132,7 @@ func (lr *logWriter) flush() error { } if file, ok := lr.dest.(*os.File); ok { - file.Sync() + return file.Sync() } return nil diff --git a/internal/reporter/log_writer_test.go b/internal/reporter/log_writer_test.go index 2eed93a2..12ff2db7 100644 --- a/internal/reporter/log_writer_test.go +++ b/internal/reporter/log_writer_test.go @@ -15,6 +15,7 @@ package reporter import ( "github.com/solarwinds/apm-go/internal/utils" + "github.com/stretchr/testify/require" "testing" "github.com/stretchr/testify/assert" @@ -23,14 +24,16 @@ import ( func TestLogWriter(t *testing.T) { sb := &utils.SafeBuffer{} eventWriter := newLogWriter(false, sb, 1e6) - eventWriter.Write(EventWT, []byte("hello event")) + _, err := eventWriter.Write(EventWT, []byte("hello event")) + require.NoError(t, err) assert.Equal(t, 0, sb.Len()) - eventWriter.Flush() + require.NoError(t, eventWriter.Flush()) assert.Equal(t, "{\"apm-data\":{\"events\":[\"aGVsbG8gZXZlbnQ=\"]}}\n", sb.String()) sb.Reset() metricWriter := newLogWriter(true, sb, 1e6) - metricWriter.Write(MetricWT, []byte("hello metric")) + _, err = metricWriter.Write(MetricWT, []byte("hello metric")) + require.NoError(t, err) assert.Equal(t, "{\"apm-data\":{\"metrics\":[\"aGVsbG8gbWV0cmlj\"]}}\n", sb.String()) assert.NotNil(t, metricWriter.Flush()) @@ -40,12 +43,14 @@ func TestLogWriter(t *testing.T) { assert.Zero(t, n) assert.Error(t, err) - writer.Write(EventWT, []byte("hello")) + _, err = writer.Write(EventWT, []byte("hello")) + require.NoError(t, err) assert.Zero(t, sb.Len()) - writer.Write(EventWT, []byte(" event")) + _, err = writer.Write(EventWT, []byte(" event")) + require.NoError(t, err) assert.Equal(t, 37, sb.Len()) assert.Equal(t, "{\"apm-data\":{\"events\":[\"aGVsbG8=\"]}}\n", sb.String()) - writer.Flush() + require.NoError(t, writer.Flush()) assert.Equal(t, "{\"apm-data\":{\"events\":[\"aGVsbG8=\"]}}\n{\"apm-data\":{\"events\":[\"IGV2ZW50\"]}}\n", sb.String()) diff --git a/internal/reporter/methods_test.go b/internal/reporter/methods_test.go index a46eef78..7321760f 100644 --- a/internal/reporter/methods_test.go +++ b/internal/reporter/methods_test.go @@ -152,7 +152,7 @@ func TestGenericMethod(t *testing.T) { err := errors.New("err connection aborted") mockTC.On("Ping", mock.Anything, mock.Anything). Return(nil, err) - pe.Call(context.Background(), mockTC) + _ = pe.Call(context.Background(), mockTC) assert.Contains(t, pe.CallSummary(), err.Error()) } diff --git a/internal/reporter/reporter_grpc_test.go b/internal/reporter/reporter_grpc_test.go index f810efdb..e9809363 100644 --- a/internal/reporter/reporter_grpc_test.go +++ b/internal/reporter/reporter_grpc_test.go @@ -74,7 +74,9 @@ func StartTestGRPCServer(t *testing.T, addr string) *TestGRPCServer { pb.RegisterTraceCollectorServer(grpcServer, testServer) require.NoError(t, err) - go grpcServer.Serve(lis) + go func() { + _ = grpcServer.Serve(lis) + }() return testServer } @@ -176,10 +178,14 @@ func (p *TestProxyServer) Start() error { } switch p.url.Scheme { case "http": - go srv.ListenAndServe() + go func() { + _ = srv.ListenAndServe() + }() p.closeFunc = closeFunc case "https": - go srv.ListenAndServeTLS(p.pemFile, p.keyFile) + go func() { + _ = srv.ListenAndServeTLS(p.pemFile, p.keyFile) + }() p.closeFunc = closeFunc // TODO: case "socks5": default: @@ -211,7 +217,9 @@ func (p *TestProxyServer) proxyHttpHandler(w http.ResponseWriter, r *http.Reques subtle.ConstantTimeCompare([]byte(pwd), []byte(expectedPwd)) != 1 { w.Header().Set("WWW-Authenticate", `Basic realm="wrong auth"`) w.WriteHeader(401) - w.Write([]byte("Unauthorised.\n")) + if _, err := w.Write([]byte("Unauthorised.\n")); err != nil { + panic(err) + } return } @@ -231,7 +239,10 @@ func (p *TestProxyServer) proxyHttpHandler(w http.ResponseWriter, r *http.Reques http.Error(w, err.Error(), http.StatusInternalServerError) return } - clientConn.Write([]byte("HTTP/1.0 200 OK\r\n\r\n")) + _, err = clientConn.Write([]byte("HTTP/1.0 200 OK\r\n\r\n")) + if err != nil { + panic(err) + } go forward(serverConn, clientConn) go forward(clientConn, serverConn) @@ -239,10 +250,10 @@ func (p *TestProxyServer) proxyHttpHandler(w http.ResponseWriter, r *http.Reques func forward(dst io.WriteCloser, src io.ReadCloser) { defer func() { - dst.Close() - src.Close() + _ = dst.Close() + _ = src.Close() }() - io.Copy(dst, src) + _, _ = io.Copy(dst, src) } func TestAppopticsCertificate(t *testing.T) { diff --git a/internal/utils/utils.go b/internal/utils/utils.go index e3389726..36e9c1f6 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -29,7 +29,10 @@ import ( // SPrintBson prints the BSON message. It's not concurrent-safe and is for testing only func SPrintBson(message []byte) string { m := make(map[string]interface{}) - bson.Unmarshal(message, m) + if err := bson.Unmarshal(message, m); err != nil { + // Since this is only used in testing/debug, we'll just return the error message + return err.Error() + } b, _ := json.MarshalIndent(m, "", " ") return string(b) } diff --git a/swo/agent.go b/swo/agent.go index 86a9e511..ac81655f 100644 --- a/swo/agent.go +++ b/swo/agent.go @@ -92,8 +92,8 @@ func SetLogOutput(w io.Writer) { } // SetServiceKey sets the service key of the agent -func SetServiceKey(key string) { - reporter.SetServiceKey(key) +func SetServiceKey(key string) error { + return reporter.SetServiceKey(key) } func createResource(resourceAttrs ...attribute.KeyValue) (*resource.Resource, error) { diff --git a/swo/agent_test.go b/swo/agent_test.go index da37c266..370254e9 100644 --- a/swo/agent_test.go +++ b/swo/agent_test.go @@ -44,11 +44,11 @@ func TestSetGetLogLevel(t *testing.T) { newLevel := GetLogLevel() assert.Equal(t, newLevel, nl) - SetLogLevel(oldLevel) + require.NoError(t, SetLogLevel(oldLevel)) } func TestShutdown(t *testing.T) { - Shutdown(context.Background()) + require.NoError(t, Shutdown(context.Background())) assert.True(t, Closed()) ctx, cancel := context.WithTimeout(context.Background(), time.Hour*24) defer cancel() @@ -58,7 +58,9 @@ func TestShutdown(t *testing.T) { func TestSetLogOutput(t *testing.T) { oldLevel := GetLogLevel() _ = SetLogLevel("DEBUG") - defer SetLogLevel(oldLevel) + defer func() { + require.NoError(t, SetLogLevel(oldLevel)) + }() var buf utils.SafeBuffer log.SetOutput(&buf)