Skip to content

feat(lib/hashring): add logs to hash ring membership wait#576

Merged
thijmv merged 1 commit intomasterfrom
thijmv/feat/hashring-membership-wait-logs
Mar 5, 2026
Merged

feat(lib/hashring): add logs to hash ring membership wait#576
thijmv merged 1 commit intomasterfrom
thijmv/feat/hashring-membership-wait-logs

Conversation

@thijmv
Copy link
Collaborator

@thijmv thijmv commented Mar 5, 2026

This PR improves observability for the hashring waitForContains method by adding logs to both paths.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves observability of hash ring membership convergence by emitting structured logs from ring.WaitForContains on both success and timeout paths.

Changes:

  • Add structured error log when WaitForContains times out.
  • Add structured info log (including elapsed duration) when WaitForContains succeeds.
  • Store elapsed duration in a local variable before recording the membership-wait histogram.
Comments suppressed due to low confidence (1)

lib/hashring/ring.go:165

  • The timeout log doesn’t include the actual time spent waiting. Since this change is specifically for observability, consider logging the elapsed duration (and optionally recording the histogram) on the error path too, so it’s easy to compare “waited” vs configured timeout when debugging slow/failed membership convergence.
	if err := backoff.Retry(operation, backoff.WithContext(b, ctx)); err != nil {
		log.With("addr", addr, "error", err, "timeout", r.config.MembershipWaitTimeout).Error("Timed out waiting to find address in hash ring")
		return fmt.Errorf("timed out waiting for membership: %w", err)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@thijmv thijmv merged commit e3cf249 into master Mar 5, 2026
14 checks passed
@thijmv thijmv deleted the thijmv/feat/hashring-membership-wait-logs branch March 5, 2026 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants