log,server: avoid global variables to log/trace server IDs

[knz]

First commit from #73678.
Fixes #58938.
(still incomplete - needs all the other PRs connected to that issue)

Prior to this change, the server identifiers (cluster ID, node ID etc)
were stored in global variables in the log package.
This was problematic when a single process contains multiple servers,
e.g. in tests, demo and multi-tenant CockroachDB.

This change switches the mechanism to use identifiers stored in the
go context. The disadvantage is that the server IDs are not any
more logged at the beginning of each log file (since a given log file
could report data from multiple servers).

Release note (cli change): The server identifiers (cluster ID, node
ID, tenant ID, instance ID) are not any more duplicated at the start
of every new log file (during log file rotations). They are now only
logged when known during server start-up.
(The copy of the identifiers is still included in per-event envelopes
for the various json output logging formats.)

[cockroach-teamcity]

This change is 

[RaduBerinde]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz and @stevendanna)

---

pkg/ccl/backupccl/backup_test.go, line 6876 at r3 (raw file):

	makeTenant := func(srv serverutils.TestServerInterface, tenant uint64) (*sqlutils.SQLRunner, func()) {
		// Prevent a logging assertion that the server ID is initialized multiple times.

🎉

---

pkg/server/config.go, line 781 at r3 (raw file):

type idProvider struct {
	clusterID *base.ClusterIDContainer
	clusterS  atomic.Value

[nit] is S for string? Then make it Str so it's more clear. Also a comment somewhere mentioning that the Str fields cache tha values of their corresponding IDs, once those IDs are set.

---

pkg/server/config.go, line 783 at r3 (raw file):

	clusterS  atomic.Value

	isTenant bool

[nit] deserves a comment

---

pkg/server/config.go, line 822 at r3 (raw file):

	case log.IdentifyInstanceID:
		if !s.isTenant {

[nit] a comment would help here and below (eg "if this is not a tenant, the serverID is a node ID and not an instance ID").

---

pkg/server/config.go, line 839 at r3 (raw file):

// SetTenant informs the provider that it provides data for
// a SQL server.
func (s *idProvider) SetTenant(tenantID roachpb.TenantID) {

[nit] mention that this cannot be called after or concurrently with ServerIdentityString

---

pkg/server/config.go, line 841 at r3 (raw file):

func (s *idProvider) SetTenant(tenantID roachpb.TenantID) {
	s.isTenant = true
	s.tenantID = tenantID

This should assert that the tenantID is not set to anything else, no? (otherwise the memoized string could be out of sync)

---

pkg/server/config.go, line 844 at r3 (raw file):

}

func (s *idProvider) maybeMemoizeServerID() string {

[nit] could use a comment

---

pkg/util/log/server_ident.go, line 36 at r3 (raw file):

	IdentifyClusterID ServerIdentificationKey = iota
	// IdentifyNodeID retrieves the KV node ID of the server.
	IdentifyNodeID

[nit] consider using KVNodeID in places like this, just so there's less confusion

[knz]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz and @stevendanna)

---

pkg/server/config.go, line 781 at r3 (raw file):

Previously, RaduBerinde wrote…

[nit] is S for string? Then make it Str so it's more clear. Also a comment somewhere mentioning that the Str fields cache tha values of their corresponding IDs, once those IDs are set.

Done.

---

pkg/server/config.go, line 783 at r3 (raw file):

Previously, RaduBerinde wrote…

[nit] deserves a comment

Done.

---

pkg/server/config.go, line 822 at r3 (raw file):

Previously, RaduBerinde wrote…

[nit] a comment would help here and below (eg "if this is not a tenant, the serverID is a node ID and not an instance ID").

Done.

---

pkg/server/config.go, line 839 at r3 (raw file):

Previously, RaduBerinde wrote…

[nit] mention that this cannot be called after or concurrently with ServerIdentityString

Done.

---

pkg/server/config.go, line 841 at r3 (raw file):

Previously, RaduBerinde wrote…

This should assert that the tenantID is not set to anything else, no? (otherwise the memoized string could be out of sync)

Done.

---

pkg/server/config.go, line 844 at r3 (raw file):

Previously, RaduBerinde wrote…

[nit] could use a comment

Done.

[knz]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @stevendanna)

---

pkg/util/log/server_ident.go, line 36 at r3 (raw file):

Previously, RaduBerinde wrote…

[nit] consider using KVNodeID in places like this, just so there's less confusion

Done.

[RaduBerinde]

Very nice comments, thank you!!

Reviewed 33 of 35 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @knz and @stevendanna)

---

pkg/server/config.go, line 867 at r4 (raw file):

// interface.
func (s *idProvider) SetTenant(tenantID roachpb.TenantID) {
	var defaultT roachpb.TenantID

[nit] can just use s.tenantID.IsSet(). By the way, couldn't we use that instead of the separate isTenant flag as well?

---

pkg/server/config.go, line 876 at r4 (raw file):

// maybeMemoizeServerID saves the representation of serverID to
// serverStr.

[nit] "if the former is initialized".

[knz]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @RaduBerinde and @stevendanna)

---

pkg/server/config.go, line 867 at r4 (raw file):

Previously, RaduBerinde wrote…

[nit] can just use s.tenantID.IsSet(). By the way, couldn't we use that instead of the separate isTenant flag as well?

Good idea.
Done.

---

pkg/server/config.go, line 876 at r4 (raw file):

Previously, RaduBerinde wrote…

[nit] "if the former is initialized".

Done.

[knz]

TFYRs!

bors r=RaduBerinde,cameronnunez

[craig]

Build succeeded:

• GitHub CI (Cockroach)