keploy / keploy/keploy

[bug]: sync.Map passed by value causing lock copy violations detected by go vet

Open
#3,509 3 comments 0 reactions 0 assignees View on GitHub
bug keploy
Dominant language
Go
Stars
18.5k
Forks
2.4k
Avg merge
1d 4h
Merged PRs (30d)
101

Description

### 👀 Is there an existing issue for this?

- [x] I have searched and didn't find similar issue

### 👍 Current behavior

Running `go vet ./...` reveals that `sync.Map` is being passed by value in the telemetry and CLI provider code. This copies the internal mutex (`sync.noCopy`) and can lead to race conditions, deadlocks, and undefined behavior in concurrent operations.

**Expected behavior:** `sync.Map` should be passed by pointer (`*sync.Map`) to avoid copying the internal lock state.

**Actual behavior:** `sync.Map` is passed by value in struct fields and function parameters, triggering `go vet` warnings about lock copying.

### 👟 Steps to Replicate

1. Clone the Keploy repository
2. Navigate to the project root directory
3. Run the following command:
```bash
go vet ./...

### 📜 Logs (if any)

# go.keploy.io/server/v3/pkg/platform/telemetry
pkg/platform/telemetry/telemetry.go:34:43: NewTelemetry passes lock by value: go.keploy.io/server/v3/pkg/platform/telemetry.Options contains sync.Map contains sync.noCopy
pkg/platform/telemetry/telemetry.go:39:19: literal copies lock value from opt.GlobalMap: sync.Map contains sync.noCopy

# go.keploy.io/server/v3/cli/provider
cli/provider/service.go:38:19: literal copies lock value from TeleGlobalMap: sync.Map contains sync.noCopy

### 💻 Operating system

Linux

### 🧾 System Info (`uname -a`)

Linux Anand 5.15.167.4-microsoft-standard-WSL2 #1 SMP Tue Nov 5 00:21:55 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

### 📦 OS Release Info (`cat /etc/os-release`)

NAME="Ubuntu"
VERSION="22.04.5 LTS (Jammy Jellyfish)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 22.04.5 LTS"
VERSION_ID="22.04"

### 🐳 Docker Info (if applicable)

Not used

### 🧱 Your Environment

Built from source (main branch)
Running on Ubuntu 22.04 via WSL2

### 🎲 Version

keploy server v3 (from go.mod: go.keploy.io/server/v3)
Go version: go1.22.1 linux/amd64

### 📦 Repository

keploy

### 🤔 What use case were you trying? (optional)

"I was performing routine static analysis and code health checks using go vet to ensure that the project follows Go's concurrency best practices. I noticed these warnings which indicate potential instability in how telemetry data and global maps are handled across different goroutines."

### 🛠️ Proposed Fix

To resolve these go vet violations, we need to ensure that sync.Map is never copied by value. I propose the following changes:

1. In pkg/platform/telemetry/telemetry.go:
Update the Options struct to use GlobalMap *sync.Map.
Update the Telemetry struct to use GlobalMap *sync.Map.
Update the NewTelemetry constructor to accept and assign the pointer.
2. In cli/provider/service.go:
Pass the address of TeleGlobalMap (i.e., &TeleGlobalMap) when initializing the telemetry.Options struct.

I have already tested these changes locally, and go vet ./... passes successfully after the fix. I would like to submit a Pull Request for this!

Contributor guide

Open the contributing guide

Research direction

Start with pkg/platform/telemetry/telemetry.go, then inspect cli/provider/service.go where the reported sync.Map copies occur. Run go vet ./... before and after the change, and confirm it completes without lock-copy warnings while telemetry and CLI provider behavior remains intact.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, cli
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.