From 67a53f3cd83114a66007d18157675b3617bfc7bb Mon Sep 17 00:00:00 2001 From: Laurent Demailly Date: Sat, 21 Oct 2023 14:31:16 -0700 Subject: [PATCH 1/6] use fortio.org as package root, remove extra workflows --- .github/workflows/main.yml | 80 -------------- .github/workflows/manual-build.yml | 50 --------- .github/workflows/test.yml | 22 ---- .golangci.yml | 169 +++++++++++++++++++++++++++++ go.mod | 13 +-- go.sum | 25 ++--- 6 files changed, 185 insertions(+), 174 deletions(-) delete mode 100644 .github/workflows/main.yml delete mode 100644 .github/workflows/manual-build.yml delete mode 100644 .github/workflows/test.yml create mode 100644 .golangci.yml diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml deleted file mode 100644 index 3ba8cc9..0000000 --- a/.github/workflows/main.yml +++ /dev/null @@ -1,80 +0,0 @@ -name: Release - -permissions: - actions: read - contents: write - packages: write - -on: - push: - tags: - # so a vX.Y.Z-test1 doesn't trigger build - - 'v[0-9]+.[0-9]+.[0-9]+' - - 'v[0-9]+.[0-9]+.[0-9]+-pre*' - -# A workflow run is made up of one or more jobs that can run sequentially or in parallel -jobs: - # This workflow contains a single job called "build" - build: - # The type of runner that the job will run on - runs-on: ubuntu-latest - - # Steps represent a sequence of tasks that will be executed as part of the job - steps: - # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it - - uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # pin@v3 - - - name: Set up QEMU - uses: docker/setup-qemu-action@68827325e0b33c7199eb31dd4e31fbe9023e06e3 # pin@v2 - - - name: Set up Docker Buildx - id: buildx - uses: docker/setup-buildx-action@f95db51fddba0c2d1ec667646a06c2ce06100226 # pin@v2 - - - name: Available platforms - run: echo ${{ steps.buildx.outputs.platforms }} - - - name: Log in to Docker Hub - uses: docker/login-action@343f7c4344506bcbf9b4de18042ae17996df046d # pin@v2 - with: - username: ${{ secrets.DOCKER_USER }} - password: ${{ secrets.DOCKER_TOKEN }} - - - name: Build - id: build - run: | - make info - make release - VERSION=$(make echo-version) - echo "VERSION=${VERSION}" >> $GITHUB_ENV - PACKAGE_VERSION=$(make echo-package-version) - echo "Version $VERSION, Package version $PACKAGE_VERSION" - - - name: Build and push Docker image - uses: docker/build-push-action@0565240e2d4ab88bba5387d719585280857ece09 # pin@v3 - with: - context: . - platforms: linux/amd64,linux/arm64,linux/ppc64le,linux/s390x - push: true - tags: fortio/fortio:${{ env.VERSION }}, fortio/fortio:latest - - - name: Create Release - id: create_release - # Need to find a replacement not using set-output - uses: actions/create-release@0cb9c9b65d5d1901c1f53e5e66eaf4afd303e70e # pin@v1 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # This token is provided by Actions, you do not need to create your own token - with: - tag_name: ${{ github.ref }} - release_name: Fortio ${{ env.VERSION }} - draft: true - - - name: Upload release artifacts - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - tag_name="${GITHUB_REF##*/}" - echo "will use tag_name=$tag_name" - # tends to fail and not find the release somehow; add a small sleep... (yuck) - sleep 10 - gh release upload "${tag_name}" release/*.{tgz,zip,rpm,deb,gz} diff --git a/.github/workflows/manual-build.yml b/.github/workflows/manual-build.yml deleted file mode 100644 index 7ad86fc..0000000 --- a/.github/workflows/manual-build.yml +++ /dev/null @@ -1,50 +0,0 @@ -name: Manual Release Update/Rebuild - -permissions: - actions: read - contents: write - packages: write - -on: - workflow_dispatch: - inputs: - tag: - description: 'Tag to rebuild' - required: true - -jobs: - build: - runs-on: ubuntu-latest - - steps: - - name: test - run: | - echo "tag is ${{ github.event.inputs.tag }}" - - - uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # pin@v3 - with: - ref: ${{ github.event.inputs.tag }} - - - name: Set up QEMU - uses: docker/setup-qemu-action@68827325e0b33c7199eb31dd4e31fbe9023e06e3 # pin@v2 - - - name: Set up Docker Buildx - id: buildx - uses: docker/setup-buildx-action@f95db51fddba0c2d1ec667646a06c2ce06100226 # pin@v2 - - - name: Build - id: build - run: | - make info - make release - VERSION=$(make echo-version) - PACKAGE_VERSION=$(make echo-package-version) - echo "Version $VERSION, Package version $PACKAGE_VERSION" - - - name: Upload release artifacts - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - tag_name="${{ github.event.inputs.tag }}" - echo "tag_name=$tag_name" - gh release upload "${tag_name}" release/*.{tgz,zip,rpm,deb,gz} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml deleted file mode 100644 index 4948c15..0000000 --- a/.github/workflows/test.yml +++ /dev/null @@ -1,22 +0,0 @@ - -name: Test Tags - -permissions: {} - -on: - push: - tags: - - 'v[0-9]+.[0-9]+.[0-9]+-test*' - -# A workflow run is made up of one or more jobs that can run sequentially or in parallel -jobs: - # This workflow contains a single job called "build" - build: - # The type of runner that the job will run on - runs-on: ubuntu-latest - - # Steps represent a sequence of tasks that will be executed as part of the job - steps: - # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it - - name: Just a test step - run: echo it works diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..3847205 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,169 @@ +# Config for golangci-lint + +# output configuration options + +# all available settings of specific linters +linters-settings: + gocritic: + disabled-checks: + - ifElseChain + dupl: + # tokens count to trigger issue, 150 by default + threshold: 100 + exhaustive: + # indicates that switch statements are to be considered exhaustive if a + # 'default' case is present, even if all enum members aren't listed in the + # switch + default-signifies-exhaustive: false + funlen: + lines: 140 + statements: 70 + gocognit: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 42 + nestif: + # minimal complexity of if statements to report, 5 by default + min-complexity: 4 + gocyclo: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 30 + godot: + # check all top-level comments, not only declarations + check-all: false + govet: + # report about shadowed variables + check-shadowing: true + # settings per analyzer + settings: + printf: # analyzer name, run `go tool vet help` to see all analyzers + funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Printf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).FErrf + enable-all: true + disable-all: false + lll: + # max line length, lines longer will be reported. Default is 120. + # '\t' is counted as 1 character by default, and can be changed with the tab-width option + line-length: 132 + # tab width in spaces. Default to 1. + tab-width: 1 + misspell: + # Correct spellings using locale preferences for US or UK. + # Default is to use a neutral variety of English. + # Setting locale to US will correct the British spelling of 'colour' to 'color'. + locale: US + ignore-words: + - fortio + nakedret: + # make an issue if func has more lines of code than this setting and it has naked returns; default is 30 + max-func-lines: 30 + nolintlint: + require-specific: true + whitespace: + multi-if: false # Enforces newlines (or comments) after every multi-line if statement + multi-func: false # Enforces newlines (or comments) after every multi-line function signature + gofumpt: + # Choose whether or not to use the extra rules that are disabled + # by default + extra-rules: false + + +linters: + disable: + # bad ones: + - musttag + # Deprecated ones: + - scopelint + - golint + - interfacer + - maligned + - varcheck + - structcheck + - nosnakecase + - deadcode + # Weird/bad ones: + - wsl + - nlreturn + - gochecknoinits + - gochecknoglobals + - gomnd + - testpackage + - wrapcheck + - exhaustivestruct + - tagliatelle + - nonamedreturns + - varnamelen + - exhaustruct # seems like a good idea at first but actually a pain and go does have zero values for a reason. +# TODO consider putting these back, when they stop being bugged (ifshort, wastedassign,...) + - paralleltest + - thelper + - forbidigo + - ifshort + - wastedassign + - cyclop + - forcetypeassert + - ireturn + - depguard + enable-all: true + disable-all: false + # Must not use fast: true in newer golangci-lint or it'll just skip a bunch of linter instead of doing caching like before (!) + fast: false + + +issues: + # Excluding configuration per-path, per-linter, per-text and per-source + exclude-rules: + # Exclude some linters from running on tests files. + - path: _test\.go + linters: + - gocyclo + - errcheck + - dupl + - gosec + - gochecknoinits + - gochecknoglobals + - forcetypeassert + - nosnakecase + - noctx + + # Exclude lll issues for long lines with go:generate + - linters: + - lll + source: "^//go:generate " + - linters: + - goerr113 + text: "do not define dynamic errors" + - linters: + - govet + text: "fieldalignment:" + - linters: + - godox + text: "TODO" + - linters: + - nosnakecase + text: "grpc_|_SERVING|O_" + + # Maximum issues count per one linter. Set to 0 to disable. Default is 50. + max-issues-per-linter: 0 + + # Maximum count of issues with the same text. Set to 0 to disable. Default is 3. + max-same-issues: 0 + +severity: + # Default value is empty string. + # Set the default severity for issues. If severity rules are defined and the issues + # do not match or no severity is provided to the rule this will be the default + # severity applied. Severities should match the supported severity names of the + # selected out format. + # - Code climate: https://docs.codeclimate.com/docs/issues#issue-severity + # - Checkstyle: https://checkstyle.sourceforge.io/property_types.html#severity + # - Github: https://help.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-error-message + default-severity: error + + # The default value is false. + # If set to true severity-rules regular expressions become case sensitive. + case-sensitive: false diff --git a/go.mod b/go.mod index cc93aed..828122f 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/wiardvanrij/slack-proxy +module fortio.org/slack-proxy go 1.20 @@ -11,12 +11,11 @@ require ( require ( github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.2.0 // indirect - github.com/golang/protobuf v1.5.3 // indirect - github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect - github.com/prometheus/client_model v0.4.1-0.20230718164431-9a2bf3000d16 // indirect - github.com/prometheus/common v0.44.0 // indirect - github.com/prometheus/procfs v0.11.1 // indirect + github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // indirect + github.com/prometheus/client_model v0.5.0 // indirect + github.com/prometheus/common v0.45.0 // indirect + github.com/prometheus/procfs v0.12.0 // indirect go.uber.org/multierr v1.11.0 // indirect - golang.org/x/sys v0.11.0 // indirect + golang.org/x/sys v0.13.0 // indirect google.golang.org/protobuf v1.31.0 // indirect ) diff --git a/go.sum b/go.sum index b8c49d9..a02d2ac 100644 --- a/go.sum +++ b/go.sum @@ -3,37 +3,32 @@ github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6r github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44= github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= -github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= -github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= -github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= -github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zkfA9PSy5pEvNWRP0ET0TIVo= -github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4= +github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 h1:jWpvCLoY8Z/e3VKvlsiIGKtc+UG6U5vzxaoagmhXfyg= +github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0/go.mod h1:QUyp042oQthUoa9bqDv0ER0wrtXnBruoNd7aNjkbP+k= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/prometheus/client_golang v1.17.0 h1:rl2sfwZMtSthVU752MqfjQozy7blglC+1SOtjMAMh+Q= github.com/prometheus/client_golang v1.17.0/go.mod h1:VeL+gMmOAxkS2IqfCq0ZmHSL+LjWfWDUmp1mBz9JgUY= -github.com/prometheus/client_model v0.4.1-0.20230718164431-9a2bf3000d16 h1:v7DLqVdK4VrYkVD5diGdl4sxJurKJEMnODWRJlxV9oM= -github.com/prometheus/client_model v0.4.1-0.20230718164431-9a2bf3000d16/go.mod h1:oMQmHW1/JoDwqLtg57MGgP/Fb1CJEYF2imWWhWtMkYU= -github.com/prometheus/common v0.44.0 h1:+5BrQJwiBB9xsMygAB3TNvpQKOwlkc25LbISbrdOOfY= -github.com/prometheus/common v0.44.0/go.mod h1:ofAIvZbQ1e/nugmZGz4/qCb9Ap1VoSTIO7x0VV9VvuY= -github.com/prometheus/procfs v0.11.1 h1:xRC8Iq1yyca5ypa9n1EZnWZkt7dwcoRPQwX/5gwaUuI= -github.com/prometheus/procfs v0.11.1/go.mod h1:eesXgaPo1q7lBpVMoMy0ZOFTth9hBn4W/y0/p/ScXhY= +github.com/prometheus/client_model v0.5.0 h1:VQw1hfvPvk3Uv6Qf29VrPF32JB6rtbgI6cYPYQjL0Qw= +github.com/prometheus/client_model v0.5.0/go.mod h1:dTiFglRmd66nLR9Pv9f0mZi7B7fk5Pm3gvsjB5tr+kI= +github.com/prometheus/common v0.45.0 h1:2BGz0eBc2hdMDLnO/8n0jeB3oPrt2D08CekT0lneoxM= +github.com/prometheus/common v0.45.0/go.mod h1:YJmSTw9BoKxJplESWWxlbyttQR4uaEcGyv9MZjVOJsY= +github.com/prometheus/procfs v0.12.0 h1:jluTpSng7V9hY0O2R9DzzJHYb2xULk9VTR1V1R/k6Bo= +github.com/prometheus/procfs v0.12.0/go.mod h1:pcuDEFsWDnvcgNzo4EEweacyhjeA9Zk3cnaOZAZEfOo= github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= go.uber.org/zap v1.26.0 h1:sI7k6L95XOKS281NhVKOFCUNIvv9e0w4BF8N3u+tCRo= go.uber.org/zap v1.26.0/go.mod h1:dtElttAiwGvoJ/vj4IwHBS/gXsEu/pZ50mUIRWuG0so= -golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM= -golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= +golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4= golang.org/x/time v0.3.0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= -google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= google.golang.org/protobuf v1.31.0 h1:g0LDEJHgrBl9N9r17Ru3sqWhkIx2NB67okBHPwC7hs8= google.golang.org/protobuf v1.31.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= From 5a47c10a3d3d0a94de0fbe02821c43dedd199e9e Mon Sep 17 00:00:00 2001 From: Laurent Demailly Date: Sat, 21 Oct 2023 15:26:08 -0700 Subject: [PATCH 2/6] switch to scli and fortio logger, fix a couple of typos along the way --- app.go | 28 +++++++++++----------------- app_test.go | 22 +++++----------------- go.mod | 10 ++++++++-- go.sum | 26 ++++++++++++++++++-------- main.go | 37 +++++++++++++++---------------------- metrics.go | 4 ++-- server.go | 2 +- 7 files changed, 60 insertions(+), 69 deletions(-) diff --git a/app.go b/app.go index 0ef4ec7..782b5bd 100644 --- a/app.go +++ b/app.go @@ -6,12 +6,12 @@ import ( "context" "encoding/json" "fmt" - "io/ioutil" + "io" "math" "net/http" "time" - "go.uber.org/zap" + "fortio.org/log" "golang.org/x/time/rate" ) @@ -138,7 +138,7 @@ func (s *SlackClient) PostMessage(request SlackPostMessageRequest, url string, t } defer resp.Body.Close() - body, err := ioutil.ReadAll(resp.Body) + body, err := io.ReadAll(resp.Body) if err != nil { return err } @@ -157,15 +157,9 @@ func (s *SlackClient) PostMessage(request SlackPostMessageRequest, url string, t func NewApp(queueSize int, httpClient *http.Client, metrics *Metrics) *App { - logger, err := zap.NewProduction() - if err != nil { - panic("failed to initialize logger: " + err.Error()) - } - return &App{ slackQueue: make(chan SlackPostMessageRequest, queueSize), messenger: &SlackClient{client: httpClient}, - logger: logger, metrics: metrics, } } @@ -193,7 +187,7 @@ func (app *App) processQueue(ctx context.Context, MaxRetries int, InitialBackoff if !ok { return } - app.logger.Debug("Got message from queue", zap.Any("message", msg)) + log.S(log.Debug, "Got message from queue", log.Any("message", msg)) // Update the queue size metric after any change on the queue size app.metrics.QueueSize.With(nil).Set(float64(len(app.slackQueue))) @@ -207,7 +201,7 @@ func (app *App) processQueue(ctx context.Context, MaxRetries int, InitialBackoff // Remove the channel from the map, so that we can process it again. If the channel isn't created in the meantime, we will just add it again. delete(doNotProcessChannels, msg.Channel) } else { - app.logger.Info("Channel is on the doNotProcess list, not trying to post this message", zap.String("channel", msg.Channel)) + log.S(log.Info, "Channel is on the doNotProcess list, not trying to post this message", log.String("channel", msg.Channel)) app.metrics.RequestsNotProcessed.WithLabelValues(msg.Channel).Inc() break } @@ -219,21 +213,21 @@ func (app *App) processQueue(ctx context.Context, MaxRetries int, InitialBackoff retryable, pause, description := CheckError(err.Error(), msg.Channel) if pause { - app.logger.Info("Channel not found, pausing for 15 minutes", zap.String("channel", msg.Channel)) + log.S(log.Info, "Channel not found, pausing for 15 minutes", log.String("channel", msg.Channel)) app.metrics.RequestsNotProcessed.WithLabelValues(msg.Channel).Inc() break } if !retryable { app.metrics.RequestsFailedTotal.WithLabelValues(msg.Channel).Inc() - app.logger.Error("Permanent error, message will not be retried", zap.Error(err), zap.String("description", description), zap.String("channel", msg.Channel), zap.Any("message", msg)) + log.S(log.Error, "Permanent error, message will not be retried", log.Any("err", err), log.String("description", description), log.String("channel", msg.Channel), log.Any("message", msg)) break } if description == "Unknown error" { - app.logger.Error("Unknown error, since we can't infer what type of error it is, we will retry it. However, please create a ticket/issue for this project for this error", zap.Error(err)) + log.S(log.Error, "Unknown error, since we can't infer what type of error it is, we will retry it. However, please create a ticket/issue for this project for this error", log.Any("err", err)) } - app.logger.Warn("Temporary error, message will be retried", zap.Error(err), zap.String("description", description), zap.String("channel", msg.Channel), zap.Any("message", msg)) + log.S(log.Warning, "Temporary error, message will be retried", log.Any("err", err), log.String("description", description), log.String("channel", msg.Channel), log.Any("message", msg)) app.metrics.RequestsRetriedTotal.WithLabelValues(msg.Channel).Inc() @@ -242,12 +236,12 @@ func (app *App) processQueue(ctx context.Context, MaxRetries int, InitialBackoff backoffDuration := time.Duration(InitialBackoffMs*int(math.Pow(2, float64(retryCount-1)))) * time.Millisecond time.Sleep(backoffDuration) } else { - app.logger.Error("Message failed after retries", zap.Error(err), zap.Int("retryCount", retryCount)) + log.S(log.Error, "Message failed after retries", log.Any("err", err), log.Int("retryCount", retryCount)) app.metrics.RequestsFailedTotal.WithLabelValues(msg.Channel).Inc() break } } else { - app.logger.Debug("Message sent successfully") + log.Debugf("Message sent successfully") app.metrics.RequestsSucceededTotal.WithLabelValues(msg.Channel).Inc() break } diff --git a/app_test.go b/app_test.go index 1f769ad..bc5c2a3 100644 --- a/app_test.go +++ b/app_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "go.uber.org/zap" + "fortio.org/log" ) type MockSlackMessenger struct { @@ -25,16 +25,10 @@ func (m *MockSlackMessenger) PostMessage(req SlackPostMessageRequest, url string func TestApp_singleBurst_Success(t *testing.T) { - logger, err := zap.NewDevelopment() - if err != nil { - panic("failed to initialize logger: " + err.Error()) - } - messenger := &MockSlackMessenger{} app := &App{ slackQueue: make(chan SlackPostMessageRequest, 2), messenger: messenger, - logger: logger, } ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) @@ -52,14 +46,14 @@ func TestApp_singleBurst_Success(t *testing.T) { } } - app.logger.Debug("Posting messages done") + log.S(log.Debug,"Posting messages done") app.wg.Wait() endTime := time.Now() diffInSeconds := endTime.Sub(startTime).Seconds() - app.logger.Debug("diffInSeconds", zap.Float64("diffInSeconds", diffInSeconds)) + log.S(log.Debug,"diffInSeconds", log.Float64("diffInSeconds", diffInSeconds)) // The sum is always: (Amount of messages * delay in seconds) minus burst. In this case 10 * 1 - 1 = 9 seconds. if math.RoundToEven(diffInSeconds) != 9 { @@ -69,16 +63,10 @@ func TestApp_singleBurst_Success(t *testing.T) { func TestApp_MultiBurst_Success(t *testing.T) { - logger, err := zap.NewDevelopment() - if err != nil { - panic("failed to initialize logger: " + err.Error()) - } - messenger := &MockSlackMessenger{} app := &App{ slackQueue: make(chan SlackPostMessageRequest, 2), messenger: messenger, - logger: logger, } ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) @@ -96,14 +84,14 @@ func TestApp_MultiBurst_Success(t *testing.T) { } } - app.logger.Debug("Posting messages done") + log.S(log.Debug,"Posting messages done") app.wg.Wait() endTime := time.Now() diffInSeconds := endTime.Sub(startTime).Seconds() - app.logger.Debug("diffInSeconds", zap.Float64("diffInSeconds", diffInSeconds)) + log.S(log.Debug,"diffInSeconds", log.Float64("diffInSeconds", diffInSeconds)) // The sum is always: (Amount of messages * delay in seconds) minus burst. In this case 20 * 1 - 10 = 10 seconds. if math.RoundToEven(diffInSeconds) != 10 { diff --git a/go.mod b/go.mod index 828122f..50aff63 100644 --- a/go.mod +++ b/go.mod @@ -3,19 +3,25 @@ module fortio.org/slack-proxy go 1.20 require ( + fortio.org/log v1.11.0 + fortio.org/scli v1.12.0 github.com/prometheus/client_golang v1.17.0 - go.uber.org/zap v1.26.0 golang.org/x/time v0.3.0 ) require ( + fortio.org/cli v1.4.2 // indirect + fortio.org/dflag v1.6.0 // indirect + fortio.org/sets v1.0.3 // indirect + fortio.org/version v1.0.3 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.2.0 // indirect + github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // indirect github.com/prometheus/client_model v0.5.0 // indirect github.com/prometheus/common v0.45.0 // indirect github.com/prometheus/procfs v0.12.0 // indirect - go.uber.org/multierr v1.11.0 // indirect + golang.org/x/exp v0.0.0-20231006140011-7918f672742d // indirect golang.org/x/sys v0.13.0 // indirect google.golang.org/protobuf v1.31.0 // indirect ) diff --git a/go.sum b/go.sum index a02d2ac..0e3c409 100644 --- a/go.sum +++ b/go.sum @@ -1,14 +1,28 @@ +fortio.org/assert v1.2.0 h1:XscfvR8yp4xW7OMCvNbCsieRFDxlwdEcb69+JZRp6LA= +fortio.org/cli v1.4.2 h1:pCYVPk5FpQuJtD31f9CG8sdgoTOWdv9Gls8As/MLJhU= +fortio.org/cli v1.4.2/go.mod h1:VGqFDEKpA6u3NbTM3aSz2lZfAYKnGaszl1pLxbBhxsY= +fortio.org/dflag v1.6.0 h1:acOc3KAu1kgp3Ov1nT2GbmcuNhPF1oMZqhT9nj3Oz1U= +fortio.org/dflag v1.6.0/go.mod h1:MeDNYnBh30L6OdMGgAL6ltDX2xdQjOiexCZYPYXH8uM= +fortio.org/log v1.11.0 h1:w7ueGPGbXz0A3+ApMz/5Q9gwEMrwSo/ohTlLo2Um6dU= +fortio.org/log v1.11.0/go.mod h1:u/8/2lyczXq52aT5Nw6reD+3cR6m/EbS2jBiIYhgiTU= +fortio.org/scli v1.12.0 h1:ABomNd0/RLgdGdf5lwgfwGhTuLwW3Of7bbTTgbN9r+g= +fortio.org/scli v1.12.0/go.mod h1:kLbbvX0QdnUOpQnW+7OsmY1d/qf5VMMkOiR+8GhqJII= +fortio.org/sets v1.0.3 h1:HzewdGjH69YmyW06yzplL35lGr+X4OcqQt0qS6jbaO4= +fortio.org/sets v1.0.3/go.mod h1:QZVj0r6KP/ZD9ebySW9SgxVNy/NjghUfyHW9NN+WU+4= +fortio.org/version v1.0.3 h1:5gJ3plj6isAOMq52cI5ifo4cC+QHmJF76Wevc5Cp4x0= +fortio.org/version v1.0.3/go.mod h1:2JQp9Ax+tm6QKiGuzR5nJY63kFeANcgrZ0osoQFDVm0= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44= github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY= +github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 h1:jWpvCLoY8Z/e3VKvlsiIGKtc+UG6U5vzxaoagmhXfyg= github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0/go.mod h1:QUyp042oQthUoa9bqDv0ER0wrtXnBruoNd7aNjkbP+k= -github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/prometheus/client_golang v1.17.0 h1:rl2sfwZMtSthVU752MqfjQozy7blglC+1SOtjMAMh+Q= github.com/prometheus/client_golang v1.17.0/go.mod h1:VeL+gMmOAxkS2IqfCq0ZmHSL+LjWfWDUmp1mBz9JgUY= github.com/prometheus/client_model v0.5.0 h1:VQw1hfvPvk3Uv6Qf29VrPF32JB6rtbgI6cYPYQjL0Qw= @@ -17,12 +31,9 @@ github.com/prometheus/common v0.45.0 h1:2BGz0eBc2hdMDLnO/8n0jeB3oPrt2D08CekT0lne github.com/prometheus/common v0.45.0/go.mod h1:YJmSTw9BoKxJplESWWxlbyttQR4uaEcGyv9MZjVOJsY= github.com/prometheus/procfs v0.12.0 h1:jluTpSng7V9hY0O2R9DzzJHYb2xULk9VTR1V1R/k6Bo= github.com/prometheus/procfs v0.12.0/go.mod h1:pcuDEFsWDnvcgNzo4EEweacyhjeA9Zk3cnaOZAZEfOo= -github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= -go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk= -go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= -go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= -go.uber.org/zap v1.26.0 h1:sI7k6L95XOKS281NhVKOFCUNIvv9e0w4BF8N3u+tCRo= -go.uber.org/zap v1.26.0/go.mod h1:dtElttAiwGvoJ/vj4IwHBS/gXsEu/pZ50mUIRWuG0so= +golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= +golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= +golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4= @@ -31,4 +42,3 @@ golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8T google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.31.0 h1:g0LDEJHgrBl9N9r17Ru3sqWhkIx2NB67okBHPwC7hs8= google.golang.org/protobuf v1.31.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= -gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/main.go b/main.go index 2e934c4..285d971 100644 --- a/main.go +++ b/main.go @@ -6,17 +6,15 @@ import ( "encoding/json" "flag" "net/http" - "os" - "os/signal" "sync" - "syscall" + "fortio.org/log" + "fortio.org/scli" "github.com/prometheus/client_golang/prometheus" - "go.uber.org/zap" ) type Metrics struct { - RequestsRecievedTotal *prometheus.CounterVec + RequestsReceivedTotal *prometheus.CounterVec RequestsFailedTotal *prometheus.CounterVec RequestsRetriedTotal *prometheus.CounterVec RequestsSucceededTotal *prometheus.CounterVec @@ -48,7 +46,6 @@ type App struct { slackQueue chan SlackPostMessageRequest wg sync.WaitGroup messenger SlackMessenger - logger *zap.Logger metrics *Metrics } @@ -74,7 +71,8 @@ func main() { flag.StringVar(&tokenFlag, "token", "", "Bearer token for the Slack API") flag.StringVar(&MetricsPort, "metricsPort", MetricsPort, "Port for the metrics server") flag.StringVar(&ApplicationPort, "applicationPort", ApplicationPort, "Port for the application server") - flag.Parse() + + scli.ServerMain() // Initialize metrics r := prometheus.NewRegistry() @@ -84,11 +82,10 @@ func main() { app := NewApp(maxQueueSize, &http.Client{}, metrics) // The only required flag is the token at the moment. if tokenFlag == "" { - app.logger.Fatal("Missing token flag") + log.Fatalf("Missing token flag") } - app.logger.Info("Starting up...") - app.logger.Info("Starting metrics server.") + log.Infof("Starting metrics server.") go StartMetricServer(r, &MetricsPort) // Main ctx @@ -99,22 +96,18 @@ func main() { serverCtx, serverCancel := context.WithCancel(context.Background()) defer serverCancel() - app.logger.Info("Starting main app logic") + log.Infof("Starting main app logic") go app.processQueue(ctx, MaxRetries, InitialBackoffMs, SlackPostMessageURL, tokenFlag, burst) - app.logger.Info("Starting reciever server") + log.Infof("Starting receiver server") go app.StartServer(serverCtx, &ApplicationPort) - app.logger.Info("Up and running!") - - // Wait for a shutdown signal - sigChan := make(chan os.Signal, 1) - signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM) - <-sigChan + log.Infof("Up and running!") - app.logger.Info("Shutdown signal received. Cleaning up...") - app.logger.Info("Shutting down server...") + // Shutdown is handled by scli + scli.UntilInterrupted() + log.Infof("Shutting down server...") serverCancel() - app.logger.Info("Shutting down queue...") + log.Infof("Shutting down queue...") app.Shutdown() - app.logger.Info("Shutdown complete.") + log.Infof("Shutdown complete.") } diff --git a/metrics.go b/metrics.go index 7acd4b9..b64ebfa 100644 --- a/metrics.go +++ b/metrics.go @@ -11,7 +11,7 @@ import ( func NewMetrics(reg prometheus.Registerer) *Metrics { m := &Metrics{ - RequestsRecievedTotal: prometheus.NewCounterVec( + RequestsReceivedTotal: prometheus.NewCounterVec( prometheus.CounterOpts{ Namespace: "slackproxy", Name: "requests_recieved_total", @@ -61,7 +61,7 @@ func NewMetrics(reg prometheus.Registerer) *Metrics { ), } - reg.MustRegister(m.RequestsRecievedTotal) + reg.MustRegister(m.RequestsReceivedTotal) reg.MustRegister(m.RequestsFailedTotal) reg.MustRegister(m.RequestsRetriedTotal) reg.MustRegister(m.RequestsSucceededTotal) diff --git a/server.go b/server.go index 03c63c0..664316c 100644 --- a/server.go +++ b/server.go @@ -61,7 +61,7 @@ func (app *App) StartServer(ctx context.Context, applicationPort *string) { return } - app.metrics.RequestsRecievedTotal.WithLabelValues(request.Channel).Inc() + app.metrics.RequestsReceivedTotal.WithLabelValues(request.Channel).Inc() responseData, err := json.Marshal(fakeSlackResponse) if err != nil { From 5311433e590b3a16c75374d78615771fb960e265 Mon Sep 17 00:00:00 2001 From: Laurent Demailly Date: Sat, 21 Oct 2023 16:10:19 -0700 Subject: [PATCH 3/6] fixing some compile and lint errors --- app.go | 7 +------ app_test.go | 2 -- main.go | 1 - metrics.go | 7 ++----- server.go | 6 +++--- server_test.go | 1 - 6 files changed, 6 insertions(+), 18 deletions(-) diff --git a/app.go b/app.go index 7724072..f8fd7d6 100644 --- a/app.go +++ b/app.go @@ -91,7 +91,6 @@ var slackRetryErrors = map[string]string{ var doNotProcessChannels = map[string]time.Time{} func CheckError(err string, channel string) (retryable bool, pause bool, description string) { - // Special case for channel_not_found, we don't want to retry this one right away. // We are making it a 'soft failure' so that we don't keep retrying it for a period of time for any message that is sent to a channel that doesn't exist. // We keep track of said channel in a map, and we will retry it after a period of time. @@ -112,7 +111,6 @@ func CheckError(err string, channel string) (retryable bool, pause bool, descrip // This should not happen, but if it does, we just try to retry it return true, false, "Unknown error" - } func (s *SlackClient) PostMessage(request SlackPostMessageRequest, url string, token string) error { @@ -151,7 +149,6 @@ func (s *SlackClient) PostMessage(request SlackPostMessageRequest, url string, t } func NewApp(queueSize int, httpClient *http.Client, metrics *Metrics) *App { - return &App{ slackQueue: make(chan SlackPostMessageRequest, queueSize), messenger: &SlackClient{client: httpClient}, @@ -186,7 +183,7 @@ func (app *App) processQueue(ctx context.Context, MaxRetries int, InitialBackoff // On shutdown, it would cancel the context, even if the queue was stopped (thus no messages would even come in). err := r.Wait(ctx) if err != nil { - log.Fatalf("Error while waiting for rate limiter. This should not happen, provide debug info + error message to an issue if it does: %w", err) + log.Fatalf("Error while waiting for rate limiter. This should not happen, provide debug info + error message to an issue if it does: %v", err) return } @@ -195,7 +192,6 @@ func (app *App) processQueue(ctx context.Context, MaxRetries int, InitialBackoff retryCount := 0 for { - // Check if the channel is in the doNotProcessChannels map, if it is, check if it's been more than 15 minutes since we last tried to send a message to it. if (doNotProcessChannels[msg.Channel] != time.Time{}) { if time.Since(doNotProcessChannels[msg.Channel]) >= 15*time.Minute { @@ -210,7 +206,6 @@ func (app *App) processQueue(ctx context.Context, MaxRetries int, InitialBackoff err := app.messenger.PostMessage(msg, SlackPostMessageURL, tokenFlag) if err != nil { - retryable, pause, description := CheckError(err.Error(), msg.Channel) if pause { diff --git a/app_test.go b/app_test.go index 23ca0ae..331fd26 100644 --- a/app_test.go +++ b/app_test.go @@ -24,7 +24,6 @@ func (m *MockSlackMessenger) PostMessage(req SlackPostMessageRequest, url string } func TestApp_singleBurst_Success(t *testing.T) { - messenger := &MockSlackMessenger{} app := &App{ slackQueue: make(chan SlackPostMessageRequest, 2), @@ -62,7 +61,6 @@ func TestApp_singleBurst_Success(t *testing.T) { } func TestApp_MultiBurst_Success(t *testing.T) { - messenger := &MockSlackMessenger{} app := &App{ slackQueue: make(chan SlackPostMessageRequest, 2), diff --git a/main.go b/main.go index 285d971..acb6353 100644 --- a/main.go +++ b/main.go @@ -50,7 +50,6 @@ type App struct { } func main() { - var ( MaxRetries = 2 InitialBackoffMs = 1000 diff --git a/metrics.go b/metrics.go index b64ebfa..d725cf5 100644 --- a/metrics.go +++ b/metrics.go @@ -9,13 +9,12 @@ import ( ) func NewMetrics(reg prometheus.Registerer) *Metrics { - m := &Metrics{ RequestsReceivedTotal: prometheus.NewCounterVec( prometheus.CounterOpts{ Namespace: "slackproxy", - Name: "requests_recieved_total", - Help: "The total number of requests recieved", + Name: "requests_received_total", + Help: "The total number of requests received", }, []string{"channel"}, ), @@ -69,11 +68,9 @@ func NewMetrics(reg prometheus.Registerer) *Metrics { reg.MustRegister(m.QueueSize) return m - } func StartMetricServer(reg *prometheus.Registry, addr *string) { - http.Handle("/metrics", promhttp.HandlerFor( reg, promhttp.HandlerOpts{ diff --git a/server.go b/server.go index 713fb03..7d75515 100644 --- a/server.go +++ b/server.go @@ -51,8 +51,8 @@ func (app *App) handleRequest(w http.ResponseWriter, r *http.Request) { maxQueueSize := int(float64(cap(app.slackQueue)) * 0.9) // Reject requests if the queue is almost full // If the channel is full, the request will block until there is space in the channel. - // Ideally we don't reject at 90%, but initialy after some tests I got blocked. So I decided to be a bit more conservative. - // ToDo: Fix this behaviour so we can reach 100% channel size without problems. + // Ideally we don't reject at 90%, but initially after some tests I got blocked. So I decided to be a bit more conservative. + // ToDo: Fix this behavior so we can reach 100% channel size without problems. if len(app.slackQueue) >= maxQueueSize { w.WriteHeader(http.StatusServiceUnavailable) @@ -116,7 +116,7 @@ func (app *App) handleRequest(w http.ResponseWriter, r *http.Request) { // Respond, this is not entirely accurate as we have no idea if the message will be processed successfully. // This is the downside of having a queue which could potentially delay responses by a lot. - // We do our due diligences on the recieved message and can make a fair assumption we will be able to process it. + // We do our due diligences on the received message and can make a fair assumption we will be able to process it. // Application should utlise this applications metrics and logs to find out if there are any issues. w.WriteHeader(http.StatusOK) _, err = w.Write(responseData) diff --git a/server_test.go b/server_test.go index a0cbf8b..50e307b 100644 --- a/server_test.go +++ b/server_test.go @@ -44,7 +44,6 @@ func TestHandleRequest(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := prometheus.NewRegistry() metrics := NewMetrics(r) From 5b1359929ba5ea6144daa4f5853ac61eabdb2f68 Mon Sep 17 00:00:00 2001 From: Laurent Demailly Date: Sat, 21 Oct 2023 16:14:41 -0700 Subject: [PATCH 4/6] fix merge miss --- app_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app_test.go b/app_test.go index 331fd26..d44d9e6 100644 --- a/app_test.go +++ b/app_test.go @@ -10,6 +10,7 @@ import ( "time" "fortio.org/log" + "github.com/prometheus/client_golang/prometheus" ) type MockSlackMessenger struct { @@ -24,10 +25,14 @@ func (m *MockSlackMessenger) PostMessage(req SlackPostMessageRequest, url string } func TestApp_singleBurst_Success(t *testing.T) { + r := prometheus.NewRegistry() + metrics := NewMetrics(r) + messenger := &MockSlackMessenger{} app := &App{ slackQueue: make(chan SlackPostMessageRequest, 2), messenger: messenger, + metrics: metrics, } ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) @@ -61,10 +66,14 @@ func TestApp_singleBurst_Success(t *testing.T) { } func TestApp_MultiBurst_Success(t *testing.T) { + r := prometheus.NewRegistry() + metrics := NewMetrics(r) + messenger := &MockSlackMessenger{} app := &App{ slackQueue: make(chan SlackPostMessageRequest, 2), messenger: messenger, + metrics: metrics, } ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) From 6aa875403d281f8bd2f5a312b385e8953c1531b1 Mon Sep 17 00:00:00 2001 From: Laurent Demailly Date: Sat, 21 Oct 2023 16:52:00 -0700 Subject: [PATCH 5/6] fixed most of linter issues as well as misc other issues like *string --- .golangci.yml | 2 ++ app.go | 14 ++++++++------ app_test.go | 2 +- go.mod | 4 ++++ go.sum | 8 ++++++++ main.go | 16 +++++++++++----- metrics.go | 10 ++++------ server.go | 26 +++++++++++++++++--------- 8 files changed, 55 insertions(+), 27 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 3847205..e237c15 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -108,6 +108,8 @@ linters: - forcetypeassert - ireturn - depguard + # TODO bis: do put that one back: + - lll enable-all: true disable-all: false # Must not use fast: true in newer golangci-lint or it'll just skip a bunch of linter instead of doing caching like before (!) diff --git a/app.go b/app.go index f8fd7d6..5d373c7 100644 --- a/app.go +++ b/app.go @@ -118,8 +118,8 @@ func (s *SlackClient) PostMessage(request SlackPostMessageRequest, url string, t if err != nil { return err } - - req, err := http.NewRequest("POST", url, bytes.NewBuffer(jsonValue)) + // TODO: context for caller: + req, err := http.NewRequestWithContext(context.TODO(), http.MethodPost, url, bytes.NewBuffer(jsonValue)) if err != nil { return err } @@ -162,7 +162,8 @@ func (app *App) Shutdown() { app.wg.Wait() } -func (app *App) processQueue(ctx context.Context, MaxRetries int, InitialBackoffMs int, SlackPostMessageURL string, tokenFlag string, burst int) { +//nolint:gocognit // but could probably use a refactor. +func (app *App) processQueue(ctx context.Context, maxRetries int, initialBackoffMs int, slackPostMessageURL string, tokenFlag string, burst int) { // This is the rate limiter, which will block until it is allowed to continue on r.Wait(ctx). // I kept the rate at 1 per second, as doing more than that will cause Slack to reject the messages anyways. We can burst however. // Do note that this is best effort, in case of failures, we will exponentially backoff and retry, which will cause the rate to be lower than 1 per second due to obvious reasons. @@ -204,7 +205,8 @@ func (app *App) processQueue(ctx context.Context, MaxRetries int, InitialBackoff } } - err := app.messenger.PostMessage(msg, SlackPostMessageURL, tokenFlag) + err := app.messenger.PostMessage(msg, slackPostMessageURL, tokenFlag) + //nolint:nestif // but simplify by not having else at least. if err != nil { retryable, pause, description := CheckError(err.Error(), msg.Channel) @@ -227,9 +229,9 @@ func (app *App) processQueue(ctx context.Context, MaxRetries int, InitialBackoff app.metrics.RequestsRetriedTotal.WithLabelValues(msg.Channel).Inc() - if retryCount < MaxRetries { + if retryCount < maxRetries { retryCount++ - backoffDuration := time.Duration(InitialBackoffMs*int(math.Pow(2, float64(retryCount-1)))) * time.Millisecond + backoffDuration := time.Duration(initialBackoffMs*int(math.Pow(2, float64(retryCount-1)))) * time.Millisecond time.Sleep(backoffDuration) } else { log.S(log.Error, "Message failed after retries", log.Any("err", err), log.Int("retryCount", retryCount)) diff --git a/app_test.go b/app_test.go index d44d9e6..78741a6 100644 --- a/app_test.go +++ b/app_test.go @@ -17,7 +17,7 @@ type MockSlackMessenger struct { shouldError bool } -func (m *MockSlackMessenger) PostMessage(req SlackPostMessageRequest, url string, token string) error { +func (m *MockSlackMessenger) PostMessage(_ SlackPostMessageRequest, _ string, _ string) error { if m.shouldError { return errors.New("mock error") } diff --git a/go.mod b/go.mod index db845f5..22bd978 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.20 require ( fortio.org/assert v1.2.0 + fortio.org/fortio v1.60.3 fortio.org/log v1.11.0 fortio.org/scli v1.12.0 github.com/prometheus/client_golang v1.17.0 @@ -19,11 +20,14 @@ require ( github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/golang/protobuf v1.5.3 // indirect + github.com/google/uuid v1.3.1 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect github.com/prometheus/client_model v0.4.1-0.20230718164431-9a2bf3000d16 // indirect github.com/prometheus/common v0.44.0 // indirect github.com/prometheus/procfs v0.11.1 // indirect golang.org/x/exp v0.0.0-20231006140011-7918f672742d // indirect + golang.org/x/net v0.17.0 // indirect golang.org/x/sys v0.13.0 // indirect + golang.org/x/text v0.13.0 // indirect google.golang.org/protobuf v1.31.0 // indirect ) diff --git a/go.sum b/go.sum index 1149455..92dab58 100644 --- a/go.sum +++ b/go.sum @@ -4,6 +4,8 @@ fortio.org/cli v1.4.2 h1:pCYVPk5FpQuJtD31f9CG8sdgoTOWdv9Gls8As/MLJhU= fortio.org/cli v1.4.2/go.mod h1:VGqFDEKpA6u3NbTM3aSz2lZfAYKnGaszl1pLxbBhxsY= fortio.org/dflag v1.6.0 h1:acOc3KAu1kgp3Ov1nT2GbmcuNhPF1oMZqhT9nj3Oz1U= fortio.org/dflag v1.6.0/go.mod h1:MeDNYnBh30L6OdMGgAL6ltDX2xdQjOiexCZYPYXH8uM= +fortio.org/fortio v1.60.3 h1:adR0uf/69M5xxKaMLAautVf9FIVkEpMwuEWyMaaSnI0= +fortio.org/fortio v1.60.3/go.mod h1:AiuXKYWtbDyXvfEQgkYhLsUbwSZF78lEd+rGYXJaziA= fortio.org/log v1.11.0 h1:w7ueGPGbXz0A3+ApMz/5Q9gwEMrwSo/ohTlLo2Um6dU= fortio.org/log v1.11.0/go.mod h1:u/8/2lyczXq52aT5Nw6reD+3cR6m/EbS2jBiIYhgiTU= fortio.org/scli v1.12.0 h1:ABomNd0/RLgdGdf5lwgfwGhTuLwW3Of7bbTTgbN9r+g= @@ -25,6 +27,8 @@ github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= +github.com/google/uuid v1.3.1 h1:KjJaJ9iWZ3jOFZIf1Lqf4laDRCasjl0BCmnEGxkdLb4= +github.com/google/uuid v1.3.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zkfA9PSy5pEvNWRP0ET0TIVo= github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4= github.com/prometheus/client_golang v1.17.0 h1:rl2sfwZMtSthVU752MqfjQozy7blglC+1SOtjMAMh+Q= @@ -37,10 +41,14 @@ github.com/prometheus/procfs v0.11.1 h1:xRC8Iq1yyca5ypa9n1EZnWZkt7dwcoRPQwX/5gwa github.com/prometheus/procfs v0.11.1/go.mod h1:eesXgaPo1q7lBpVMoMy0ZOFTth9hBn4W/y0/p/ScXhY= golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= +golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM= +golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k= +golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4= golang.org/x/time v0.3.0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/main.go b/main.go index acb6353..0660019 100644 --- a/main.go +++ b/main.go @@ -33,9 +33,9 @@ type SlackPostMessageRequest struct { Text string `json:"text"` AsUser bool `json:"as_user,omitempty"` Username string `json:"username,omitempty"` - IconUrl string `json:"icon_url,omitempty"` + IconURL string `json:"icon_url,omitempty"` IconEmoji string `json:"icon_emoji,omitempty"` - ThreadTs string `json:"thread_ts,omitempty"` + ThreadTS string `json:"thread_ts,omitempty"` Parse string `json:"parse,omitempty"` LinkNames bool `json:"link_names,omitempty"` Blocks json.RawMessage `json:"blocks,omitempty"` // JSON serialized array of blocks @@ -61,7 +61,7 @@ func main() { ApplicationPort = ":8080" ) - // Define the flags with the default values + // Define the flags with the default values // TODO: move the ones that can change to dflag flag.IntVar(&MaxRetries, "maxRetries", MaxRetries, "Maximum number of retries for posting a message") flag.IntVar(&InitialBackoffMs, "initialBackoffMs", InitialBackoffMs, "Initial backoff in milliseconds for retries") flag.StringVar(&SlackPostMessageURL, "slackURL", SlackPostMessageURL, "Slack Post Message API URL") @@ -85,7 +85,7 @@ func main() { } log.Infof("Starting metrics server.") - go StartMetricServer(r, &MetricsPort) + StartMetricServer(r, MetricsPort) // Main ctx ctx, cancel := context.WithCancel(context.Background()) @@ -98,7 +98,13 @@ func main() { log.Infof("Starting main app logic") go app.processQueue(ctx, MaxRetries, InitialBackoffMs, SlackPostMessageURL, tokenFlag, burst) log.Infof("Starting receiver server") - go app.StartServer(serverCtx, &ApplicationPort) + // Check error return of app.StartServer in go routine anon function: + go func() { + err := app.StartServer(serverCtx, ApplicationPort) + if err != nil { + log.Fatalf("Error starting server: %v", err) + } + }() log.Infof("Up and running!") diff --git a/metrics.go b/metrics.go index d725cf5..ede90b3 100644 --- a/metrics.go +++ b/metrics.go @@ -1,9 +1,7 @@ package main import ( - "log" - "net/http" - + "fortio.org/fortio/fhttp" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" ) @@ -70,8 +68,9 @@ func NewMetrics(reg prometheus.Registerer) *Metrics { return m } -func StartMetricServer(reg *prometheus.Registry, addr *string) { - http.Handle("/metrics", promhttp.HandlerFor( +func StartMetricServer(reg *prometheus.Registry, addr string) { + mux, _ := fhttp.HTTPServer("metrics", addr) + mux.Handle("/metrics", promhttp.HandlerFor( reg, promhttp.HandlerOpts{ // Opt into OpenMetrics to support exemplars. @@ -80,5 +79,4 @@ func StartMetricServer(reg *prometheus.Registry, addr *string) { Registry: reg, }, )) - log.Fatal(http.ListenAndServe(*addr, nil)) } diff --git a/server.go b/server.go index 7d75515..f4a9cc9 100644 --- a/server.go +++ b/server.go @@ -4,26 +4,33 @@ package main import ( "context" "encoding/json" + "errors" "fmt" "net/http" "strings" + "fortio.org/fortio/fhttp" "fortio.org/log" ) -func (app *App) StartServer(ctx context.Context, applicationPort *string) error { +func (app *App) StartServer(ctx context.Context, applicationPort string) error { + // TODO probably switch to fhttp but need to see if I can add a shutdown hook. + name := "tbd" // TODO: Add a name field to "App" mux := http.NewServeMux() mux.HandleFunc("/", app.handleRequest) server := &http.Server{ - Addr: *applicationPort, - Handler: mux, + Addr: applicationPort, + Handler: mux, + ReadHeaderTimeout: fhttp.ServerIdleTimeout.Get(), + IdleTimeout: fhttp.ServerIdleTimeout.Get(), + ErrorLog: log.NewStdLogger("http srv "+name, log.Error), } doneCh := make(chan error) go func() { // Start the server - if err := server.ListenAndServe(); err != http.ErrServerClosed { + if err := server.ListenAndServe(); !errors.Is(err, http.ErrServerClosed) { doneCh <- err } close(doneCh) @@ -87,14 +94,15 @@ func (app *App) handleRequest(w http.ResponseWriter, r *http.Request) { // Validate the request err = validate(request) if err != nil { + log.S(log.Error, "Invalid request", log.Any("err", err)) + // TODO: jrpc.(Client)ErrorReply ? w.WriteHeader(http.StatusBadRequest) fakeSlackResponse.Ok = false fakeSlackResponse.Error = err.Error() - responseData, err := json.Marshal(fakeSlackResponse) - log.S(log.Error, "Invalid request", log.Any("err", err)) - _, err = w.Write(responseData) - if err != nil { - log.S(log.Error, "Failed to write response", log.Any("err", err)) + responseData, err2 := json.Marshal(fakeSlackResponse) + _, err3 := w.Write(responseData) + if err2 != nil || err3 != nil { + log.S(log.Error, "Failed to write response", log.Any("err2", err2), log.Any("err3", err3)) } return } From d057287de240140d80519fb12f30f17004af1f88 Mon Sep 17 00:00:00 2001 From: Laurent Demailly Date: Sat, 21 Oct 2023 17:58:28 -0700 Subject: [PATCH 6/6] new logger to solve reported issue --- app.go | 4 ++-- go.mod | 2 +- go.sum | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app.go b/app.go index 5d373c7..90ae81e 100644 --- a/app.go +++ b/app.go @@ -118,8 +118,8 @@ func (s *SlackClient) PostMessage(request SlackPostMessageRequest, url string, t if err != nil { return err } - // TODO: context for caller: - req, err := http.NewRequestWithContext(context.TODO(), http.MethodPost, url, bytes.NewBuffer(jsonValue)) + // Detach from the caller/new context. TODO: have some timeout (or use jrpc package functions which do that already) + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, url, bytes.NewBuffer(jsonValue)) if err != nil { return err } diff --git a/go.mod b/go.mod index 22bd978..fd4a9e2 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.20 require ( fortio.org/assert v1.2.0 fortio.org/fortio v1.60.3 - fortio.org/log v1.11.0 + fortio.org/log v1.12.0-pre1 fortio.org/scli v1.12.0 github.com/prometheus/client_golang v1.17.0 golang.org/x/time v0.3.0 diff --git a/go.sum b/go.sum index 92dab58..d885b3f 100644 --- a/go.sum +++ b/go.sum @@ -6,8 +6,8 @@ fortio.org/dflag v1.6.0 h1:acOc3KAu1kgp3Ov1nT2GbmcuNhPF1oMZqhT9nj3Oz1U= fortio.org/dflag v1.6.0/go.mod h1:MeDNYnBh30L6OdMGgAL6ltDX2xdQjOiexCZYPYXH8uM= fortio.org/fortio v1.60.3 h1:adR0uf/69M5xxKaMLAautVf9FIVkEpMwuEWyMaaSnI0= fortio.org/fortio v1.60.3/go.mod h1:AiuXKYWtbDyXvfEQgkYhLsUbwSZF78lEd+rGYXJaziA= -fortio.org/log v1.11.0 h1:w7ueGPGbXz0A3+ApMz/5Q9gwEMrwSo/ohTlLo2Um6dU= -fortio.org/log v1.11.0/go.mod h1:u/8/2lyczXq52aT5Nw6reD+3cR6m/EbS2jBiIYhgiTU= +fortio.org/log v1.12.0-pre1 h1:uhk9Xa8KxijA4xvNKYDPJ4AT6JoA1OsJPLO4x0+OPr8= +fortio.org/log v1.12.0-pre1/go.mod h1:u/8/2lyczXq52aT5Nw6reD+3cR6m/EbS2jBiIYhgiTU= fortio.org/scli v1.12.0 h1:ABomNd0/RLgdGdf5lwgfwGhTuLwW3Of7bbTTgbN9r+g= fortio.org/scli v1.12.0/go.mod h1:kLbbvX0QdnUOpQnW+7OsmY1d/qf5VMMkOiR+8GhqJII= fortio.org/sets v1.0.3 h1:HzewdGjH69YmyW06yzplL35lGr+X4OcqQt0qS6jbaO4=