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.

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

[TT-13723] [release-5.3] Update to Go 1.23 #6823

Merged
merged 2 commits into from
Jan 10, 2025
Merged

Conversation

titpetric
Copy link
Contributor

@titpetric titpetric commented Jan 9, 2025

PR Type

Enhancement, Tests, Configuration changes


Description

  • Upgraded Go version to 1.23 across the project.

  • Introduced debug2.Record for goroutine state tracking and comparison.

  • Enhanced CI workflows with new linting and testing steps.

  • Added new system and unit tests for goroutine leak detection.


Changes walkthrough 📝

Relevant files
Tests
3 files
gateway_test.go
Removed goroutine leak detection tests                                     
+0/-56   
goroutine_test.go
Added unit tests for `debug2.Record`                                         
+103/-0 
goroutine_test.go
Added system tests for goroutine leak detection                   
+97/-0   
Enhancement
1 files
goroutine.go
Added `debug2.Record` for goroutine state tracking             
+124/-0 
Configuration changes
9 files
ci-tests.yml
Enhanced CI with linting and testing steps                             
+131/-53
plugin-compiler-build.yml
Updated plugin compiler workflow for Go 1.23                         
+15/-5   
release.yml
Updated release workflow for Go 1.23                                         
+8/-8     
deps.yml
Added task dependencies for CI tooling                                     
[link]   
hooks.yml
Added pre-commit and pre-push hooks                                           
+23/-0   
test.yml
Updated test taskfile to include dependencies                       
+1/-1     
Dockerfile
Updated Dockerfile to use Go 1.23 base image                         
+11/-51 
Taskfile.yml
Updated Taskfile to include hooks and Go version                 
+3/-2     
Dockerfile
Updated plugin compiler Dockerfile to Go 1.23                       
+1/-1     
Dependencies
1 files
go.mod
Updated Go module to use Go 1.23                                                 
+1/-1     
Documentation
1 files
README.md
Added README for system tests                                                       
+5/-0     

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

titpetric and others added 2 commits January 9, 2025 14:11
<details open>
<summary><a href="https://tyktech.atlassian.net/browse/TT-13723"
title="TT-13723" target="_blank">TT-13723</a></summary>
  <br />
  <table>
    <tr>
      <th>Summary</th>
      <td>Update to Go 1.23</td>
    </tr>
    <tr>
      <th>Type</th>
      <td>
<img alt="Story"
src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10315?size=medium"
/>
        Story
      </td>
    </tr>
    <tr>
      <th>Status</th>
      <td>In Dev</td>
    </tr>
    <tr>
      <th>Points</th>
      <td>N/A</td>
    </tr>
    <tr>
      <th>Labels</th>
      <td>-</td>
    </tr>
  </table>
</details>
<!--
  do not remove this marker as it will break jira-lint's functionality.
  added_by_jira_lint
-->

---

https://tyktech.atlassian.net/browse/TT-13723

It seems some tests detect goroutine leaks now. The detected goroutines
leaked have been listed in the ignores of a debug2.Record; both
goroutine leak tests detect goroutines in background reliably. Both are
flaky otherwise, this passes a -count=100 run, with and without -race.

___

Enhancement, Tests, Configuration changes

___

- Introduced `debug2.Record` to enhance goroutine state tracking and
comparison.
- Improved goroutine leak detection tests using `debug2.Record`.
- Added unit and benchmark tests for `debug2.Record`.
- Updated CI workflows to use Go 1.23.x.
- Simplified Dockerfile by switching to Go 1.23-bullseye base image and
optimizing build steps.
- Updated plugin compiler and release workflows to support Go 1.23.
- Enhanced Taskfile to dynamically use the Go version from `go.mod`.
- Bumped Go version in `go.mod` to 1.23.4.

___

<table><thead><tr><th></th><th align="left">Relevant
files</th></tr></thead><tbody><tr><td><strong>Tests</strong></td><td><details><summary>2
files</summary><table>
<tr>
  <td>
    <details>
<summary><strong>gateway_test.go</strong><dd><code>Improved goroutine
leak detection in tests.</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

gateway/gateway_test.go

<li>Enhanced goroutine leak tests with <code>debug2.Record</code> for
better <br>reliability.<br> <li> Introduced <code>newRecord</code>
helper function to manage ignored goroutines.<br> <li> Updated
assertions to use <code>debug2.Record</code> for goroutine count
<br>validation.<br>

</details>

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6812/files#diff-d34c7069ce5e81d45082b19eb3e869ee1a086e185dcd6630e75e3ed0d368b546">+37/-15</a>&nbsp;
</td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>goroutine_test.go</strong><dd><code>Added unit and
benchmark tests for `debug2.Record`.</code>&nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; </dd></summary>
<hr>

internal/debug2/goroutine_test.go

<li>Added unit tests for <code>debug2.Record</code> to validate
goroutine tracking.<br> <li> Included benchmark tests for performance
evaluation.<br> <li> Verified goroutine cleanup using <code>Since</code>
method.<br>

</details>

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6812/files#diff-763a0643c49a4bbfa70fbf12088045a74be7fc5f4789d09f14a9298d0b65c226">+103/-0</a>&nbsp;
</td>

</tr>

</table></details></td></tr><tr><td><strong>Enhancement</strong></td><td><details><summary>4
files</summary><table>
<tr>
  <td>
    <details>
<summary><strong>goroutine.go</strong><dd><code>Introduced
`debug2.Record` for goroutine state tracking.</code>&nbsp;
</dd></summary>
<hr>

internal/debug2/goroutine.go

<li>Added <code>debug2.Record</code> to capture and compare goroutine
states.<br> <li> Implemented methods for parsing, counting, and
filtering goroutines.<br> <li> Introduced functionality to ignore
specific goroutines during <br>comparison.<br>

</details>

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6812/files#diff-c6edbd39849f1acdcae221ef5e8b6a5886d644ec719064cf051be79f8c9377f8">+124/-0</a>&nbsp;
</td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>Dockerfile</strong><dd><code>Simplified Dockerfile with
Go 1.23 base image.</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

Dockerfile

<li>Simplified Dockerfile by using Go 1.23-bullseye base image.<br> <li>
Removed redundant Python installation steps.<br> <li> Optimized build
process with caching for Go modules.<br>

</details>

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6812/files#diff-dd2c0eb6ea5cfc6c4bd4eac30934e2d5746747af48fef6da689e85b752f39557">+11/-51</a>&nbsp;
</td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>Taskfile.yml</strong><dd><code>Enhanced Taskfile to
support dynamic Go version.</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

Taskfile.yml

- Added dynamic Go version argument for Docker builds.

</details>

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6812/files#diff-cd2d359855d0301ce190f1ec3b4c572ea690c83747f6df61c9340720e3d2425e">+1/-1</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>go.mod</strong><dd><code>Bumped Go version in module to
1.23.4.</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; </dd></summary>
<hr>

go.mod

- Updated Go version requirement to 1.23.4.

</details>

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6812/files#diff-33ef32bf6c23acb95f5902d7097b7a1d5128ca061167ec0716715b0b9eeaa5f6">+1/-1</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>
</table></details></td></tr><tr><td><strong>Configuration
changes</strong></td><td><details><summary>4 files</summary><table>
<tr>
  <td>
    <details>
<summary><strong>ci-tests.yml</strong><dd><code>Updated CI workflow to
use Go 1.23.x.</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

.github/workflows/ci-tests.yml

- Updated Go version in CI matrix to 1.23.x.

</details>

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6812/files#diff-03609cb60b0c6e92fb771eb8787d6722b8c31ca4c03eabc788e147acd8c6fb43">+1/-1</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>plugin-compiler-build.yml</strong><dd><code>Updated
plugin compiler workflow for Go 1.23.</code>&nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

.github/workflows/plugin-compiler-build.yml

<li>Updated base image to use Go 1.23-bullseye.<br> <li> Fixed
<code>BASE_IMAGE</code> argument in Docker build steps.<br>

</details>

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6812/files#diff-f3a95a900eb0ac23af6314e9cdea29fa16af0a9bcb61793a83a32ff13d4c4e79">+3/-3</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>release.yml</strong><dd><code>Updated release workflow
to support Go 1.23.</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

.github/workflows/release.yml

<li>Updated Go version in release workflow to 1.23-bullseye.<br> <li>
Adjusted Docker build conditions for the new Go version.<br>

</details>

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6812/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34">+11/-11</a>&nbsp;
</td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>Dockerfile</strong><dd><code>Updated plugin compiler
Dockerfile for Go 1.23.</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

ci/images/plugin-compiler/Dockerfile

- Updated base image to Go 1.23-bullseye.

</details>

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6812/files#diff-0ded1ed63ca128bd2d22721b0bc19dc85e440e4922164f465ac647917321971e">+1/-1</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>
</table></details></td></tr></tr></tbody></table>

___

> 💡 **PR-Agent usage**: Comment `/help "your question"` on any pull
request to receive relevant information

---------

Co-authored-by: Tit Petric <[email protected]>
@titpetric titpetric requested review from a team as code owners January 9, 2025 13:21
Copy link
Contributor

github-actions bot commented Jan 9, 2025

API Changes

--- prev.txt	2025-01-09 13:21:58.434208276 +0000
+++ current.txt	2025-01-09 13:21:54.384230483 +0000
@@ -11982,6 +11982,8 @@
 
 package regression // import "github.com/TykTechnologies/tyk/tests/regression"
 
+# Package: ./tests/system
+
 # Package: ./trace
 
 package trace // import "github.com/TykTechnologies/tyk/trace"

Copy link
Contributor

github-actions bot commented Jan 9, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

The parseGoroutines function in the Record struct may not handle edge cases where the buffer contains malformed or incomplete goroutine stack traces. This could lead to unexpected behavior or crashes.

func (r *Record) parseGoroutines() map[string][]string {
	goroutines := make(map[string][]string)
	var currentHeader string
	var currentStack []string
	toDelete := []string{}
	lines := strings.Split(r.buffer.String(), "\n")

	for _, line := range lines {
		var skip bool
		for _, ign := range r.ignores {
			if strings.Contains(line, ign) {
				skip = true
				break
			}
		}

		if skip {
			toDelete = append(toDelete, currentHeader)
		}

		if headerMatchRe.MatchString(line) {
			// Save the previous goroutine and reset
			if currentHeader != "" {
				goroutines[currentHeader] = currentStack
			}
			currentHeader = line
			currentStack = []string{line}
		} else if currentHeader != "" {
			// Add stack trace lines to the current goroutine
			currentStack = append(currentStack, line)
		}
	}

	// Save the last goroutine
	if currentHeader != "" {
		goroutines[currentHeader] = currentStack
	}

	for _, key := range toDelete {
		delete(goroutines, key)
	}

	return goroutines
Test Coverage

The removal of TestReloadGoroutineLeakWithTest and TestReloadGoroutineLeakWithCircuitBreaker in gateway_test.go should be reviewed to ensure that equivalent test coverage is provided elsewhere.

	_, _ = ts.Run(t, test.TestCase{
		Path: "/tyk-foo/",
		Code: 200,
	})
}

func listenProxyProto(ls net.Listener) error {
	pl := &proxyproto.Listener{Listener: ls}
	for {
		conn, err := pl.Accept()
Workflow Complexity

The addition of multiple steps and conditions in the CI workflow increases complexity. Ensure that all new steps, such as golangci-lint and sonar-cloud-analysis, are necessary and correctly configured.

      - ready_for_review
  push:
    branches:
      - master
      - release-**

concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: ${{ github.event_name == 'pull_request' }}

env:
  PYTHON_VERSION: "3.11"
  PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION: python

jobs:
  golangci-lint:
    runs-on: ubuntu-latest
    if: ${{ !github.event.pull_request.draft }}
    steps:
      - name: "Checkout PR"
        uses: TykTechnologies/github-actions/.github/actions/checkout-pr@main
        with:
          token: ${{ secrets.ORG_GH_TOKEN }}

      - name: "Get base ref"
        run: |
          git fetch origin ${{ github.base_ref }}
          git rev-parse origin/${{ github.base_ref }}

      - name: Setup Golang
        uses: actions/setup-go@v5
        with:
          go-version-file: go.mod
          cache-dependency-path: go.sum

      - name: Cache
        uses: actions/cache@v4
        with:
          path: |
            ~/.cache/golangci-lint
          key: 'golangci-lint-${{ runner.os }}-${{ hashFiles(''**/go.sum'') }}'
          restore-keys: |
            golangci-lint-${{ runner.os }}-

      - name: Setup CI Tooling
        uses: shrink/actions-docker-extract@v3
        with:
          image: tykio/ci-tools:latest
          path: /usr/local/bin/golangci-lint
          destination: /usr/local/bin

      - run: golangci-lint version && golangci-lint cache status

      - name: golangci-lint
        if: ${{ github.event_name == 'pull_request' }}
        run: |
          golangci-lint run --out-format colored-line-number,checkstyle:golangci-lint-report.json --issues-exit-code=0 --new-from-rev=origin/${{ github.base_ref }} -v ./...

      - name: golangci-lint-on-push
        if: ${{ github.event_name == 'push' }}
        run: |
          golangci-lint run --out-format checkstyle:golangci-lint-report.json --issues-exit-code=0 -v ./...

      - uses: actions/upload-artifact@v4
        if: ${{ always() }}
        with:
          name: golangcilint
          retention-days: 1
          path: |
            golangci-lint-report.json

  test:
    name: Go ${{ matrix.go-version }} Redis ${{ matrix.redis-version }}
    if: ${{ !github.event.pull_request.draft }}
    needs: golangci-lint
    # Runs on is pinned to a version that provides python 3.10.
    # See: https://github.com/actions/runner-images?tab=readme-ov-file#available-images
    # Avoid using ubuntu-latest as it would upgrade python unattended.
    runs-on: ubuntu-22.04
    strategy:
      fail-fast: false
      matrix:
        redis-version: [7]
        go-version: [1.23.x]

    env:
      REDIS_IMAGE: redis:${{ matrix.redis-version }}

    steps:
      - name: Checkout Tyk
        uses: actions/checkout@v4
        with:
          ref: ${{ github.event.pull_request.head.ref }}

      # Regardless that the base image provides a python release, we need
      # setup-python so it properly configures the python3-venv.
      - name: Setup Python
        uses: actions/setup-python@v5
        with:
          python-version: ${{ env.PYTHON_VERSION }}

      - name: Print runtime python version
        run: python3 -c 'import sys; print("%d.%d" % (sys.version_info[0], sys.version_info[1]))'

      - name: Print runtime pip version
        run: pip -V && pip3 -V

      - name: Setup CI Tooling
        uses: shrink/actions-docker-extract@v3
        with:
          image: tykio/ci-tools:latest
          path: /usr/local/bin/.
          destination: /usr/local/bin

      - name: Setup Golang
        uses: actions/setup-go@v5
        with:
          go-version-file: go.mod
          cache-dependency-path: go.sum

      - name: Install Dependencies and basic hygiene test
        id: hygiene
        run: |
          sudo apt-get install libluajit-5.1-dev

          python -m pip install --upgrade pip
          pip install setuptools
          pip install google
          pip install 'protobuf==4.24.4'

          task --version
          task lint

          git add --all
          git diff HEAD > git-state.log
          git_state_count=$(wc -l < git-state.log)

          if [[ $git_state_count -ne 0 ]]
          then
            echo "git-state<<EOF" >> $GITHUB_OUTPUT
            cat git-state.log >> $GITHUB_OUTPUT
            echo "EOF" >> $GITHUB_OUTPUT

            echo "task lint made git state dirty, please run task lint locally and update PR"
            echo
            cat git-state.log
            exit 1
          fi

      - name: Bring up test services
        run: task services:up

      - name: Preflight Python tests
        if: runner.debug == '1'
        run: TYK_LOGLEVEL=debug go test -p 1 -parallel 1 -race -v ./dlpython ./coprocess/...

      - name: Run Gateway Tests
        id: ci-tests
        run: |
          task test:e2e-combined args="-race -timeout=15m"
          task test:coverage

      - uses: actions/upload-artifact@v4
        if: ${{ always() }}
        with:
          name: coverage
          retention-days: 1
          path: coverage/gateway-all.cov

      - uses: actions/upload-artifact@v4
        if: ${{ always() }}
        with:
          name: testjson
          retention-days: 1
          path: coverage/gateway-all.json

  sonar-cloud-analysis:
    runs-on: ubuntu-latest
    if: ${{ !github.event.pull_request.draft }}
    needs: [test, golangci-lint]
    steps:
      - name: "Checkout repository"
        uses: TykTechnologies/github-actions/.github/actions/checkout-pr@main
        with:
          token: ${{ secrets.ORG_GH_TOKEN }}

      - name: Download coverage artifacts
        uses: actions/download-artifact@v4
        with:
          name: coverage

      - name: Download golangcilint artifacts
        uses: actions/download-artifact@v4
        with:
          name: golangcilint

      - name: Check reports existence
        id: check_files
        uses: andstor/file-existence-action@v3
        with:
          files: 'gateway-all.cov, golangci-lint-report.json'
          fail: true

      - name: Install Dependencies
        env:
          TOKEN: '${{ secrets.ORG_GH_TOKEN }}'
        run: git config --global url."https://${TOKEN}@github.com".insteadOf "https://github.com"

      - name: SonarCloud Scan
        if: always()
        uses: sonarsource/sonarcloud-github-action@master
        with:
          args: >
            -Dsonar.organization=tyktechnologies
            -Dsonar.projectKey=TykTechnologies_tyk
            -Dsonar.sources=.
            -Dsonar.exclusions=**/testdata/*,test/**,coprocess/**/*,ci/**,smoke-tests/**,apidef/oas/schema/schema.gen.go,templates/**
            -Dsonar.coverage.exclusions=**/*_test.go,**/mock/*
            -Dsonar.test.inclusions=**/*_test.go
            -Dsonar.tests=.
            -Dsonar.go.coverage.reportPaths=gateway-all.cov
            -Dsonar.go.golangci-lint.reportPaths=golangci-lint-report.json
        env:

Copy link
Contributor

github-actions bot commented Jan 9, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
General
Update the golangci-lint step to fail the workflow on linting issues

Ensure that the golangci-lint step fails the workflow if any linting issues are
found, rather than allowing it to pass with issues.

.github/workflows/ci-tests.yml [70]

-golangci-lint run --out-format colored-line-number,checkstyle:golangci-lint-report.json --issues-exit-code=0 --new-from-rev=origin/${{ github.base_ref }} -v ./...
+golangci-lint run --out-format colored-line-number,checkstyle:golangci-lint-report.json --new-from-rev=origin/${{ github.base_ref }} -v ./...
Suggestion importance[1-10]: 9

Why: Ensuring that the golangci-lint step fails the workflow on linting issues enforces code quality and prevents merging code with unresolved linting problems. This is a critical improvement for maintaining code standards.

9
Add a timeout to prevent infinite loops when waiting for goroutines to finish

Add a timeout mechanism to the loop waiting for goroutines to finish to prevent
infinite loops in case of unexpected behavior.

internal/debug2/goroutine_test.go [40-60]

+timeout := time.After(5 * time.Second)
 for {
     ...
     if remainingGoroutines.Count() == 0 {
         break
     }
     if ctx.Err() != nil {
         break
     }
+    select {
+    case <-timeout:
+        t.Error("Timeout waiting for goroutines to finish")
+        return
+    default:
+    }
 }
Suggestion importance[1-10]: 8

Why: Adding a timeout mechanism is crucial to prevent infinite loops during testing, which can cause the test suite to hang indefinitely. This significantly improves the reliability and robustness of the test.

8
Use a dynamic threshold for goroutine count validation to improve test adaptability

Validate the before.Count() value against a more dynamic threshold instead of a
hardcoded value of 100 to accommodate varying environments.

tests/system/goroutine_test.go [35]

-require.Less(t, before.Count(), 100, "before count over a 100, leak: %s", before)
+dynamicThreshold := 100 // Adjust based on environment
+require.Less(t, before.Count(), dynamicThreshold, "before count exceeds threshold, leak: %s", before)
Suggestion importance[1-10]: 5

Why: Using a dynamic threshold makes the test more adaptable to different environments, but the improvement is minor as the hardcoded value might already suffice in most cases.

5
Possible issue
Clear the toDelete slice after processing to prevent unintended deletions

Ensure that the toDelete slice is cleared after processing each goroutine to avoid
unintended deletions in subsequent iterations.

internal/debug2/goroutine.go [44-79]

 toDelete := []string{}
 ...
 for _, key := range toDelete {
     delete(goroutines, key)
 }
+toDelete = []string{}
Suggestion importance[1-10]: 6

Why: Clearing the toDelete slice after processing ensures that deletions from previous iterations do not affect subsequent ones. This improves the robustness of the code, though the impact is moderate since the current implementation may not always lead to issues.

6

Copy link

sonarqubecloud bot commented Jan 9, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@titpetric titpetric enabled auto-merge (squash) January 9, 2025 21:51
@titpetric titpetric disabled auto-merge January 10, 2025 08:51
@titpetric titpetric merged commit 91408d4 into release-5.3 Jan 10, 2025
36 of 40 checks passed
@titpetric titpetric deleted the backport/go-1.23 branch January 10, 2025 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant