Skip to content

all: switch to math/rand/v2 - #954

Merged
rita7lopes merged 1 commit into
metrico:masterfrom
janeblower:math-rand-v2
Sep 2, 2026
Merged

rita7lopes merged 1 commit into
metrico:masterfrom
janeblower:math-rand-v2

Conversation

@janeblower

Copy link
Copy Markdown
Contributor

The top-level math/rand/v2 functions are safe for concurrent use and need no seeding, so the per-struct *rand.Rand fields and the locks guarding them can go. Two of those locks guarded nothing else: the one in the writer service registry, and the global one in the pattern controller, which was taken on every single log line whenever pattern downsampling was enabled.

Two bugs fell out along the way:

  • writer/pattern/controller: random was declared but never assigned, so skipLine() dereferenced a nil *rand.Rand as soon as LogPatternsDownsampling was configured below 1.
  • CLokiQueriable.random was written and copied but never read — removed.

Mutexes that protect more than the generator (reader/registry/static.go, writer/service/generic_insert.go) are kept; only the lock around the rand call itself is dropped.

9 files, −29 lines. go build ./... and go test ./... pass.

Top-level math/rand/v2 functions are safe for concurrent use and need no
seeding, so the per-struct *rand.Rand fields and the locks around them go
away. Two of those locks guarded nothing else: the one in the writer
service registry, and the global one in the pattern controller, taken on
every log line when pattern downsampling is on.

The pattern controller's `random` was declared but never assigned, so
skipLine() dereferenced a nil *rand.Rand as soon as downsampling was
configured below 1.

CLokiQueriable.random was written and copied but never read; drop it.

@rita7lopes rita7lopes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good one @janeblower !
Merging

@rita7lopes
rita7lopes merged commit 6804c91 into metrico:master Sep 2, 2026
9 checks passed
@janeblower
janeblower deleted the math-rand-v2 branch September 2, 2026 21:06
tsouza added a commit to tsouza/gigapipe that referenced this pull request Sep 4, 2026
Resolves the two real conflicts by hand: master's math/rand/v2 modernization
(metrico#954) touched the same rand.Int64() lines this branch's determinism fix
replaces with NextSubstituteName() -- kept this branch's fix, which fully
supersedes the modernization (no rand import needed once the counter replaces
random naming entirely). go.yml auto-merged cleanly, keeping both master's
gofmt step and this branch's lint/-race/arch-lint additions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants