diff --git a/fleetspeak/src/client/client.go b/fleetspeak/src/client/client.go index 513ebcbe..60a26f31 100644 --- a/fleetspeak/src/client/client.go +++ b/fleetspeak/src/client/client.go @@ -33,6 +33,7 @@ import ( "github.com/google/fleetspeak/fleetspeak/src/client/internal/message" "github.com/google/fleetspeak/fleetspeak/src/client/service" "github.com/google/fleetspeak/fleetspeak/src/client/signer" + "github.com/google/fleetspeak/fleetspeak/src/client/stats" "github.com/google/fleetspeak/fleetspeak/src/common" fspb "github.com/google/fleetspeak/fleetspeak/src/common/proto/fleetspeak" @@ -50,6 +51,7 @@ type Components struct { ServiceFactories map[string]service.Factory // Required to instantiate any local services. Signers []signer.Signer // If set, will be given a chance to sign data before sending it to the server. Filter *flow.Filter // If set, will be used to filter messages to the server. + Stats stats.Collector } // A Client is an active fleetspeak client instance. @@ -95,7 +97,7 @@ type Client struct { // TODO: Add support for multiple Communicators. func New(cfg config.Configuration, cmps Components) (*Client, error) { configChanges := make(chan *fspb.ClientInfoData) - cm, err := intconfig.StartManager(&cfg, configChanges) + cm, err := intconfig.StartManager(&cfg, configChanges, cmps.Stats) if err != nil { return nil, fmt.Errorf("bad configuration: %v", err) } diff --git a/fleetspeak/src/client/client/client.go b/fleetspeak/src/client/client/client.go index 3fff0534..344de1f0 100644 --- a/fleetspeak/src/client/client/client.go +++ b/fleetspeak/src/client/client/client.go @@ -15,6 +15,7 @@ import ( "github.com/google/fleetspeak/fleetspeak/src/client/https" "github.com/google/fleetspeak/fleetspeak/src/client/service" "github.com/google/fleetspeak/fleetspeak/src/client/socketservice" + "github.com/google/fleetspeak/fleetspeak/src/client/stats" "github.com/google/fleetspeak/fleetspeak/src/client/stdinservice" gpb "github.com/google/fleetspeak/fleetspeak/src/client/generic/proto/fleetspeak_client_generic" @@ -56,6 +57,7 @@ func innerMain() { "Stdin": stdinservice.Factory, }, Communicator: com, + Stats: stats.NoopCollector{}, }) if err != nil { log.Exitf("Error starting client: %v", err) diff --git a/fleetspeak/src/client/internal/config/manager.go b/fleetspeak/src/client/internal/config/manager.go index f38934db..c8d820a6 100644 --- a/fleetspeak/src/client/internal/config/manager.go +++ b/fleetspeak/src/client/internal/config/manager.go @@ -28,6 +28,7 @@ import ( log "github.com/golang/glog" "github.com/google/fleetspeak/fleetspeak/src/client/config" + "github.com/google/fleetspeak/fleetspeak/src/client/stats" "github.com/google/fleetspeak/fleetspeak/src/common" "google.golang.org/protobuf/proto" @@ -39,8 +40,9 @@ import ( // It exports a view of the current configuration and is safe for concurrent // access. type Manager struct { - cfg *config.Configuration // does not change - cc *clpb.CommunicatorConfig + cfg *config.Configuration // does not change + cc *clpb.CommunicatorConfig + stats stats.Collector lock sync.RWMutex // protects the state variables below state *clpb.ClientState @@ -60,10 +62,13 @@ type Manager struct { // // The labels parameter defines what client labels the client should // report to the server. -func StartManager(cfg *config.Configuration, configChanges chan<- *fspb.ClientInfoData) (*Manager, error) { +func StartManager(cfg *config.Configuration, configChanges chan<- *fspb.ClientInfoData, c stats.Collector) (*Manager, error) { if cfg == nil { return nil, errors.New("configuration must be provided") } + if c == nil { + c = stats.NoopCollector{} + } if cfg.PersistenceHandler == nil { log.Warning("PersistenceHandler not provided. Using NoopPersistenceHandler.") @@ -77,8 +82,9 @@ func StartManager(cfg *config.Configuration, configChanges chan<- *fspb.ClientIn } r := Manager{ - cfg: cfg, - cc: cfg.CommunicatorConfig, + cfg: cfg, + cc: cfg.CommunicatorConfig, + stats: c, state: &clpb.ClientState{}, revokedSerials: make(map[string]bool), @@ -174,9 +180,12 @@ func (m *Manager) Sync() error { m.lock.Lock() defer m.lock.Unlock() - if err := m.cfg.PersistenceHandler.WriteState(m.state); err != nil { + err := m.cfg.PersistenceHandler.WriteState(m.state) + if err != nil { return fmt.Errorf("Failed to sync state to writeback: %v", err) } + m.stats.ConfigSynced(err) + // Todo metric: need collector m.dirty = false return nil diff --git a/fleetspeak/src/client/internal/config/manager_test.go b/fleetspeak/src/client/internal/config/manager_test.go index 595abbca..8f0b2d93 100644 --- a/fleetspeak/src/client/internal/config/manager_test.go +++ b/fleetspeak/src/client/internal/config/manager_test.go @@ -29,7 +29,7 @@ func TestRekey(t *testing.T) { m, err := StartManager(&config.Configuration{ FixedServices: make([]*fspb.ClientServiceConfig, 0), - }, make(chan *fspb.ClientInfoData)) + }, make(chan *fspb.ClientInfoData), nil) if err != nil { t.Errorf("unable to create config manager: %v", err) return @@ -60,7 +60,7 @@ func TestWriteback(t *testing.T) { m1, err := StartManager(&config.Configuration{ PersistenceHandler: ph, - }, make(chan *fspb.ClientInfoData)) + }, make(chan *fspb.ClientInfoData), nil) if err != nil { t.Errorf("unable to create config manager: %v", err) return @@ -78,7 +78,7 @@ func TestWriteback(t *testing.T) { m2, err := StartManager(&config.Configuration{ PersistenceHandler: ph, - }, make(chan *fspb.ClientInfoData)) + }, make(chan *fspb.ClientInfoData), nil) if err != nil { t.Errorf("Unable to create new config manager: %v", err) return diff --git a/fleetspeak/src/client/stats/collector.go b/fleetspeak/src/client/stats/collector.go new file mode 100644 index 00000000..a29f4301 --- /dev/null +++ b/fleetspeak/src/client/stats/collector.go @@ -0,0 +1,30 @@ +// Copyright 2017 Google Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package stats contains interfaces and utilities relating to the collection of +// statistics from a fleetspeak server. +package stats + +// A Collector is a component which is notified when certain events occurred, to support +// performance monitoring of a fleetspeak installation. +type Collector interface { + // ConfigSynced is called when the client config is synced. + ConfigSynced(status error) +} + +// NoopCollector implements Collector by doing nothing. +type NoopCollector struct{} + +// ConfigSynced does nothing. +func (c NoopCollector) ConfigSynced(status error) {}