Make aggregationFile an optional setting

cfg/table.go

@@ -210,13 +210,12 @@ func InitRoutes(table table.Interface, config Config, meta toml.MetaData) error
 			table.AddRoute(route)
 		case "grafanaNet":
 
-			cfg, err := route.NewGrafanaNetConfig(routeConfig.Addr, routeConfig.ApiKey, routeConfig.SchemasFile)
+			cfg, err := route.NewGrafanaNetConfig(routeConfig.Addr, routeConfig.ApiKey, routeConfig.SchemasFile, routeConfig.AggregationFile)
 			if err != nil {
 				log.Error(err.Error())
 				log.Info("grafanaNet route configuration details: https://github.com/grafana/carbon-relay-ng/blob/master/docs/config.md#grafananet-route")
 				return fmt.Errorf("error adding route '%s'", routeConfig.Key)
 			}
-			cfg.AggregationFile = routeConfig.AggregationFile
 
 			// by merely looking at a boolean field we can't differentiate between:
 			// * the user not specifying the option (which should leave our config unaffected)

cfg/table_test.go

@@ -29,7 +29,7 @@ func TestTomlToGrafanaNetRoute(t *testing.T) {
 
 	var testCases []testCase
 
-	cfg, err := route.NewGrafanaNetConfig("http://foo/metrics", "apiKey", schemasFile.Name())
+	cfgWithAgg, err := route.NewGrafanaNetConfig("http://foo/metrics", "apiKey", schemasFile.Name(), "")
 	if err != nil {
 		t.Fatal(err) // should never happen
 	}
@@ -42,7 +42,7 @@ type = 'grafanaNet'
 addr = 'http://foo/metrics'
 apikey = 'apiKey'
 schemasFile = '` + schemasFile.Name() + `'`,
-		expCfg: cfg,
+		expCfg: cfgWithAgg,
 		expErr: false,
 	})
 

imperatives/imperatives.go

@@ -510,7 +510,9 @@ func readAddRouteGrafanaNet(s *toki.Scanner, table table.Interface) error {
 	}
 	schemasFile := string(t.Value)
 
-	cfg, err := route.NewGrafanaNetConfig(addr, apiKey, schemasFile)
+	// The aggregationFile argument is blank - it will be set later if it's found
+	// in the list of optional arguments
+	cfg, err := route.NewGrafanaNetConfig(addr, apiKey, schemasFile, "")
 	if err != nil {
 		return errFmtAddRouteGrafanaNet
 	}

imperatives/imperatives_test.go

@@ -93,7 +93,7 @@ func TestApplyAddRouteGrafanaNet(t *testing.T) {
 	var testCases []testCase
 
 	// trivial case. mostly defaults, so let's rely on the helper that generates the (mostly default) config
-	cfg, err := route.NewGrafanaNetConfig("http://foo/metrics", "apiKey", schemasFile.Name())
+	cfg, err := route.NewGrafanaNetConfig("http://foo/metrics", "apiKey", schemasFile.Name(), "")
 	if err != nil {
 		t.Fatal(err) // should never happen
 	}

route/grafananet.go

@@ -54,7 +54,7 @@ type GrafanaNetConfig struct {
 	ErrBackoffFactor float64
 }
 
-func NewGrafanaNetConfig(addr, apiKey, schemasFile string) (GrafanaNetConfig, error) {
+func NewGrafanaNetConfig(addr, apiKey, schemasFile, aggregationFile string) (GrafanaNetConfig, error) {
 
 	u, err := url.Parse(addr)
 	if err != nil || !u.IsAbs() || u.Host == "" { // apparently "http://" is a valid absolute URL (with empty host), but we don't want that
@@ -77,10 +77,18 @@ func NewGrafanaNetConfig(addr, apiKey, schemasFile string) (GrafanaNetConfig, er
 		return GrafanaNetConfig{}, fmt.Errorf("NewGrafanaNetConfig: could not read schemasFile %q: %s", schemasFile, err.Error())
 	}
 
+	if aggregationFile != "" {
+		_, err = conf.ReadAggregations(aggregationFile)
+		if err != nil {
+			return GrafanaNetConfig{}, fmt.Errorf("NewGrafanaNetConfig: could not read aggregationFile %q: %s", aggregationFile, err.Error())
+		}
+	}
+
 	return GrafanaNetConfig{
-		Addr:        addr,
-		ApiKey:      apiKey,
-		SchemasFile: schemasFile,
+		Addr:            addr,
+		ApiKey:          apiKey,
+		SchemasFile:     schemasFile,
+		AggregationFile: aggregationFile,
 
 		BufSize:      1e7, // since a message is typically around 100B this is 1GB
 		FlushMaxNum:  5000,
@@ -163,7 +171,7 @@ func NewGrafanaNet(key string, matcher matcher.Matcher, cfg GrafanaNetConfig) (R
 	if cfg.AggregationFile != "" {
 		aggregation, err = conf.ReadAggregations(cfg.AggregationFile)
 		if err != nil {
-			return nil, fmt.Errorf("NewGrafanaNet: could not read aggregationFile %q: %s", cfg.AggregationFile, err.Error())
+			return nil, err
 		}
 		aggregationStr = aggregation.String()
 	}

route/grafananet_test.go

@@ -63,25 +63,34 @@ func TestNewGrafanaNetConfig(t *testing.T) {
 		{otherFile.Name(), true},
 		{schemasFile.Name(), false},
 	}
+	aggregationFileOptions := []option{
+		{"", false},
+		{"some-path-that-definitely-will-not-exist-for-carbon-relay-ng", true},
+		{otherFile.Name(), true},
+		{aggregationFile.Name(), false},
+	}
 
 	var testCases []testCase
 	for _, addr := range addrOptions {
 		for _, key := range keyOptions {
 			for _, schemasFile := range schemasFileOptions {
-				testCases = append(testCases, testCase{
-					in: input{
-						addr:        addr.str,
-						apiKey:      key.str,
-						schemasFile: schemasFile.str,
-					},
-					expErr: addr.expErr || key.expErr || schemasFile.expErr,
-				})
+				for _, aggregationFile := range aggregationFileOptions {
+					testCases = append(testCases, testCase{
+						in: input{
+							addr:            addr.str,
+							apiKey:          key.str,
+							schemasFile:     schemasFile.str,
+							aggregationFile: aggregationFile.str,
+						},
+						expErr: addr.expErr || key.expErr || schemasFile.expErr || aggregationFile.expErr,
+					})
+				}
 			}
 		}
 	}
 
 	for _, testCase := range testCases {
-		_, err := NewGrafanaNetConfig(testCase.in.addr, testCase.in.apiKey, testCase.in.schemasFile)
+		_, err := NewGrafanaNetConfig(testCase.in.addr, testCase.in.apiKey, testCase.in.schemasFile, testCase.in.aggregationFile)
 		if !testCase.expErr && err != nil {
 			t.Errorf("test with input %+v expected no error but got %s", testCase.in, err.Error())
 		}