From 5f0a6bef929a9117b957ccc792626a5196be543a Mon Sep 17 00:00:00 2001 From: stewartboyd119 Date: Mon, 22 Jul 2024 17:04:40 -0700 Subject: [PATCH 1/3] Updated zfmt (internal to github version) --- client.go | 2 +- client_test.go | 2 +- config.go | 2 +- config_test.go | 2 +- example/consumer/consumer.go | 2 +- example/producer/producer.go | 2 +- example/worker/bench/main.go | 2 +- example/worker/worker.go | 2 +- formatter.go | 2 +- go.mod | 4 ++-- go.sum | 8 ++++---- message.go | 2 +- message_test.go | 2 +- reader_test.go | 2 +- test/integration_test.go | 2 +- test/work_test.go | 2 +- test/writer_test.go | 2 +- testhelper.go | 2 +- testhelper_test.go | 2 +- writer_test.go | 2 +- 20 files changed, 24 insertions(+), 24 deletions(-) diff --git a/client.go b/client.go index 3791d43..6f5cdc3 100644 --- a/client.go +++ b/client.go @@ -7,7 +7,7 @@ import ( "fmt" "sync" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" + "github.com/zillow/zfmt" "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/trace" ) diff --git a/client_test.go b/client_test.go index 61d85bd..d384bc4 100644 --- a/client_test.go +++ b/client_test.go @@ -14,8 +14,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/require" + "github.com/zillow/zfmt" mock_confluent "github.com/zillow/zkafka/mocks/confluent" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/trace" "go.opentelemetry.io/otel/trace/noop" diff --git a/config.go b/config.go index a87844a..c84ffa8 100644 --- a/config.go +++ b/config.go @@ -6,7 +6,7 @@ import ( "strings" "github.com/confluentinc/confluent-kafka-go/v2/kafka" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" + "github.com/zillow/zfmt" ) const ( diff --git a/config_test.go b/config_test.go index bfaf680..f30f255 100644 --- a/config_test.go +++ b/config_test.go @@ -6,7 +6,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/require" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" + "github.com/zillow/zfmt" ) func Test_getDefaultConsumerTopicConfig(t *testing.T) { diff --git a/example/consumer/consumer.go b/example/consumer/consumer.go index 53747fe..66503b8 100644 --- a/example/consumer/consumer.go +++ b/example/consumer/consumer.go @@ -5,8 +5,8 @@ import ( "fmt" "log" + "github.com/zillow/zfmt" "github.com/zillow/zkafka" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" ) func main() { diff --git a/example/producer/producer.go b/example/producer/producer.go index 1196455..79b6a91 100644 --- a/example/producer/producer.go +++ b/example/producer/producer.go @@ -7,8 +7,8 @@ import ( "time" "github.com/google/uuid" + "github.com/zillow/zfmt" "github.com/zillow/zkafka" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" ) func main() { diff --git a/example/worker/bench/main.go b/example/worker/bench/main.go index aa1da3c..9459bad 100644 --- a/example/worker/bench/main.go +++ b/example/worker/bench/main.go @@ -10,9 +10,9 @@ import ( "time" "github.com/golang/mock/gomock" + "github.com/zillow/zfmt" "github.com/zillow/zkafka" zkafka_mocks "github.com/zillow/zkafka/mocks" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" ) func main() { diff --git a/example/worker/worker.go b/example/worker/worker.go index 6f4c803..832c42f 100644 --- a/example/worker/worker.go +++ b/example/worker/worker.go @@ -9,8 +9,8 @@ import ( "syscall" "time" + "github.com/zillow/zfmt" "github.com/zillow/zkafka" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" ) // Demonstrates reading from a topic via the zkafka.Work struct which is more convenient, typically, than using the consumer directly diff --git a/formatter.go b/formatter.go index f236367..ac08af4 100644 --- a/formatter.go +++ b/formatter.go @@ -3,7 +3,7 @@ package zkafka import ( "errors" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" + "github.com/zillow/zfmt" ) const ( diff --git a/go.mod b/go.mod index ef28870..d5dce24 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/pkg/errors v0.9.1 github.com/sony/gobreaker v1.0.0 github.com/stretchr/testify v1.9.0 - gitlab.zgtools.net/devex/archetypes/gomods/zfmt v1.0.68 + github.com/zillow/zfmt v1.0.1 go.opentelemetry.io/otel v1.27.0 go.opentelemetry.io/otel/trace v1.27.0 golang.org/x/sync v0.7.0 @@ -25,6 +25,6 @@ require ( github.com/heetch/avro v0.4.5 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect go.opentelemetry.io/otel/metric v1.27.0 // indirect - google.golang.org/protobuf v1.34.1 // indirect + google.golang.org/protobuf v1.34.2 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 3cf1ac1..165e2ff 100644 --- a/go.sum +++ b/go.sum @@ -295,8 +295,8 @@ github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQ github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= github.com/yusufpapurcu/wmi v1.2.3 h1:E1ctvB7uKFMOJw3fdOW32DwGE9I7t++CRUEMKvFoFiw= github.com/yusufpapurcu/wmi v1.2.3/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0= -gitlab.zgtools.net/devex/archetypes/gomods/zfmt v1.0.68 h1:EMTWPKIGT8Vh2JwqEbOkSFLFUsB2Cdc5gr0sn7YmUa4= -gitlab.zgtools.net/devex/archetypes/gomods/zfmt v1.0.68/go.mod h1:h8Wbct3spciNuM5Me/BccIu95vo2bpkhCIJvZx4v9sk= +github.com/zillow/zfmt v1.0.1 h1:JLN5WaxoqqoEPUpVWer83uhXhDPAA2nZkfQqgKnWp+w= +github.com/zillow/zfmt v1.0.1/go.mod h1:0PpKh4rWh+5Ghr2bbuN5UvEcqEz6PkHfE0Idgjyxy7Y= go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.45.0 h1:RsQi0qJ2imFfCvZabqzM9cNXBG8k6gXMv1A0cXRmH6A= go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.45.0/go.mod h1:vsh3ySueQCiKPxFLvjWC4Z135gIa34TQ/NSqkDTZYUM= go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace v0.45.0 h1:2ea0IkZBsWH+HA2GkD+7+hRw2u97jzdFyRtXuO14a1s= @@ -385,8 +385,8 @@ google.golang.org/genproto/googleapis/rpc v0.0.0-20240318140521-94a12d6c2237 h1: google.golang.org/genproto/googleapis/rpc v0.0.0-20240318140521-94a12d6c2237/go.mod h1:WtryC6hu0hhx87FDGxWCDptyssuo68sk10vYjF+T9fY= google.golang.org/grpc v1.62.1 h1:B4n+nfKzOICUXMgyrNd19h/I9oH0L1pizfk1d4zSgTk= google.golang.org/grpc v1.62.1/go.mod h1:IWTG0VlJLCh1SkC58F7np9ka9mx/WNkjl4PGJaiq+QE= -google.golang.org/protobuf v1.34.1 h1:9ddQBjfCyZPOHPUiPxpYESBLc+T8P3E+Vo4IbKZgFWg= -google.golang.org/protobuf v1.34.1/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= +google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg= +google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= diff --git a/message.go b/message.go index e97758b..dcad019 100644 --- a/message.go +++ b/message.go @@ -10,7 +10,7 @@ import ( "github.com/confluentinc/confluent-kafka-go/v2/kafka" "github.com/google/uuid" "github.com/pkg/errors" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" + "github.com/zillow/zfmt" ) // Message is a container for kafka message diff --git a/message_test.go b/message_test.go index d5d1f2b..d8b4372 100644 --- a/message_test.go +++ b/message_test.go @@ -8,7 +8,7 @@ import ( "github.com/confluentinc/confluent-kafka-go/v2/kafka" "github.com/stretchr/testify/require" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" + "github.com/zillow/zfmt" ) func Test_makeProducerMessageRaw(t *testing.T) { diff --git a/reader_test.go b/reader_test.go index 02beaf3..b4d536c 100644 --- a/reader_test.go +++ b/reader_test.go @@ -12,7 +12,7 @@ import ( mock_confluent "github.com/zillow/zkafka/mocks/confluent" "github.com/golang/mock/gomock" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" + "github.com/zillow/zfmt" ) func TestReader_Read_NilReturn(t *testing.T) { diff --git a/test/integration_test.go b/test/integration_test.go index acad4e6..b03375a 100644 --- a/test/integration_test.go +++ b/test/integration_test.go @@ -18,8 +18,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/uuid" "github.com/stretchr/testify/require" + "github.com/zillow/zfmt" "github.com/zillow/zkafka" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" "golang.org/x/sync/errgroup" ) diff --git a/test/work_test.go b/test/work_test.go index da3f54e..9282473 100644 --- a/test/work_test.go +++ b/test/work_test.go @@ -15,9 +15,9 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/require" + "github.com/zillow/zfmt" "github.com/zillow/zkafka" zkafka_mocks "github.com/zillow/zkafka/mocks" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" "github.com/golang/mock/gomock" ) diff --git a/test/writer_test.go b/test/writer_test.go index bbee877..4027502 100644 --- a/test/writer_test.go +++ b/test/writer_test.go @@ -11,9 +11,9 @@ import ( "github.com/confluentinc/confluent-kafka-go/v2/kafka" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" + "github.com/zillow/zfmt" "github.com/zillow/zkafka" mock_confluent "github.com/zillow/zkafka/mocks/confluent" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" ) func TestWriter_Write_LifecycleHooksCalled(t *testing.T) { diff --git a/testhelper.go b/testhelper.go index f7d18e6..7f4e350 100644 --- a/testhelper.go +++ b/testhelper.go @@ -6,7 +6,7 @@ import ( "sync" "time" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" + "github.com/zillow/zfmt" ) func GetFakeMessage(key string, value any, fmt zfmt.Formatter, doneFunc func()) *Message { diff --git a/testhelper_test.go b/testhelper_test.go index 8a87126..a955511 100644 --- a/testhelper_test.go +++ b/testhelper_test.go @@ -4,7 +4,7 @@ import ( "testing" "github.com/stretchr/testify/require" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" + "github.com/zillow/zfmt" ) func TestGetFakeMessage(t *testing.T) { diff --git a/writer_test.go b/writer_test.go index 2dda56b..0fc56b7 100644 --- a/writer_test.go +++ b/writer_test.go @@ -9,8 +9,8 @@ import ( "github.com/confluentinc/confluent-kafka-go/v2/kafka" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" + "github.com/zillow/zfmt" mock_confluent "github.com/zillow/zkafka/mocks/confluent" - "gitlab.zgtools.net/devex/archetypes/gomods/zfmt" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/trace/noop" From a6f136219613cc81933ad32fd168ea64185bfff8 Mon Sep 17 00:00:00 2001 From: stewartboyd119 Date: Mon, 22 Jul 2024 17:15:01 -0700 Subject: [PATCH 2/3] 1. Added linter 2. Fixed per linting recommendations --- .golangci.yml | 69 ++++++++++++++++++++++++++++++++++++ Makefile | 15 +++++++- client.go | 13 ++++--- example/worker/bench/main.go | 4 ++- example/worker/worker.go | 4 ++- message.go | 1 + writer.go | 6 +++- 7 files changed, 101 insertions(+), 11 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..31a7ca0 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,69 @@ +run: + skip-dirs: + - docs + - datadog + - kustomize + skip-files: + - 'wire_gen.go' + tests: false +linters-settings: + errcheck: + check-type-assertions: true + check-blank: true + gci: + sections: + - standard + - default + gosimple: + go: '1.17' + govet: + check-shadowing: true + settings: + printf: + funcs: + - (gitlab.zgtools.net/devex/archetypes/gomods/zlog.Logger).Debug + - (gitlab.zgtools.net/devex/archetypes/gomods/zlog.Logger).Info + - (gitlab.zgtools.net/devex/archetypes/gomods/zlog.Logger).Warn + - (gitlab.zgtools.net/devex/archetypes/gomods/zlog.Logger).Error + depguard: + rules: + Main: + files: + - $all + - "!$test" + deny: + - github.com/satori/go.uuid: Prefer "github.com/google/uuid" + disable-all: true + enable: + - asciicheck + - bidichk + - bodyclose + - cyclop + - decorder + - depguard + - deadcode + - dupl + - errcheck + - errchkjson + - errname + - errorlint + - exportloopref + - gci + - gocognit + - goconst + - gocritic + - gocyclo + - gofmt + - gosimple + - govet + - ineffassign + - nolintlint + - prealloc + - staticcheck + - structcheck + - typecheck + - unconvert + - unparam + - unused + - varcheck + - whitespace diff --git a/Makefile b/Makefile index 1396851..20577ca 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,6 @@ +# Directories containing independent Go modules. +MODULE_DIRS = . + .PHONY: test-no-setup test-no-setup: ./coverage.sh @@ -20,4 +23,14 @@ example-producer: .PHONY: example-worker example-worker: - go run example/worker/worker.go \ No newline at end of file + go run example/worker/worker.go + +.PHONY: lint +lint: golangci-lint + +.PHONY: golangci-lint +golangci-lint: + @$(foreach mod,$(MODULE_DIRS), \ + (cd $(mod) && \ + echo "[lint] golangci-lint: $(mod)" && \ + golangci-lint run --path-prefix $(mod) ./...) &&) true diff --git a/client.go b/client.go index 6f5cdc3..c6f0b93 100644 --- a/client.go +++ b/client.go @@ -133,17 +133,16 @@ func (c *Client) Writer(_ context.Context, topicConfig ProducerTopicConfig, opts } func getFormatter(topicConfig TopicConfig) (zfmt.Formatter, error) { - var fmtter zfmt.Formatter switch topicConfig.GetFormatter() { case CustomFmt: - fmtter = &noopFormatter{} + return &noopFormatter{}, nil default: - fmtter, _ = zfmt.GetFormatter(topicConfig.GetFormatter(), topicConfig.GetSchemaID()) - } - if fmtter == nil { - return nil, fmt.Errorf("unsupported formatter %s", topicConfig.GetFormatter()) + f, err := zfmt.GetFormatter(topicConfig.GetFormatter(), topicConfig.GetSchemaID()) + if err != nil { + return nil, fmt.Errorf("unsupported formatter %s", topicConfig.GetFormatter()) + } + return f, nil } - return fmtter, nil } // Close terminates all cached readers and writers gracefully. diff --git a/example/worker/bench/main.go b/example/worker/bench/main.go index 9459bad..eae21ba 100644 --- a/example/worker/bench/main.go +++ b/example/worker/bench/main.go @@ -48,7 +48,9 @@ func main() { ) ctx, c := context.WithTimeout(context.Background(), 2*time.Minute) defer c() - w.Run(ctx, nil) + if err := w.Run(ctx, nil); err != nil { + log.Panic(err) + } } type kafkaProcessorError struct{} diff --git a/example/worker/worker.go b/example/worker/worker.go index 832c42f..a41d314 100644 --- a/example/worker/worker.go +++ b/example/worker/worker.go @@ -61,7 +61,9 @@ func main() { // Register a processor which is executed per message. // Speedup is used to create multiple processor goroutines. Order is still maintained with this setup by way of `virtual partitions` work := wf.Create(topicConfig, &Processor{}, zkafka.Speedup(5)) - work.Run(ctx, shutdown) + if err := work.Run(ctx, shutdown); err != nil { + log.Panic(err) + } } type Processor struct{} diff --git a/message.go b/message.go index dcad019..fdaee5f 100644 --- a/message.go +++ b/message.go @@ -65,6 +65,7 @@ func makeProducerMessageRaw(_ context.Context, serviceName, topic string, key *s Key: obsKeyOriginService, Value: []byte(serviceName), }) + //nolint:errcheck // Its not particularly noteworthy if if host isn't propagated forward. We'll suppress the error hostname, _ := os.Hostname() // hn is empty string if there's an error kafkaMessage.Headers = append(kafkaMessage.Headers, kafka.Header{ diff --git a/writer.go b/writer.go index b14bea1..2224742 100644 --- a/writer.go +++ b/writer.go @@ -126,7 +126,11 @@ func (w *KWriter) WriteRaw(ctx context.Context, key *string, value []byte, opts e := <-deliveryChan w.lifecyclePostAck(ctx, begin) - m := e.(*kafka.Message) + + m, ok := e.(*kafka.Message) + if !ok { + return Response{}, errors.New("unexpected message delivered on kafka delivery channel") + } span.SetAttributes( semconv.MessagingMessageIDKey.Int64(int64(m.TopicPartition.Offset)), From c433bed61f9ad79e50d608fec0a917d8b365d1a1 Mon Sep 17 00:00:00 2001 From: stewartboyd119 Date: Mon, 22 Jul 2024 17:20:56 -0700 Subject: [PATCH 3/3] 1. Updated to use custom coverage script (integration tests off) --- Dockerfile | 16 ---------------- Makefile | 3 +-- coverage.sh | 17 +++-------------- 3 files changed, 4 insertions(+), 32 deletions(-) delete mode 100644 Dockerfile diff --git a/Dockerfile b/Dockerfile deleted file mode 100644 index f140a23..0000000 --- a/Dockerfile +++ /dev/null @@ -1,16 +0,0 @@ -FROM golang:1.22 AS build - -ENV CGO_ENABLED=1 -ENV GOPROXY=https://proxy.golang.org\|https://artifactory.zgtools.net/artifactory/api/go/devex-go\|direct -ENV GONOSUMDB=*gitlab.zgtools.net* - -WORKDIR /go/src/zkafka -COPY . . - -RUN go mod download -RUN go build -o zkafka - -FROM debian -COPY --from=build /go/src/zkafka / -ENTRYPOINT ["/zkafka"] - diff --git a/Makefile b/Makefile index 20577ca..76e8511 100644 --- a/Makefile +++ b/Makefile @@ -14,8 +14,7 @@ test-local: setup-test test-no-setup .PHONY: cover cover: - go test -v ./... -count=1 -coverprofile=cover.out -covermode atomic && \ - go tool cover -html=cover.out -o cover.html + ./coverage.sh .PHONY: example-producer example-producer: diff --git a/coverage.sh b/coverage.sh index 0934cac..bf35a51 100755 --- a/coverage.sh +++ b/coverage.sh @@ -22,11 +22,11 @@ function quit() { } # change to example directory for execution (because it uses hardcoded filepaths, and the testable # examples don't work when executed outside of that directory -go test -c -coverpkg=$pck1 -covermode=atomic -o "$root_res" -tags integration $pck1 +go test -c -coverpkg=$pck1 -covermode=atomic -o "$root_res" $pck1 # convert binary to go formatted go tool test2json -t "$root_res" -test.v -test.coverprofile "$root_out" -go test -c -coverpkg=$pck1 -covermode=atomic -o "$source_res" -tags integration $pck2 +go test -c -coverpkg=$pck1 -covermode=atomic -o "$source_res" $pck2 go tool test2json -t "$source_res" -test.v -test.coverprofile "$source_out" # delete aggregate file @@ -38,15 +38,4 @@ tail -n+2 "$source_out" >> "$omni_out" # print aggregated results go tool cover -func="$omni_out" - -# we need to create a cobertura.xml file (this is used for code coverage visualization) -# download the tool to convert gocover into covertura -go install github.com/boumenot/gocover-cobertura@latest -gocover-cobertura < $omni_out > coverage.tmp.xml -# the cobertura generated file has two issues. Its source array doesn't include the curdir and -# the class node filepaths aren't relative to the root. -# We'll run two commands -# 1. Remove the prefixed go.mod package name from filenames inside of the cobertura with a brute force replace with empty string -# 2. Add the workingdirectory to the sources array using a find replace (search for sources node, and replace with sources node but new workdir source nod) -pkg=gitlab.zgtools.net/devex/archetypes/gomods/zkafka -sed "s|$pkg/||" coverage.tmp.xml | sed "s||\n$(pwd)|"> coverage.xml \ No newline at end of file +go tool cover -html="$omni_out" -o cover.html