null object

main.go

@@ -72,7 +72,7 @@ type configuration struct {
 // with all signers and permissions configured
 type autographer struct {
 	db                   *database.Handler
-	stats                *statsd.Client
+	stats                statsd.ClientInterface
 	nonces               *lru.Cache
 	debug                bool
 	heartbeatConf        *heartbeatConfig
@@ -166,11 +166,10 @@ func run(conf configuration, listen string, debug bool) {
 		ag.initHSM(conf)
 	}
 
-	if conf.Statsd.Addr != "" {
-		err = ag.addStats(conf)
-		if err != nil {
-			log.Fatal(err)
-		}
+	// FIXME add TODO or some up with better pattern than this `addStats` stuff. This god object setter pattern is terrible and confusing.
+	err = ag.addStats(conf)
+	if err != nil {
+		log.Fatal(err)
 	}
 
 	err = ag.addSigners(conf.Signers)

signer/signer.go

@@ -523,14 +523,11 @@ type StatsClient struct {
 	signerTags []string
 
 	// stats is the statsd client for reporting metrics
-	stats *statsd.Client
+	stats statsd.ClientInterface
 }
 
 // NewStatsClient makes a new stats client
-func NewStatsClient(signerConfig Configuration, stats *statsd.Client) (*StatsClient, error) {
-	if stats == nil {
-		return nil, fmt.Errorf("xpi: statsd client is nil. Could not create StatsClient for signer %s", signerConfig.ID)
-	}
+func NewStatsClient(signerConfig Configuration, stats statsd.ClientInterface) (*StatsClient, error) {
 	return &StatsClient{
 		stats: stats,
 		signerTags: []string{

signer/xpi/xpi_test.go

@@ -57,6 +57,7 @@ func TestSignFile(t *testing.T) {
 					StatsSampleRate:        10 * time.Second,
 				}
 
+				// FIXME there's a MockClient we can use instead of needing a running statsd
 				statsdClient, err := statsd.NewBuffered("localhost:8135", 1)
 				if err != nil {
 					t.Fatalf("passing testcase %d: Error constructing statsdClient: %v", i, err)

stats.go

@@ -19,7 +19,13 @@ func loadStatsd(conf configuration) (*statsd.Client, error) {
 }
 
 func (a *autographer) addStats(conf configuration) (err error) {
+	if conf.Statsd.Addr != "" {
+		a.stats = &statsd.NoOpClient{}
+		log.Infof("Statsd disabled as no statsd address was provided")
+		return nil
+	}
 	a.stats, err = loadStatsd(conf)
+	// FIXME if it returned an error this shouldn't be logged
 	log.Infof("Statsd enabled at %s with namespace %s", conf.Statsd.Addr, conf.Statsd.Namespace)
 	return err
 }