Promtail: use --client.external-labels for all clients (#2474)

* --client.external-labels command line argument will now apply the labels to all clients configured in the clients config section of the config file.

* do better Ed

docs/sources/clients/promtail/configuration.md

@@ -258,6 +258,14 @@ backoff_config:
 # Static labels to add to all logs being sent to Loki.
 # Use map like {"foo": "bar"} to add a label foo with
 # value bar.
+# These can also be specified from command line:
+# -client.external-labels=k1=v1,k2=v2 
+# (or --client.external-labels depending on your OS)
+# labels supplied by the command line are applied 
+# to all clients configured in the `clients` section.
+# NOTE: values defined in the config file will replace values
+# defined on the command line for a given client if the 
+# label keys are the same.
 external_labels:
   [ <labelname>: <labelvalue> ... ]
 

pkg/promtail/client/logger.go

@@ -12,6 +12,8 @@ import (
 	"github.com/go-kit/kit/log"
 	"github.com/prometheus/common/model"
 	"gopkg.in/yaml.v2"
+
+	lokiflag "github.com/grafana/loki/pkg/util/flagext"
 )
 
 var (
@@ -32,9 +34,9 @@ type logger struct {
 }
 
 // NewLogger creates a new client logger that logs entries instead of sending them.
-func NewLogger(log log.Logger, cfgs ...Config) (Client, error) {
+func NewLogger(log log.Logger, externalLabels lokiflag.LabelSet, cfgs ...Config) (Client, error) {
 	// make sure the clients config is valid
-	c, err := NewMulti(log, cfgs...)
+	c, err := NewMulti(log, externalLabels, cfgs...)
 	if err != nil {
 		return nil, err
 	}

pkg/promtail/client/logger_test.go

@@ -6,16 +6,18 @@ import (
 	"time"
 
 	"github.com/cortexproject/cortex/pkg/util"
-	"github.com/cortexproject/cortex/pkg/util/flagext"
+	cortexflag "github.com/cortexproject/cortex/pkg/util/flagext"
 	"github.com/prometheus/common/model"
 	"github.com/stretchr/testify/require"
+
+	"github.com/grafana/loki/pkg/util/flagext"
 )
 
 func TestNewLogger(t *testing.T) {
-	_, err := NewLogger(util.Logger, []Config{}...)
+	_, err := NewLogger(util.Logger, flagext.LabelSet{}, []Config{}...)
 	require.Error(t, err)
 
-	l, err := NewLogger(util.Logger, []Config{{URL: flagext.URLValue{URL: &url.URL{Host: "string"}}}}...)
+	l, err := NewLogger(util.Logger, flagext.LabelSet{}, []Config{{URL: cortexflag.URLValue{URL: &url.URL{Host: "string"}}}}...)
 	require.NoError(t, err)
 	err = l.Handle(model.LabelSet{"foo": "bar"}, time.Now(), "entry")
 	require.NoError(t, err)

pkg/promtail/client/multi.go

@@ -8,19 +8,27 @@ import (
 	"github.com/prometheus/common/model"
 
 	"github.com/grafana/loki/pkg/util"
+	"github.com/grafana/loki/pkg/util/flagext"
 )
 
 // MultiClient is client pushing to one or more loki instances.
 type MultiClient []Client
 
 // NewMulti creates a new client
-func NewMulti(logger log.Logger, cfgs ...Config) (Client, error) {
+func NewMulti(logger log.Logger, externalLabels flagext.LabelSet, cfgs ...Config) (Client, error) {
 	if len(cfgs) == 0 {
 		return nil, errors.New("at least one client config should be provided")
 	}
 
 	clients := make([]Client, 0, len(cfgs))
 	for _, cfg := range cfgs {
+		// Merge the provided external labels from the single client config/command line with each client config from
+		// `clients`. This is done to allow --client.external-labels=key=value passed at command line to apply to all clients
+		// The order here is specified to allow the yaml to override the command line flag if there are any labels
+		// which exist in both the command line arguments as well as the yaml, and while this is
+		// not typically the order of precedence, the assumption here is someone providing a specific config in
+		// yaml is doing so explicitly to make a key specific to a client.
+		cfg.ExternalLabels = flagext.LabelSet{externalLabels.Merge(cfg.ExternalLabels.LabelSet)}
 		client, err := New(cfg, logger)
 		if err != nil {
 			return nil, err

pkg/promtail/client/multi_test.go

@@ -7,43 +7,73 @@ import (
 	"testing"
 	"time"
 
-	"github.com/grafana/loki/pkg/promtail/api"
-
 	"github.com/cortexproject/cortex/pkg/util"
 	"github.com/cortexproject/cortex/pkg/util/flagext"
 	"github.com/prometheus/common/model"
 
+	"github.com/grafana/loki/pkg/promtail/api"
+	lokiflag "github.com/grafana/loki/pkg/util/flagext"
+
 	"github.com/grafana/loki/pkg/promtail/client/fake"
 )
 
 func TestNewMulti(t *testing.T) {
-	_, err := NewMulti(util.Logger, []Config{}...)
+	_, err := NewMulti(util.Logger, lokiflag.LabelSet{}, []Config{}...)
 	if err == nil {
 		t.Fatal("expected err but got nil")
 	}
 	host1, _ := url.Parse("http://localhost:3100")
 	host2, _ := url.Parse("https://grafana.com")
-	expectedCfg1 := Config{BatchSize: 20, BatchWait: 1 * time.Second, URL: flagext.URLValue{URL: host1}}
-	expectedCfg2 := Config{BatchSize: 10, BatchWait: 1 * time.Second, URL: flagext.URLValue{URL: host2}}
+	cc1 := Config{
+		BatchSize:      20,
+		BatchWait:      1 * time.Second,
+		URL:            flagext.URLValue{URL: host1},
+		ExternalLabels: lokiflag.LabelSet{LabelSet: model.LabelSet{"order": "yaml"}},
+	}
+	cc2 := Config{
+		BatchSize:      10,
+		BatchWait:      1 * time.Second,
+		URL:            flagext.URLValue{URL: host2},
+		ExternalLabels: lokiflag.LabelSet{LabelSet: model.LabelSet{"hi": "there"}},
+	}
 
-	clients, err := NewMulti(util.Logger, expectedCfg1, expectedCfg2)
+	clients, err := NewMulti(util.Logger, lokiflag.LabelSet{LabelSet: model.LabelSet{"order": "command"}}, cc1, cc2)
 	if err != nil {
 		t.Fatalf("expected err: nil got:%v", err)
 	}
 	multi := clients.(MultiClient)
 	if len(multi) != 2 {
 		t.Fatalf("expected client: 2 got:%d", len(multi))
 	}
-	cfg1 := clients.(MultiClient)[0].(*client).cfg
+	actualCfg1 := clients.(MultiClient)[0].(*client).cfg
+	// Yaml should overried the command line so 'order: yaml' should be expected
+	expectedCfg1 := Config{
+		BatchSize:      20,
+		BatchWait:      1 * time.Second,
+		URL:            flagext.URLValue{URL: host1},
+		ExternalLabels: lokiflag.LabelSet{LabelSet: model.LabelSet{"order": "yaml"}},
+	}
 
-	if !reflect.DeepEqual(cfg1, expectedCfg1) {
-		t.Fatalf("expected cfg: %v got:%v", expectedCfg1, cfg1)
+	if !reflect.DeepEqual(actualCfg1, expectedCfg1) {
+		t.Fatalf("expected cfg: %v got:%v", expectedCfg1, actualCfg1)
 	}
 
-	cfg2 := clients.(MultiClient)[1].(*client).cfg
+	actualCfg2 := clients.(MultiClient)[1].(*client).cfg
+	// No overlapping label keys so both should be in the output
+	expectedCfg2 := Config{
+		BatchSize: 10,
+		BatchWait: 1 * time.Second,
+		URL:       flagext.URLValue{URL: host2},
+		ExternalLabels: lokiflag.LabelSet{
+			LabelSet: model.LabelSet{
+				"order": "command",
+				"hi":    "there",
+			},
+		},
+	}
 
-	if !reflect.DeepEqual(cfg2, expectedCfg2) {
-		t.Fatalf("expected cfg: %v got:%v", expectedCfg2, cfg2)
+	if !reflect.DeepEqual(actualCfg2, expectedCfg2) {
+		t.Fatalf("expected cfg: %v got:%v", expectedCfg2, actualCfg2)
 	}
 }
 

pkg/promtail/promtail.go

@@ -52,13 +52,13 @@ func New(cfg config.Config, dryRun bool, opts ...Option) (*Promtail, error) {
 
 	var err error
 	if dryRun {
-		promtail.client, err = client.NewLogger(promtail.logger, cfg.ClientConfigs...)
+		promtail.client, err = client.NewLogger(promtail.logger, cfg.ClientConfig.ExternalLabels, cfg.ClientConfigs...)
 		if err != nil {
 			return nil, err
 		}
 		cfg.PositionsConfig.ReadOnly = true
 	} else {
-		promtail.client, err = client.NewMulti(promtail.logger, cfg.ClientConfigs...)
+		promtail.client, err = client.NewMulti(promtail.logger, cfg.ClientConfig.ExternalLabels, cfg.ClientConfigs...)
 		if err != nil {
 			return nil, err
 		}