diff --git a/clients/pkg/promtail/client/logger.go b/clients/pkg/promtail/client/logger.go index e2ad615679e37..6b723dc354460 100644 --- a/clients/pkg/promtail/client/logger.go +++ b/clients/pkg/promtail/client/logger.go @@ -13,8 +13,6 @@ import ( "gopkg.in/yaml.v2" "github.com/grafana/loki/clients/pkg/promtail/api" - - lokiflag "github.com/grafana/loki/pkg/util/flagext" ) var ( @@ -38,9 +36,9 @@ type logger struct { } // NewLogger creates a new client logger that logs entries instead of sending them. -func NewLogger(reg prometheus.Registerer, log log.Logger, externalLabels lokiflag.LabelSet, cfgs ...Config) (Client, error) { +func NewLogger(reg prometheus.Registerer, log log.Logger, cfgs ...Config) (Client, error) { // make sure the clients config is valid - c, err := NewMulti(reg, log, externalLabels, cfgs...) + c, err := NewMulti(reg, log, cfgs...) if err != nil { return nil, err } @@ -82,6 +80,5 @@ func (l *logger) run() { fmt.Fprint(l.Writer, "\n") l.Flush() } - } func (l *logger) StopNow() { l.Stop() } diff --git a/clients/pkg/promtail/client/logger_test.go b/clients/pkg/promtail/client/logger_test.go index 6ebfd464844e0..f76825851a002 100644 --- a/clients/pkg/promtail/client/logger_test.go +++ b/clients/pkg/promtail/client/logger_test.go @@ -13,14 +13,13 @@ import ( "github.com/grafana/loki/clients/pkg/promtail/api" "github.com/grafana/loki/pkg/logproto" - "github.com/grafana/loki/pkg/util/flagext" ) func TestNewLogger(t *testing.T) { - _, err := NewLogger(nil, util_log.Logger, flagext.LabelSet{}, []Config{}...) + _, err := NewLogger(nil, util_log.Logger, []Config{}...) require.Error(t, err) - l, err := NewLogger(nil, util_log.Logger, flagext.LabelSet{}, []Config{{URL: cortexflag.URLValue{URL: &url.URL{Host: "string"}}}}...) + l, err := NewLogger(nil, util_log.Logger, []Config{{URL: cortexflag.URLValue{URL: &url.URL{Host: "string"}}}}...) require.NoError(t, err) l.Chan() <- api.Entry{Labels: model.LabelSet{"foo": "bar"}, Entry: logproto.Entry{Timestamp: time.Now(), Line: "entry"}} l.Stop() diff --git a/clients/pkg/promtail/client/multi.go b/clients/pkg/promtail/client/multi.go index 0759c4161f8c2..7c66f627b1839 100644 --- a/clients/pkg/promtail/client/multi.go +++ b/clients/pkg/promtail/client/multi.go @@ -8,8 +8,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/grafana/loki/clients/pkg/promtail/api" - - "github.com/grafana/loki/pkg/util/flagext" ) // MultiClient is client pushing to one or more loki instances. @@ -22,20 +20,13 @@ type MultiClient struct { } // NewMulti creates a new client -func NewMulti(reg prometheus.Registerer, logger log.Logger, externalLabels flagext.LabelSet, cfgs ...Config) (Client, error) { +func NewMulti(reg prometheus.Registerer, logger log.Logger, 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{LabelSet: externalLabels.Merge(cfg.ExternalLabels.LabelSet)} client, err := New(reg, cfg, logger) if err != nil { return nil, err diff --git a/clients/pkg/promtail/client/multi_test.go b/clients/pkg/promtail/client/multi_test.go index b0787aec7da36..92a07f39c96c7 100644 --- a/clients/pkg/promtail/client/multi_test.go +++ b/clients/pkg/promtail/client/multi_test.go @@ -22,7 +22,7 @@ import ( ) func TestNewMulti(t *testing.T) { - _, err := NewMulti(nil, util_log.Logger, lokiflag.LabelSet{}, []Config{}...) + _, err := NewMulti(nil, util_log.Logger, []Config{}...) if err == nil { t.Fatal("expected err but got nil") } @@ -41,7 +41,7 @@ func TestNewMulti(t *testing.T) { ExternalLabels: lokiflag.LabelSet{LabelSet: model.LabelSet{"hi": "there"}}, } - clients, err := NewMulti(prometheus.DefaultRegisterer, util_log.Logger, lokiflag.LabelSet{LabelSet: model.LabelSet{"order": "command"}}, cc1, cc2) + clients, err := NewMulti(prometheus.DefaultRegisterer, util_log.Logger, cc1, cc2) if err != nil { t.Fatalf("expected err: nil got:%v", err) } @@ -50,7 +50,7 @@ func TestNewMulti(t *testing.T) { t.Fatalf("expected client: 2 got:%d", len(multi.clients)) } actualCfg1 := clients.(*MultiClient).clients[0].(*client).cfg - // Yaml should overried the command line so 'order: yaml' should be expected + // Yaml should overridden the command line so 'order: yaml' should be expected expectedCfg1 := Config{ BatchSize: 20, BatchWait: 1 * time.Second, @@ -61,24 +61,6 @@ func TestNewMulti(t *testing.T) { if !reflect.DeepEqual(actualCfg1, expectedCfg1) { t.Fatalf("expected cfg: %v got:%v", expectedCfg1, actualCfg1) } - - actualCfg2 := clients.(*MultiClient).clients[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(actualCfg2, expectedCfg2) { - t.Fatalf("expected cfg: %v got:%v", expectedCfg2, actualCfg2) - } } func TestMultiClient_Stop(t *testing.T) { diff --git a/clients/pkg/promtail/config/config.go b/clients/pkg/promtail/config/config.go index 6fbcae9119082..90bd3344c6e39 100644 --- a/clients/pkg/promtail/config/config.go +++ b/clients/pkg/promtail/config/config.go @@ -11,6 +11,8 @@ import ( "github.com/grafana/loki/clients/pkg/promtail/scrapeconfig" "github.com/grafana/loki/clients/pkg/promtail/server" "github.com/grafana/loki/clients/pkg/promtail/targets/file" + + "github.com/grafana/loki/pkg/util/flagext" ) // Config for promtail, describing what files to watch. @@ -45,3 +47,31 @@ func (c Config) String() string { } return string(b) } + +func (c *Config) Setup() { + if c.ClientConfig.URL.URL != nil { + // if a single client config is used we add it to the multiple client config for backward compatibility + c.ClientConfigs = append(c.ClientConfigs, c.ClientConfig) + } + + // This is a bit crude but if the Loki Push API target is specified, + // force the log level to match the promtail log level + for i := range c.ScrapeConfig { + if c.ScrapeConfig[i].PushConfig != nil { + c.ScrapeConfig[i].PushConfig.Server.LogLevel = c.ServerConfig.LogLevel + c.ScrapeConfig[i].PushConfig.Server.LogFormat = c.ServerConfig.LogFormat + } + } + + // 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. + if len(c.ClientConfig.ExternalLabels.LabelSet) > 0 { + for i := range c.ClientConfigs { + c.ClientConfigs[i].ExternalLabels = flagext.LabelSet{LabelSet: c.ClientConfig.ExternalLabels.LabelSet.Merge(c.ClientConfigs[i].ExternalLabels.LabelSet)} + } + } +} diff --git a/clients/pkg/promtail/config/config_test.go b/clients/pkg/promtail/config/config_test.go index 8c80abb02e88d..661b7b96ae845 100644 --- a/clients/pkg/promtail/config/config_test.go +++ b/clients/pkg/promtail/config/config_test.go @@ -1,10 +1,18 @@ package config import ( + "fmt" + "net/url" "testing" + dskitflagext "github.com/grafana/dskit/flagext" + "github.com/prometheus/common/model" "github.com/stretchr/testify/require" "gopkg.in/yaml.v2" + + "github.com/grafana/loki/clients/pkg/promtail/client" + + "github.com/grafana/loki/pkg/util/flagext" ) const testFile = ` @@ -28,8 +36,91 @@ scrape_configs: ` func Test_Load(t *testing.T) { - var dst Config err := yaml.Unmarshal([]byte(testFile), &dst) require.Nil(t, err) } + +func TestConfig_Setup(t *testing.T) { + for i, tt := range []struct { + in Config + expected Config + }{ + { + Config{ + ClientConfig: client.Config{ + ExternalLabels: flagext.LabelSet{LabelSet: model.LabelSet{"foo": "bar"}}, + }, + ClientConfigs: []client.Config{ + { + ExternalLabels: flagext.LabelSet{LabelSet: model.LabelSet{"client1": "1"}}, + }, + { + ExternalLabels: flagext.LabelSet{LabelSet: model.LabelSet{"client2": "2"}}, + }, + }, + }, + Config{ + ClientConfig: client.Config{ + ExternalLabels: flagext.LabelSet{LabelSet: model.LabelSet{"foo": "bar"}}, + }, + ClientConfigs: []client.Config{ + { + ExternalLabels: flagext.LabelSet{LabelSet: model.LabelSet{"client1": "1", "foo": "bar"}}, + }, + { + ExternalLabels: flagext.LabelSet{LabelSet: model.LabelSet{"client2": "2", "foo": "bar"}}, + }, + }, + }, + }, + { + Config{ + ClientConfig: client.Config{ + ExternalLabels: flagext.LabelSet{LabelSet: model.LabelSet{"foo": "bar"}}, + URL: dskitflagext.URLValue{URL: mustURL("http://foo")}, + }, + ClientConfigs: []client.Config{ + { + ExternalLabels: flagext.LabelSet{LabelSet: model.LabelSet{"client1": "1"}}, + }, + { + ExternalLabels: flagext.LabelSet{LabelSet: model.LabelSet{"client2": "2"}}, + }, + }, + }, + Config{ + ClientConfig: client.Config{ + ExternalLabels: flagext.LabelSet{LabelSet: model.LabelSet{"foo": "bar"}}, + URL: dskitflagext.URLValue{URL: mustURL("http://foo")}, + }, + ClientConfigs: []client.Config{ + { + ExternalLabels: flagext.LabelSet{LabelSet: model.LabelSet{"client1": "1", "foo": "bar"}}, + }, + { + ExternalLabels: flagext.LabelSet{LabelSet: model.LabelSet{"client2": "2", "foo": "bar"}}, + }, + { + ExternalLabels: flagext.LabelSet{LabelSet: model.LabelSet{"foo": "bar"}}, + URL: dskitflagext.URLValue{URL: mustURL("http://foo")}, + }, + }, + }, + }, + } { + tt := tt + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + tt.in.Setup() + require.Equal(t, tt.expected, tt.in) + }) + } +} + +func mustURL(u string) *url.URL { + res, err := url.Parse(u) + if err != nil { + panic(err) + } + return res +} diff --git a/clients/pkg/promtail/promtail.go b/clients/pkg/promtail/promtail.go index 462e4136295e1..e058a24d74da1 100644 --- a/clients/pkg/promtail/promtail.go +++ b/clients/pkg/promtail/promtail.go @@ -55,29 +55,17 @@ func New(cfg config.Config, dryRun bool, opts ...Option) (*Promtail, error) { o(promtail) } - if cfg.ClientConfig.URL.URL != nil { - // if a single client config is used we add it to the multiple client config for backward compatibility - cfg.ClientConfigs = append(cfg.ClientConfigs, cfg.ClientConfig) - } - - // This is a bit crude but if the Loki Push API target is specified, - // force the log level to match the promtail log level - for i := range cfg.ScrapeConfig { - if cfg.ScrapeConfig[i].PushConfig != nil { - cfg.ScrapeConfig[i].PushConfig.Server.LogLevel = cfg.ServerConfig.LogLevel - cfg.ScrapeConfig[i].PushConfig.Server.LogFormat = cfg.ServerConfig.LogFormat - } - } + cfg.Setup() var err error if dryRun { - promtail.client, err = client.NewLogger(prometheus.DefaultRegisterer, promtail.logger, cfg.ClientConfig.ExternalLabels, cfg.ClientConfigs...) + promtail.client, err = client.NewLogger(prometheus.DefaultRegisterer, promtail.logger, cfg.ClientConfigs...) if err != nil { return nil, err } cfg.PositionsConfig.ReadOnly = true } else { - promtail.client, err = client.NewMulti(prometheus.DefaultRegisterer, promtail.logger, cfg.ClientConfig.ExternalLabels, cfg.ClientConfigs...) + promtail.client, err = client.NewMulti(prometheus.DefaultRegisterer, promtail.logger, cfg.ClientConfigs...) if err != nil { return nil, err }