Add reporting of client ids to the config manager stats collector

fleetspeak/src/client/internal/config/manager.go

@@ -62,8 +62,6 @@ type Manager struct {
 //
 // The labels parameter defines what client labels the client should
 // report to the server.
-// TODO(b/297019580): Consider defining and consuming a more specific `ConfigManagerCollector`
-// interface here, containing only the methods that Manager actually cares about.
 func StartManager(cfg *config.Configuration, configChanges chan<- *fspb.ClientInfoData, c stats.ConfigManagerCollector) (*Manager, error) {
 	if cfg == nil {
 		return nil, errors.New("configuration must be provided")
@@ -107,7 +105,7 @@ func StartManager(cfg *config.Configuration, configChanges chan<- *fspb.ClientIn
 	r.AddRevokedSerials(r.state.RevokedCertSerials)
 	r.AddRevokedSerials(cfg.RevokedCertSerials)
 
-	if r.state.ClientKey == nil {
+	if len(r.state.GetClientKey()) == 0 {
 		if err := r.Rekey(); err != nil {
 			return nil, fmt.Errorf("no key present, and %v", err)
 		}
@@ -120,6 +118,7 @@ func StartManager(cfg *config.Configuration, configChanges chan<- *fspb.ClientIn
 		if err != nil {
 			return nil, fmt.Errorf("unable to create clientID: %v", err)
 		}
+		r.stats.AfterKeyLoaded(r.id, false, nil)
 		log.Infof("Using client id: %v", r.id)
 	}
 
@@ -143,8 +142,9 @@ func StartManager(cfg *config.Configuration, configChanges chan<- *fspb.ClientIn
 
 // Rekey creates a new private key and identity for the client.
 func (m *Manager) Rekey() (err error) {
+	var id common.ClientID
 	defer func() {
-		m.stats.AfterRekey(err)
+		m.stats.AfterKeyLoaded(id, true, err)
 	}()
 
 	k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
@@ -155,7 +155,7 @@ func (m *Manager) Rekey() (err error) {
 	if err != nil {
 		return fmt.Errorf("unable to marshal new key: %v", err)
 	}
-	id, err := common.MakeClientID(k.Public())
+	id, err = common.MakeClientID(k.Public())
 	if err != nil {
 		return fmt.Errorf("unable to create client id: %v", err)
 	}
@@ -220,7 +220,7 @@ func (m *Manager) AddRevokedSerials(revoked [][]byte) {
 	}
 }
 
-// Stop shuts down the Manager, in particular it will stop sychronizing to the
+// Stop shuts down the Manager, in particular it will stop synchronizing to the
 // writeback file.
 func (m *Manager) Stop() {
 	if m.syncTicker != nil {
@@ -250,7 +250,7 @@ func (m *Manager) ClientID() common.ClientID {
 // RecordRunningService adds name to the list of services which this client is
 // currently running. This list will be included when sending a ClientInfo
 // record to the server. The optional parameter sig should be set when the
-// configuration was signed to emake it clear to the server which instance of
+// configuration was signed to make it clear to the server which instance of
 // the service is running.
 func (m *Manager) RecordRunningService(name string, sig []byte) {
 	m.lock.Lock()

fleetspeak/src/client/internal/config/manager_test.go

@@ -32,12 +32,16 @@ type statsCollector struct {
 	stats.ConfigManagerCollector
 	mu     sync.Mutex
 	rekeys int
+	ids    []common.ClientID
 }
 
-func (sc *statsCollector) AfterRekey(err error) {
+func (sc *statsCollector) AfterKeyLoaded(id common.ClientID, new bool, err error) {
 	sc.mu.Lock()
 	defer sc.mu.Unlock()
-	sc.rekeys++
+	sc.ids = append(sc.ids, id)
+	if new {
+		sc.rekeys++
+	}
 }
 
 func (sc *statsCollector) AfterConfigSync(err error) {
@@ -72,6 +76,9 @@ func TestRekey(t *testing.T) {
 	if sc.rekeys != 2 {
 		t.Errorf("Unexpected amount of rekeys reported, got: %d, want: 2", sc.rekeys)
 	}
+	if id1 != sc.ids[0] || id2 != sc.ids[1] {
+		t.Errorf("Unexpected client IDs reported, got: %v, %v, want: %v, %v", sc.ids[0], sc.ids[1], id1, id2)
+	}
 }
 
 func TestWriteback(t *testing.T) {

fleetspeak/src/client/stats/collector.go

@@ -17,6 +17,8 @@
 package stats
 
 import (
+	"github.com/google/fleetspeak/fleetspeak/src/common"
+
 	fspb "github.com/google/fleetspeak/fleetspeak/src/common/proto/fleetspeak"
 )
 
@@ -42,8 +44,11 @@ type ConfigManagerCollector interface {
 	// AfterConfigSync is called after each config sync attempt by the config manager.
 	// err is the result of the operation.
 	AfterConfigSync(err error)
-	// AfterRekey is called after each rekey attempt by the config manager.
-	AfterRekey(err error)
+	// AfterKeyLoaded is called after the config manager loads a client key. This
+	// happens after reading the persisted state and on rekey operations.
+	// id is the client ID that corresponds to the loaded key. new is true if the
+	// key is newly generated, and err is the result of that operation.
+	AfterKeyLoaded(id common.ClientID, new bool, err error)
 }
 
 // ClientCollector gets notified about client operations.
@@ -132,8 +137,8 @@ func (c NoopCollector) MessageAcknowledged(msg *fspb.Message, size int) {}
 // AfterConfigSync implements Collector by doing nothing.
 func (c NoopCollector) AfterConfigSync(err error) {}
 
-// AfterRekey implements Collector by doing nothing.
-func (c NoopCollector) AfterRekey(err error) {}
+// AfterKeyLoaded implements Collector by doing nothing.
+func (c NoopCollector) AfterKeyLoaded(id common.ClientID, new bool, err error) {}
 
 // AfterMessageProcessed implements Collector by doing nothing.
 func (c NoopCollector) AfterMessageProcessed(msg *fspb.Message, isLocal bool, err error) {}