Merge pull request #155 from sourcegraph/sg/3-influx-config

InfluxDBStore: cleanup InfluxDBStore configuration by adding sane defaults

CHANGELOG.md

@@ -3,6 +3,7 @@
 - Apr 26, 2016
   - [#153](https://github.com/sourcegraph/appdash/pull/153) Added a Recorder.Logger field which, when non-nil, causes errors to be logged instead of checked explicitly via the Errors method.
   - [#154](https://github.com/sourcegraph/appdash/pull/154) Added trace permalinks which encode the trace within the URL.
+  - [#155](https://github.com/sourcegraph/appdash/pull/155) Cleaned up InfluxDBStore configuration by adding sane defaults.
 - Apr 15, 2016 - **Breaking Changes!**
   - [#136](https://github.com/sourcegraph/appdash/pull/136) Users must now call `Recorder.Finish` when finished recording, or else data    will not be collected.
   - [#136](https://github.com/sourcegraph/appdash/pull/136) AggregateStore is removed in favor of InfluxDBStore, which is also embeddable, and is generally faster and more reliable. Refer to the [cmd/webapp-influxdb](https://github.com/sourcegraph/appdash/blob/master/examples/cmd/webapp-influxdb/main.go#L50) for further information on how to migrate to `InfluxDBStore`, or [read more about why this change was made](https://github.com/sourcegraph/appdash/issues/137).

examples/cmd/webapp-influxdb/main.go

@@ -13,58 +13,42 @@ import (
 	"github.com/codegangsta/negroni"
 	"github.com/gorilla/context"
 	"github.com/gorilla/mux"
-
-	influxDBServer "github.com/influxdata/influxdb/cmd/influxd/run"
-	"github.com/influxdata/influxdb/toml"
 )
 
 const CtxSpanID = 0
 
 var collector appdash.Collector
 
 func main() {
-	conf, err := influxDBServer.NewDemoConfig()
+	// Create a default InfluxDB configuration.
+	conf, err := appdash.NewInfluxDBConfig()
 	if err != nil {
 		log.Fatalf("failed to create influxdb config, error: %v", err)
 	}
 
-	// Enables InfluxDB server authentication.
-	conf.HTTPD.AuthEnabled = true
-
-	// Enables retention policies which will be executed within an interval of 30 minutes.
-	conf.Retention.Enabled = true
-	conf.Retention.CheckInterval = toml.Duration(30 * time.Minute)
+	// Enable InfluxDB server HTTP basic auth.
+	conf.Server.HTTPD.AuthEnabled = true
+	conf.AdminUser = appdash.InfluxDBAdminUser{
+		Username: "demo",
+		Password: "demo",
+	}
 
 	// If you do not want metrics to be reported (see: https://docs.influxdata.com/influxdb/v0.10/administration/config/#reporting-disabled-false) uncomment the following line:
-	//conf.ReportingDisabled = true
-
-	// InfluxDB server auth credentials. If user does not exist yet it will
-	// be created as admin user.
-	user := appdash.InfluxDBAdminUser{Username: "demo", Password: "demo"}
-
-	// Retention policy named "one_day_only" with a duration of "1d" - meaning db data older than "1d" will be deleted
-	// with an interval checking set by `conf.Retention.CheckInterval`.
-	// Minimum duration time is 1 hour ("1h") - See: github.com/influxdata/influxdb/issues/5198
-	defaultRP := appdash.InfluxDBRetentionPolicy{Name: "one_day_only", Duration: "1d"}
+	//conf.Server.ReportingDisabled = true
 
 	// Configure InfluxDB ports, if you desire:
-	//conf.Admin.BindAddress = ":8083"
-	//conf.BindAddress = ":8088"
-	//conf.CollectdInputs[0].BindAddress = "" // auto-chosen
-	//conf.GraphiteInputs[0].BindAddress = ":2003"
-	//conf.HTTPD.BindAddress = ":8086"
-	//conf.OpenTSDBInputs[0].BindAddress = ":4242"
-	//conf.UDPInputs[0].BindAddress = "" // auto-chosen
-
-	store, err := appdash.NewInfluxDBStore(appdash.InfluxDBStoreConfig{
-		AdminUser: user,
-		BuildInfo: &influxDBServer.BuildInfo{},
-		DefaultRP: defaultRP,
-		Server:    conf,
-
-		// Configure where log output goes, if you desire:
-		//LogOutput: ioutil.Discard,
-	})
+	//conf.Server.Admin.BindAddress = ":8083"
+	//conf.Server.BindAddress = ":8088"
+	//conf.Server.CollectdInputs[0].BindAddress = "" // auto-chosen
+	//conf.Server.GraphiteInputs[0].BindAddress = ":2003"
+	//conf.Server.HTTPD.BindAddress = ":8086"
+	//conf.Server.OpenTSDBInputs[0].BindAddress = ":4242"
+	//conf.Server.UDPInputs[0].BindAddress = "" // auto-chosen
+
+	// Control where InfluxDB server logs are written to, if desired:
+	//conf.LogOutput = ioutil.Discard
+
+	store, err := appdash.NewInfluxDBStore(conf)
 	if err != nil {
 		log.Fatalf("failed to create influxdb store, error: %v", err)
 	}

influxdb_store.go

@@ -14,6 +14,7 @@ import (
 	influxDBServer "github.com/influxdata/influxdb/cmd/influxd/run"
 	influxDBModels "github.com/influxdata/influxdb/models"
 	influxDBErrors "github.com/influxdata/influxdb/services/meta"
+	"github.com/influxdata/influxdb/toml"
 )
 
 const (
@@ -47,14 +48,12 @@ var (
 type pointFields map[string]interface{}
 
 type InfluxDBStore struct {
-	adminUser    InfluxDBAdminUser       // InfluxDB server auth credentials.
-	con          *influxDBClient.Client  // InfluxDB client connection.
-	dbName       string                  // InfluxDB database name for this store.
-	defaultRP    InfluxDBRetentionPolicy // Default retention policy for `dbName`.
-	clientTarget *url.URL                // HTTP URL that the client should connect to,
+	config       *InfluxDBConfig
+	con          *influxDBClient.Client // InfluxDB client connection.
+	dbName       string                 // InfluxDB database name for this store.
+	clientTarget *url.URL               // HTTP URL that the client should connect to,
 
 	// When set to `testMode` - `testDBName` will be dropped and created, so newly database is ready for tests.
-	mode          mode                   // Used to check current mode(release or test).
 	server        *influxDBServer.Server // InfluxDB API server.
 	tracesPerPage int                    // Number of traces per page.
 }
@@ -399,14 +398,15 @@ func (in *InfluxDBStore) Close() error {
 func (in *InfluxDBStore) createDBIfNotExists() error {
 	q := fmt.Sprintf("CREATE DATABASE IF NOT EXISTS %s", in.dbName)
 
-	// If `in.defaultRP` info is provided, it's used to extend the query in order to create the database with
+	// If a default retention policy is provided, it's used to extend the query in order to create the database with
 	// a default retention policy.
-	if in.defaultRP.Duration != "" {
-		q = fmt.Sprintf("%s WITH DURATION %s", q, in.defaultRP.Duration)
+	rp := in.config.DefaultRP
+	if rp.Duration != "" {
+		q = fmt.Sprintf("%s WITH DURATION %s", q, rp.Duration)
 
 		// Retention policy name must be placed to the end of the query or it will be syntactically incorrect.
-		if in.defaultRP.Name != "" {
-			q = fmt.Sprintf("%s NAME %s", q, in.defaultRP.Name)
+		if rp.Name != "" {
+			q = fmt.Sprintf("%s NAME %s", q, rp.Name)
 		}
 	}
 
@@ -423,9 +423,10 @@ func (in *InfluxDBStore) createDBIfNotExists() error {
 
 // createAdminUserIfNotExists finds admin user(`in.adminUser`) if not found it's created.
 func (in *InfluxDBStore) createAdminUserIfNotExists() error {
-	userInfo, err := in.server.MetaClient.Authenticate(in.adminUser.Username, in.adminUser.Password)
+	admin := in.config.AdminUser
+	userInfo, err := in.server.MetaClient.Authenticate(admin.Username, admin.Password)
 	if err == influxDBErrors.ErrUserNotFound {
-		if _, createUserErr := in.server.MetaClient.CreateUser(in.adminUser.Username, in.adminUser.Password, true); createUserErr != nil {
+		if _, createUserErr := in.server.MetaClient.CreateUser(admin.Username, admin.Password, true); createUserErr != nil {
 			return createUserErr
 		}
 		return nil
@@ -463,8 +464,8 @@ func (in *InfluxDBStore) init(server *influxDBServer.Server) error {
 	// We're currently using v1.
 	con, err := influxDBClient.NewClient(influxDBClient.Config{
 		URL:      *in.clientTarget,
-		Username: in.adminUser.Username,
-		Password: in.adminUser.Password,
+		Username: in.config.AdminUser.Username,
+		Password: in.config.AdminUser.Password,
 	})
 	if err != nil {
 		return err
@@ -473,7 +474,7 @@ func (in *InfluxDBStore) init(server *influxDBServer.Server) error {
 	if err := in.createAdminUserIfNotExists(); err != nil {
 		return err
 	}
-	switch in.mode {
+	switch in.config.Mode {
 	case testMode:
 		if err := in.setUpTestMode(); err != nil {
 			return err
@@ -487,7 +488,7 @@ func (in *InfluxDBStore) init(server *influxDBServer.Server) error {
 		return err
 	}
 
-	// TODO: let lib users decide `in.tracesPerPage` through InfluxDBStoreConfig.
+	// TODO: let lib users decide `in.tracesPerPage` through InfluxDBConfig.
 	in.tracesPerPage = defaultTracesPerPage
 	return nil
 }
@@ -772,28 +773,9 @@ func spansFromRow(row influxDBModels.Row) ([]*Span, error) {
 	return spans, nil
 }
 
-type InfluxDBRetentionPolicy struct {
-	Name     string // Name used to indentify this retention policy.
-	Duration string // How long InfluxDB keeps the data. Eg: "1h", "1d", "1w".
-}
-
-type InfluxDBStoreConfig struct {
-	AdminUser InfluxDBAdminUser
-	BuildInfo *influxDBServer.BuildInfo
-	DefaultRP InfluxDBRetentionPolicy
-	Mode      mode
-	Server    *influxDBServer.Config
-
-	// LogOutput, if specified, controls where all InfluxDB logs are written to.
-	LogOutput io.Writer
-}
-
-type InfluxDBAdminUser struct {
-	Username string
-	Password string
-}
-
-func NewInfluxDBStore(config InfluxDBStoreConfig) (*InfluxDBStore, error) {
+// NewInfluxDBStore returns a new InfluxDB-backed store. It starts an
+// in-process / embedded InfluxDB server.
+func NewInfluxDBStore(config *InfluxDBConfig) (*InfluxDBStore, error) {
 	s, err := influxDBServer.NewServer(config.Server, config.BuildInfo)
 	if err != nil {
 		return nil, err
@@ -816,13 +798,63 @@ func NewInfluxDBStore(config InfluxDBStoreConfig) (*InfluxDBStore, error) {
 		return nil, err
 	}
 	in := InfluxDBStore{
-		adminUser:    config.AdminUser,
-		defaultRP:    config.DefaultRP,
-		mode:         config.Mode,
+		config:       config,
 		clientTarget: clientTarget,
 	}
 	if err := in.init(s); err != nil {
 		return nil, err
 	}
 	return &in, nil
 }
+
+// NewInfluxDBConfig returns a new InfluxDBConfig with the default values.
+func NewInfluxDBConfig() (*InfluxDBConfig, error) {
+	// Create the default InfluxDB server configuration.
+	server, err := influxDBServer.NewDemoConfig()
+	if err != nil {
+		return nil, err
+	}
+
+	// Enables retention policies which will be executed within an interval of 30 minutes.
+	server.Retention.Enabled = true
+	server.Retention.CheckInterval = toml.Duration(30 * time.Minute)
+
+	return &InfluxDBConfig{
+		Server: server,
+
+		// Specify the branch as "appdash" just for identification purposes.
+		BuildInfo: &influxDBServer.BuildInfo{
+			Branch: "appdash",
+		},
+
+		// Create a retention policy which keeps data for only three days, this is
+		// because the Dashboard is hard-coded to displaying a 72hr timeline.
+		//
+		// Minimum duration time is 1 hour ("1h") - See: github.com/influxdata/influxdb/issues/5198
+		DefaultRP: InfluxDBRetentionPolicy{
+			Name:     "three_days_only",
+			Duration: "3d",
+		},
+	}, nil
+}
+
+type InfluxDBConfig struct {
+	AdminUser InfluxDBAdminUser
+	BuildInfo *influxDBServer.BuildInfo
+	DefaultRP InfluxDBRetentionPolicy
+	Mode      mode
+	Server    *influxDBServer.Config
+
+	// LogOutput, if specified, controls where all InfluxDB logs are written to.
+	LogOutput io.Writer
+}
+
+type InfluxDBRetentionPolicy struct {
+	Name     string // Name used to indentify this retention policy.
+	Duration string // How long InfluxDB keeps the data. Eg: "1h", "1d", "1w".
+}
+
+type InfluxDBAdminUser struct {
+	Username string
+	Password string
+}

influxdb_store_test.go

@@ -5,8 +5,6 @@ import (
 	"sort"
 	"strings"
 	"testing"
-
-	influxDBServer "github.com/influxdata/influxdb/cmd/influxd/run"
 )
 
 func TestAddChildren(t *testing.T) {
@@ -543,23 +541,26 @@ func benchmarkInfluxDBStoreCreateTraces(store *InfluxDBStore, n int) ([]*Trace,
 }
 
 func newTestInfluxDBStore() (*InfluxDBStore, error) {
-	conf, err := influxDBServer.NewDemoConfig()
+	// Create a default InfluxDB configuration.
+	conf, err := NewInfluxDBConfig()
 	if err != nil {
 		return nil, err
 	}
-	conf.Data.QueryLogEnabled = false
-	conf.HTTPD.AuthEnabled = true
-	conf.HTTPD.LogEnabled = false
-	conf.ReportingDisabled = true
-	user := InfluxDBAdminUser{Username: "demo", Password: "demo"}
-	defaultRP := InfluxDBRetentionPolicy{Name: "one_hour_only", Duration: "1h"}
-	store, err := NewInfluxDBStore(InfluxDBStoreConfig{
-		AdminUser: user,
-		BuildInfo: &influxDBServer.BuildInfo{},
-		DefaultRP: defaultRP,
-		Mode:      testMode,
-		Server:    conf,
-	})
+
+	// Enable InfluxDB server HTTP basic auth.
+	conf.Server.HTTPD.AuthEnabled = true
+	conf.AdminUser = InfluxDBAdminUser{
+		Username: "demo",
+		Password: "demo",
+	}
+
+	// Disable metrics reporting because we're just a test.
+	conf.Server.ReportingDisabled = true
+
+	// Switch mode to testMode.
+	conf.Mode = testMode
+
+	store, err := NewInfluxDBStore(conf)
 	if err != nil {
 		return nil, err
 	}