Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci(lint): bump golangci-lint to v1.50.1 #4195

Merged
merged 1 commit into from
Jan 30, 2023
Merged

ci(lint): bump golangci-lint to v1.50.1 #4195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ draft-release:
.PHONY: install-tools
install-tools:
go install github.com/vektra/mockery/[email protected]
go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.48.0
go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.50.1
go install mvdan.cc/gofumpt@latest

.PHONY: install-ci
Expand Down
2 changes: 1 addition & 1 deletion cmd/collector/app/zipkin/http_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (aH *APIHandler) saveSpans(w http.ResponseWriter, r *http.Request) {
var tSpans []*zipkincore.Span
switch contentType {
case "application/x-thrift":
tSpans, err = zipkin.DeserializeThrift(bodyBytes)
tSpans, err = zipkin.DeserializeThrift(r.Context(), bodyBytes)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the point of passing context here? DeserializeThrift is not an IO-bound function.

Copy link
Contributor Author

@mmorel-35 mmorel-35 Jan 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First there was

Function DeserializeThrift should pass the context parameter (contextcheck)

If I just use

zipkin.DeserializeThrift(context.Background(), bodyBytes)

I get

Non-inherited new context, use function like context.WithXXX or r.Context instead (contextcheck)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the problem:

jaeger/model/converter/thrift/zipkin/deserialize.go

Lines 40 to 41 in 74d6a12

func DeserializeThrift(b []byte) ([]*zipkincore.Span, error) {
ctx := context.Background()

That's why it's flagging this specific function vs. other functions. It's a false positive because all serialization happens in-memory, without IO, but the way Thrift APIs are defined it looks like there is an IO and requires a context.

case "application/json":
tSpans, err = zipkindeser.DeserializeJSON(bodyBytes)
default:
Expand Down
5 changes: 3 additions & 2 deletions cmd/collector/app/zipkin/http_handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package zipkin
import (
"bytes"
"compress/gzip"
"context"
"crypto/rand"
"encoding/json"
"fmt"
Expand Down Expand Up @@ -109,7 +110,7 @@ func waitForSpans(t *testing.T, handler *mockZipkinHandler, expecting int) {
func TestThriftFormat(t *testing.T) {
server, _ := initializeTestServer(nil)
defer server.Close()
bodyBytes := zipkinTrift.SerializeThrift([]*zipkincore.Span{{}})
bodyBytes := zipkinTrift.SerializeThrift(context.Background(), []*zipkincore.Span{{}})
statusCode, resBodyStr, err := postBytes(server.URL+`/api/v1/spans`, bodyBytes, createHeader("application/x-thrift"))
assert.NoError(t, err)
assert.EqualValues(t, http.StatusAccepted, statusCode)
Expand Down Expand Up @@ -202,7 +203,7 @@ func TestZipkinJsonV1Format(t *testing.T) {
func TestGzipEncoding(t *testing.T) {
server, _ := initializeTestServer(nil)
defer server.Close()
bodyBytes := zipkinTrift.SerializeThrift([]*zipkincore.Span{{}})
bodyBytes := zipkinTrift.SerializeThrift(context.Background(), []*zipkincore.Span{{}})
header := createHeader("application/x-thrift")
header.Add("Content-Encoding", "gzip")
statusCode, resBodyStr, err := postBytes(server.URL+`/api/v1/spans`, gzipEncode(bodyBytes), header)
Expand Down
1 change: 1 addition & 0 deletions crossdock/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ func main() {
}
handler.xHandler.ServeHTTP(w, r)
})
//nolint:gosec
http.ListenAndServe(":8080", nil)
}

Expand Down
6 changes: 2 additions & 4 deletions model/converter/thrift/zipkin/deserialize.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ import (
)

// SerializeThrift is only used in tests.
func SerializeThrift(spans []*zipkincore.Span) []byte {
ctx := context.Background()
func SerializeThrift(ctx context.Context, spans []*zipkincore.Span) []byte {
t := thrift.NewTMemoryBuffer()
p := thrift.NewTBinaryProtocolConf(t, &thrift.TConfiguration{})
p.WriteListBegin(ctx, thrift.STRUCT, len(spans))
Expand All @@ -37,8 +36,7 @@ func SerializeThrift(spans []*zipkincore.Span) []byte {
}

// DeserializeThrift decodes Thrift bytes to a list of spans.
func DeserializeThrift(b []byte) ([]*zipkincore.Span, error) {
ctx := context.Background()
func DeserializeThrift(ctx context.Context, b []byte) ([]*zipkincore.Span, error) {
buffer := thrift.NewTMemoryBuffer()
buffer.Write(b)

Expand Down
16 changes: 10 additions & 6 deletions model/converter/thrift/zipkin/deserialize_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package zipkin

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -24,20 +25,23 @@ import (
)

func TestDeserializeWithBadListStart(t *testing.T) {
spanBytes := SerializeThrift([]*zipkincore.Span{{}})
_, err := DeserializeThrift(append([]byte{0, 255, 255}, spanBytes...))
ctx := context.Background()
spanBytes := SerializeThrift(ctx, []*zipkincore.Span{{}})
_, err := DeserializeThrift(ctx, append([]byte{0, 255, 255}, spanBytes...))
assert.Error(t, err)
}

func TestDeserializeWithCorruptedList(t *testing.T) {
spanBytes := SerializeThrift([]*zipkincore.Span{{}})
ctx := context.Background()
spanBytes := SerializeThrift(ctx, []*zipkincore.Span{{}})
spanBytes[2] = 255
_, err := DeserializeThrift(spanBytes)
_, err := DeserializeThrift(ctx, spanBytes)
assert.Error(t, err)
}

func TestDeserialize(t *testing.T) {
spanBytes := SerializeThrift([]*zipkincore.Span{{}})
_, err := DeserializeThrift(spanBytes)
ctx := context.Background()
spanBytes := SerializeThrift(ctx, []*zipkincore.Span{{}})
_, err := DeserializeThrift(ctx, spanBytes)
assert.NoError(t, err)
}
5 changes: 3 additions & 2 deletions plugin/storage/kafka/marshalling_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package kafka

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -45,7 +46,7 @@ func testMarshallerAndUnmarshaller(t *testing.T, marshaller Marshaller, unmarsha

func TestZipkinThriftUnmarshaller(t *testing.T) {
operationName := "foo"
bytes := zipkin.SerializeThrift([]*zipkincore.Span{
bytes := zipkin.SerializeThrift(context.Background(), []*zipkincore.Span{
{
ID: 12345,
Name: operationName,
Expand All @@ -62,7 +63,7 @@ func TestZipkinThriftUnmarshaller(t *testing.T) {
}

func TestZipkinThriftUnmarshallerErrorNoService(t *testing.T) {
bytes := zipkin.SerializeThrift([]*zipkincore.Span{
bytes := zipkin.SerializeThrift(context.Background(), []*zipkincore.Span{
{
ID: 12345,
Name: "foo",
Expand Down
3 changes: 2 additions & 1 deletion plugin/storage/kafka/unmarshaller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package kafka

import (
"bytes"
"context"

"github.com/gogo/protobuf/jsonpb"
"github.com/gogo/protobuf/proto"
Expand Down Expand Up @@ -69,7 +70,7 @@ func NewZipkinThriftUnmarshaller() *ZipkinThriftUnmarshaller {

// Unmarshal decodes a json byte array to a span
func (h *ZipkinThriftUnmarshaller) Unmarshal(msg []byte) (*model.Span, error) {
tSpans, err := zipkin.DeserializeThrift(msg)
tSpans, err := zipkin.DeserializeThrift(context.Background(), msg)
if err != nil {
return nil, err
}
Expand Down