Merge branch 'main' into feat/commit-tracking-log-store-checker

Author: peterxcli

.github/CODEOWNERS

@@ -1,4 +1,13 @@
-* @hashicorp/consul-core-reviewers @hashicorp/nomad-eng
+# Each line is a file pattern followed by one or more owners.
+# More on CODEOWNERS files: https://help.github.com/en/github/creating-cloning-and-archiving-repositories/about-code-owners
 
-/.release/                           @hashicorp/release-engineering
-/.github/workflows/ci.yml            @hashicorp/release-engineering
+# Default owner
+* @hashicorp/team-ip-compliance @hashicorp/consul-core-reviewers @hashicorp/nomad-eng @hashicorp/raft-force
+
+# Add override rules below. Each line is a file/folder pattern followed by one or more owners.
+# Being an owner means those groups or individuals will be added as reviewers to PRs affecting
+# those areas of the code.
+# Examples:
+# /docs/  @docs-team
+# *.js    @js-team
+# *.go    @go-team

.gitignore

@@ -21,3 +21,6 @@ _testmain.go
 
 *.exe
 *.test
+
+# Goland IDE
+.idea

.golangci-lint.yml

@@ -9,13 +9,25 @@ linters-settings:
     check-shadowing: true
   golint:
     min-confidence: 0
+  depguard:
+    rules:
+      main:
+        list-mode: lax
+        allow:
+          - "github.com/hashicorp/go-metrics/compat"
+        deny:
+          - pkg: "github.com/hashicorp/go-metrics"
+            desc: not allowed, use github.com/hashicorp/go-metrics/compat instead
+          - pkg: "github.com/armon/go-metrics"
+            desc: not allowed, use github.com/hashicorp/go-metrics/compat instead
 
 linters:
   disable-all: true
   enable:
     - gofmt
     #- golint
     - govet
+    - depguard
     #- varcheck
     #- typecheck
     #- gosimple

CHANGELOG.md

@@ -1,5 +1,9 @@
 # UNRELEASED
 
+IMPROVEMENETS
+
+* Added a flag to skip legacy duplicate telemetry. [GH-630](https://github.com/hashicorp/raft/pull/630)
+
 # 1.7.0 (June 5th, 2024)
 
 CHANGES
@@ -37,7 +41,7 @@ CHANGES
 
   go-msgpack v2.1.1 is by default binary compatible with v0.5.5 ("non-builtin" encoding of `time.Time`), but can decode messages produced by v1.1.5 as well ("builtin" encoding of `time.Time`).
 
-  However, if users of this libary overrode the version of go-msgpack (especially to v1), this **could break** compatibility if raft nodes are running a mix of versions.
+  However, if users of this library overrode the version of go-msgpack (especially to v1), this **could break** compatibility if raft nodes are running a mix of versions.
 
   This compatibility can be configured at runtime in Raft using `NetworkTransportConfig.MsgpackUseNewTimeFormat` -- the default is `false`, which maintains compatibility with `go-msgpack` v0.5.5, but if set to `true`, will be compatible with `go-msgpack` v1.1.5.
 

README.md

@@ -110,3 +110,29 @@ greatly sacrificing performance.
 In terms of performance, Raft is comparable to Paxos. Assuming stable leadership,
 committing a log entry requires a single round trip to half of the cluster.
 Thus performance is bound by disk I/O and network latency.
+
+
+  ## Metrics Emission and Compatibility
+
+  This library can emit metrics using either `github.com/armon/go-metrics` or `github.com/hashicorp/go-metrics`. Choosing between the libraries is controlled via build tags. 
+
+  **Build Tags**
+  * `armonmetrics` - Using this tag will cause metrics to be routed to `armon/go-metrics`
+  * `hashicorpmetrics` - Using this tag will cause all metrics to be routed to `hashicorp/go-metrics`
+
+  If no build tag is specified, the default behavior is to use `armon/go-metrics`. 
+
+  **Deprecating `armon/go-metrics`**
+
+  Emitting metrics to `armon/go-metrics` is officially deprecated. Usage of `armon/go-metrics` will remain the default until mid-2025 with opt-in support continuing to the end of 2025.
+
+  **Migration**
+  To migrate an application currently using the older `armon/go-metrics` to instead use `hashicorp/go-metrics` the following should be done.
+
+  1. Upgrade libraries using `armon/go-metrics` to consume `hashicorp/go-metrics/compat` instead. This should involve only changing import statements. All repositories in the `hashicorp` namespace
+  2. Update an applications library dependencies to those that have the compatibility layer configured.
+  3. Update the application to use `hashicorp/go-metrics` for configuring metrics export instead of `armon/go-metrics`
+     * Replace all application imports of `github.com/armon/go-metrics` with `github.com/hashicorp/go-metrics`
+     * Instrument your build system to build with the `hashicorpmetrics` tag.
+
+  Eventually once the default behavior changes to use `hashicorp/go-metrics` by default (mid-2025), you can drop the `hashicorpmetrics` build tag.

api.go

@@ -12,8 +12,8 @@ import (
 	"sync/atomic"
 	"time"
 
-	metrics "github.com/armon/go-metrics"
 	hclog "github.com/hashicorp/go-hclog"
+	metrics "github.com/hashicorp/go-metrics/compat"
 )
 
 const (
@@ -59,7 +59,7 @@ var (
 	ErrEnqueueTimeout = errors.New("timed out enqueuing operation")
 
 	// ErrNothingNewToSnapshot is returned when trying to create a snapshot
-	// but there's nothing new commited to the FSM since we started.
+	// but there's nothing new committed to the FSM since we started.
 	ErrNothingNewToSnapshot = errors.New("nothing new to snapshot")
 
 	// ErrUnsupportedProtocol is returned when an operation is attempted
@@ -222,6 +222,11 @@ type Raft struct {
 	// prevote feature is disabled if set to true.
 	preVoteDisabled bool
 
+	// noLegacyTelemetry allows to skip the legacy metrics to avoid duplicates.
+	// legacy metrics are those that have `_peer_name` as metric suffix instead as labels.
+	// e.g: raft_replication_heartbeat_peer0
+	noLegacyTelemetry bool
+
 	// RestoreCommittedLogs is used to enable restore committed logs mode
 	// restore committed logs mode is disabled if set to false.
 	RestoreCommittedLogs bool
@@ -240,7 +245,8 @@ type Raft struct {
 // listing just itself as a Voter, then invoke AddVoter() on it to add other
 // servers to the cluster.
 func BootstrapCluster(conf *Config, logs LogStore, stable StableStore,
-	snaps SnapshotStore, trans Transport, configuration Configuration) error {
+	snaps SnapshotStore, trans Transport, configuration Configuration,
+) error {
 	// Validate the Raft server config.
 	if err := ValidateConfig(conf); err != nil {
 		return err
@@ -313,7 +319,8 @@ func BootstrapCluster(conf *Config, logs LogStore, stable StableStore,
 // the sole voter, and then join up other new clean-state peer servers using
 // the usual APIs in order to bring the cluster back into a known state.
 func RecoverCluster(conf *Config, fsm FSM, logs LogStore, stable StableStore,
-	snaps SnapshotStore, trans Transport, configuration Configuration) error {
+	snaps SnapshotStore, trans Transport, configuration Configuration,
+) error {
 	// Validate the Raft server config.
 	if err := ValidateConfig(conf); err != nil {
 		return err
@@ -444,7 +451,8 @@ func RecoverCluster(conf *Config, fsm FSM, logs LogStore, stable StableStore,
 // without starting a Raft instance or connecting to the cluster. This function
 // has identical behavior to Raft.GetConfiguration.
 func GetConfiguration(conf *Config, fsm FSM, logs LogStore, stable StableStore,
-	snaps SnapshotStore, trans Transport) (Configuration, error) {
+	snaps SnapshotStore, trans Transport,
+) (Configuration, error) {
 	conf.skipStartup = true
 	r, err := NewRaft(conf, fsm, logs, stable, snaps, trans)
 	if err != nil {
@@ -574,6 +582,7 @@ func NewRaft(conf *Config, fsm FSM, logs LogStore, stable StableStore, snaps Sna
 		followerNotifyCh:      make(chan struct{}, 1),
 		mainThreadSaturation:  newSaturationMetric([]string{"raft", "thread", "main", "saturation"}, 1*time.Second),
 		preVoteDisabled:       conf.PreVoteDisabled || !transportSupportPreVote,
+		noLegacyTelemetry:     conf.NoLegacyTelemetry,
 		RestoreCommittedLogs:  conf.RestoreCommittedLogs,
 	}
 	if !transportSupportPreVote && !conf.PreVoteDisabled {
@@ -1147,12 +1156,12 @@ func (r *Raft) State() RaftState {
 // lose it.
 //
 // Receivers can expect to receive a notification only if leadership
-// transition has occured.
+// transition has occurred.
 //
 // If receivers aren't ready for the signal, signals may drop and only the
 // latest leadership transition. For example, if a receiver receives subsequent
 // `true` values, they may deduce that leadership was lost and regained while
-// the the receiver was processing first leadership transition.
+// the receiver was processing first leadership transition.
 func (r *Raft) LeaderCh() <-chan bool {
 	return r.leaderCh
 }
@@ -1256,6 +1265,11 @@ func (r *Raft) Stats() map[string]string {
 	return s
 }
 
+// CurrentTerm returns the current term.
+func (r *Raft) CurrentTerm() uint64 {
+	return r.getCurrentTerm()
+}
+
 // LastIndex returns the last index in stable storage,
 // either from the last log or from the last snapshot.
 func (r *Raft) LastIndex() uint64 {

bench/bench.go

@@ -99,7 +99,7 @@ func StoreLogs(b *testing.B, store raft.LogStore) {
 func DeleteRange(b *testing.B, store raft.LogStore) {
 	// Create some fake data. In this case, we create 3 new log entries for each
 	// test case, and separate them by index in multiples of 10. This allows
-	// some room so that we can test deleting ranges with "extra" logs to
+	// some room so that we can test deleting ranges with "extra" logs
 	// to ensure we stop going to the database once our max index is hit.
 	var logs []*raft.Log
 	for n := 0; n < b.N; n++ {

config.go

@@ -235,6 +235,11 @@ type Config struct {
 	// PreVoteDisabled deactivate the pre-vote feature when set to true
 	PreVoteDisabled bool
 
+	// NoLegacyTelemetry allows to skip the legacy metrics to avoid duplicates.
+	// legacy metrics are those that have `_peer_name` as metric suffix instead as labels.
+	// e.g: raft_replication_heartbeat_peer0
+	NoLegacyTelemetry bool
+
 	// RestoreCommittedLogs controls if the Raft server should use the restore committed logs
 	// mechanism. Restore committed logs requires a LogStore implementation that
 	// support commit tracking. When such a store is used and this config

configuration.go

@@ -180,7 +180,7 @@ func hasVote(configuration Configuration, id ServerID) bool {
 	return false
 }
 
-// inConfiguration returns true if the server identified by 'id' is in in the
+// inConfiguration returns true if the server identified by 'id' is in the
 // provided Configuration.
 func inConfiguration(configuration Configuration, id ServerID) bool {
 	for _, server := range configuration.Servers {

docs/apply.md

@@ -1,7 +1,7 @@
 # Raft Apply
 
 Apply is the primary operation provided by raft. A client calls `raft.Apply` to apply
-a command to the FSM. A command will first be commited, i.e., durably stored on a
+a command to the FSM. A command will first be committed, i.e., durably stored on a
 quorum of raft nodes. Then, the committed command is applied to fsm.
 
 This sequence diagram shows the steps involved in a `raft.Apply` operation. Each box
@@ -63,7 +63,7 @@ leader's lastIndex). Another parameter to AppendEntries is the LeaderCommitIndex
 is some examples:
 
 ```
-AppenEntries(Log: 1..5, LeaderCommitIndex: 0)    // Replicating log entries 1..5, 
+AppendEntries(Log: 1..5, LeaderCommitIndex: 0)   // Replicating log entries 1..5, 
                                                  // the leader hasn't committed any log entry;
 AppendEntries(Log: 6..8, LeaderCommitIndex: 4)   // Replicating log entries 6..8,
                                                  // log 0..4 are committed after the leader receives
@@ -92,7 +92,7 @@ Therefore, it's possible that a very small window of time exists when all follow
 committed the log to disk, the write has been realized in the FSM of the leader but the
 followers have not yet applied the log to their FSM.
 
-7. The peer applies the commited entries to the FSM.
+7. The peer applies the committed entries to the FSM.
 
 8. If all went well, the follower responds success (`resp.Success = true`) to the 
 `appendEntries` RPC call.
@@ -108,9 +108,9 @@ grouping the entries that can be applied to the fsm.
 
 11. `processLogs` applies all the committed entries that haven't been applied by batching the log entries and forwarding them through the `fsmMutateCh` channel to fsm.
 
-12. The actual place applying the commited log entries is in the main loop of `runFSM()`.
+12. The actual place applying the committed log entries is in the main loop of `runFSM()`.
 
 13. After the log entries that contains the client req are applied to the fsm, the fsm
-module will set the reponses to the client request (`req.future.respond(nil)`). From the
+module will set the responses to the client request (`req.future.respond(nil)`). From the
 client's point of view, the future returned by `raft.Apply` should now be unblocked and
 calls to `Error()` or `Response()` should return the data at this point.

fsm.go

@@ -8,8 +8,8 @@ import (
 	"io"
 	"time"
 
-	"github.com/armon/go-metrics"
 	hclog "github.com/hashicorp/go-hclog"
+	"github.com/hashicorp/go-metrics/compat"
 )
 
 // FSM is implemented by clients to make use of the replicated log.
@@ -34,6 +34,12 @@ type FSM interface {
 	// Apply and Snapshot are always called from the same thread, but Apply will
 	// be called concurrently with FSMSnapshot.Persist. This means the FSM should
 	// be implemented to allow for concurrent updates while a snapshot is happening.
+	//
+	// Clients of this library should make no assumptions about whether a returned
+	// Snapshot() will actually be stored by Raft. In fact it's quite possible that
+	// any Snapshot returned by this call will be discarded, and that
+	// FSMSnapshot.Persist will never be called. Raft will always call
+	// FSMSnapshot.Release however.
 	Snapshot() (FSMSnapshot, error)
 
 	// Restore is used to restore an FSM from a snapshot. It is not called

fuzzy/cluster.go

@@ -184,7 +184,7 @@ func (c *cluster) Stop(t *testing.T, maxWait time.Duration) {
 }
 
 // WaitTilUptoDate blocks until all nodes in the cluster have gotten their
-// commitedIndex upto the Index from the last successful call to Apply
+// committedIndex upto the Index from the last successful call to Apply
 func (c *cluster) WaitTilUptoDate(t *testing.T, maxWait time.Duration) {
 	idx := c.lastApplySuccess.Index()
 	start := time.Now()

fuzzy/verifier.go

@@ -58,7 +58,7 @@ func (v *appendEntriesVerifier) PreAppendEntries(src, target string, req *raft.A
 	if ldr != src {
 		v.Lock()
 		defer v.Unlock()
-		v.errors = append(v.errors, fmt.Sprintf("Node %v sent an appendEnties request for term %d that said the leader was some other node %v", src, term, ldr))
+		v.errors = append(v.errors, fmt.Sprintf("Node %v sent an appendEntries request for term %d that said the leader was some other node %v", src, term, ldr))
 	}
 	v.RLock()
 	tl, exists := v.leaderForTerm[term]

go.mod

@@ -5,13 +5,14 @@ go 1.20
 retract v1.1.3 // Deleted original tag; module checksum may not be accurate.
 
 require (
-	github.com/armon/go-metrics v0.4.1
 	github.com/hashicorp/go-hclog v1.6.2
+	github.com/hashicorp/go-metrics v0.5.4
 	github.com/hashicorp/go-msgpack/v2 v2.1.2
 	github.com/stretchr/testify v1.8.4
 )
 
 require (
+	github.com/armon/go-metrics v0.4.1 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/fatih/color v1.13.0 // indirect
 	github.com/hashicorp/go-immutable-radix v1.0.0 // indirect