Update log conversion for legacy file path (#6812)

* Update log conversion. * linter feedback * fix test
grafana · Apr 2, 2024 · d8d8872 · d8d8872
1 parent d1afb48
commit d8d8872
Show file tree

Hide file tree

Showing 32 changed files with 144 additions and 66 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -36,6 +36,8 @@ Main (unreleased)
 
 - Added support for `otelcol` configuration conversion in `grafana-agent convert` and `grafana-agent run` commands. (@rfratto, @erikbaranowski, @tpaschalis, @hainenber)
 
+- Add automatic conversion for `legacy_positions_file` in component `loki.source.file`. (@mattdurham)
+
 ### Features
 
 - Added a new CLI flag `--stability.level` which defines the minimum stability

diff --git a/internal/component/common/loki/positions/positions.go b/internal/component/common/loki/positions/positions.go
@@ -104,7 +104,6 @@ type LegacyFile struct {
 // ConvertLegacyPositionsFile will convert the legacy positions file to the new format if:
 // 1. There is no file at the newpath
 // 2. There is a file at the legacy path and that it is valid yaml
-// If all the above is true then the legacy file will be deleted.
 func ConvertLegacyPositionsFile(legacyPath, newPath string, l log.Logger) {
 	legacyPositions := readLegacyFile(legacyPath, l)
 	// LegacyPositions did not exist or was invalid so return.
@@ -131,9 +130,6 @@ func ConvertLegacyPositionsFile(legacyPath, newPath string, l log.Logger) {
 	if err != nil {
 		level.Error(l).Log("msg", "error writing new positions file from legacy", "path", newPath, "error", err)
 	}
-
-	// Finally remove the old path.
-	_ = os.Remove(legacyPath)
 }
 
 func readLegacyFile(legacyPath string, l log.Logger) *LegacyFile {

diff --git a/internal/component/common/loki/positions/positions_test.go b/internal/component/common/loki/positions/positions_test.go
@@ -67,9 +67,6 @@ func TestLegacyConversion(t *testing.T) {
 		require.True(t, k.Path == "/tmp/random.log")
 		require.True(t, v == "17623")
 	}
-	// Ensure old file is deleted.
-	_, err = os.Stat(legacy)
-	require.True(t, os.IsNotExist(err))
 }
 
 func TestLegacyConversionWithNewFile(t *testing.T) {

diff --git a/internal/converter/internal/promtailconvert/internal/build/scrape_builder.go b/internal/converter/internal/promtailconvert/internal/build/scrape_builder.go
@@ -11,11 +11,13 @@ import (
 	filematch "github.com/grafana/agent/internal/component/local/file_match"
 	"github.com/grafana/agent/internal/component/loki/process"
 	"github.com/grafana/agent/internal/component/loki/process/stages"
+
 	lokirelabel "github.com/grafana/agent/internal/component/loki/relabel"
 	lokisourcefile "github.com/grafana/agent/internal/component/loki/source/file"
 	"github.com/grafana/agent/internal/converter/diag"
 	"github.com/grafana/agent/internal/converter/internal/common"
 	"github.com/grafana/agent/internal/converter/internal/prometheusconvert/component"
+	"github.com/grafana/loki/clients/pkg/promtail/positions"
 	"github.com/grafana/loki/clients/pkg/promtail/scrapeconfig"
 	"github.com/grafana/loki/clients/pkg/promtail/targets/file"
 	"github.com/grafana/river/scanner"
@@ -42,7 +44,6 @@ func NewScrapeConfigBuilder(
 	diags *diag.Diagnostics,
 	cfg *scrapeconfig.Config,
 	globalCtx *GlobalContext,
-
 ) *ScrapeConfigBuilder {
 
 	return &ScrapeConfigBuilder{
@@ -61,7 +62,7 @@ func (s *ScrapeConfigBuilder) Sanitize() {
 	}
 }
 
-func (s *ScrapeConfigBuilder) AppendLokiSourceFile(watchConfig *file.WatchConfig) {
+func (s *ScrapeConfigBuilder) AppendLokiSourceFile(watchConfig *file.WatchConfig, positionsCfg *positions.Config) {
 	// If there were no targets expressions collected, that means
 	// we didn't have any components that produced SD targets, so
 	// we can skip this component.
@@ -76,6 +77,7 @@ func (s *ScrapeConfigBuilder) AppendLokiSourceFile(watchConfig *file.WatchConfig
 		Encoding:            s.cfg.Encoding,
 		DecompressionConfig: convertDecompressionConfig(s.cfg.DecompressionCfg),
 		FileWatch:           convertFileWatchConfig(watchConfig),
+		LegacyPositionsFile: positionsCfg.PositionsFile,
 	}
 	overrideHook := func(val interface{}) interface{} {
 		if _, ok := val.([]discovery.Target); ok {

diff --git a/internal/converter/internal/promtailconvert/promtailconvert.go b/internal/converter/internal/promtailconvert/promtailconvert.go
@@ -93,8 +93,8 @@ func Convert(in []byte, extraArgs []string) ([]byte, diag.Diagnostics) {
 func AppendAll(f *builder.File, cfg *promtailcfg.Config, labelPrefix string, diags diag.Diagnostics) diag.Diagnostics {
 	validateTopLevelConfig(cfg, &diags)
 
-	var writeReceivers = make([]loki.LogsReceiver, len(cfg.ClientConfigs))
-	var writeBlocks = make([]*builder.Block, len(cfg.ClientConfigs))
+	writeReceivers := make([]loki.LogsReceiver, len(cfg.ClientConfigs))
+	writeBlocks := make([]*builder.Block, len(cfg.ClientConfigs))
 	// Each client config needs to be a separate remote_write,
 	// because they may have different ExternalLabels fields.
 	for i, cc := range cfg.ClientConfigs {
@@ -108,7 +108,7 @@ func AppendAll(f *builder.File, cfg *promtailcfg.Config, labelPrefix string, dia
 	}
 
 	for _, sc := range cfg.ScrapeConfig {
-		appendScrapeConfig(f, &sc, &diags, gc, &cfg.Global.FileWatch)
+		appendScrapeConfig(f, &sc, &diags, gc, &cfg.Global.FileWatch, &cfg.PositionsConfig)
 	}
 
 	for _, write := range writeBlocks {
@@ -137,6 +137,7 @@ func appendScrapeConfig(
 	diags *diag.Diagnostics,
 	gctx *build.GlobalContext,
 	watchConfig *file.WatchConfig,
+	positionsCfg *positions.Config,
 ) {
 
 	b := build.NewScrapeConfigBuilder(f, diags, cfg, gctx)
@@ -151,7 +152,7 @@ func appendScrapeConfig(
 	// If any relabelling is required, it will be done via a discovery.relabel component.
 	// The files will be watched and the globs in file paths will be expanded using discovery.file component.
 	// The log entries are sent to loki.process if processing is needed, or directly to loki.write components.
-	b.AppendLokiSourceFile(watchConfig)
+	b.AppendLokiSourceFile(watchConfig, positionsCfg)
 
 	// Append all the components that produce logs directly.
 	// If any relabelling is required, it will be done via a loki.relabel component.

diff --git a/internal/converter/internal/promtailconvert/testdata/azure.river b/internal/converter/internal/promtailconvert/testdata/azure.river
@@ -17,6 +17,7 @@ local.file_match "fun" {
 }
 
 loki.source.file "fun" {
-	targets    = local.file_match.fun.targets
-	forward_to = []
+	targets               = local.file_match.fun.targets
+	forward_to            = []
+	legacy_positions_file = "/var/log/positions.yaml"
 }
diff --git a/internal/converter/internal/promtailconvert/testdata/consul.river b/internal/converter/internal/promtailconvert/testdata/consul.river
@@ -33,6 +33,7 @@ local.file_match "fun" {
 }
 
 loki.source.file "fun" {
-	targets    = local.file_match.fun.targets
-	forward_to = []
+	targets               = local.file_match.fun.targets
+	forward_to            = []
+	legacy_positions_file = "/var/log/positions.yaml"
 }
diff --git a/internal/converter/internal/promtailconvert/testdata/consulagent.river b/internal/converter/internal/promtailconvert/testdata/consulagent.river
@@ -32,6 +32,7 @@ local.file_match "fun" {
 }
 
 loki.source.file "fun" {
-	targets    = local.file_match.fun.targets
-	forward_to = []
+	targets               = local.file_match.fun.targets
+	forward_to            = []
+	legacy_positions_file = "/var/log/positions.yaml"
 }
diff --git a/internal/converter/internal/promtailconvert/testdata/digitalocean.river b/internal/converter/internal/promtailconvert/testdata/digitalocean.river
@@ -8,6 +8,7 @@ local.file_match "fun" {
 }
 
 loki.source.file "fun" {
-	targets    = local.file_match.fun.targets
-	forward_to = []
+	targets               = local.file_match.fun.targets
+	forward_to            = []
+	legacy_positions_file = "/var/log/positions.yaml"
 }
diff --git a/internal/converter/internal/promtailconvert/testdata/dockerswarm.river b/internal/converter/internal/promtailconvert/testdata/dockerswarm.river
@@ -33,6 +33,7 @@ local.file_match "fun" {
 }
 
 loki.source.file "fun" {
-	targets    = local.file_match.fun.targets
-	forward_to = []
+	targets               = local.file_match.fun.targets
+	forward_to            = []
+	legacy_positions_file = "/var/log/positions.yaml"
 }
diff --git a/internal/converter/internal/promtailconvert/testdata/ec2.river b/internal/converter/internal/promtailconvert/testdata/ec2.river
@@ -10,6 +10,7 @@ local.file_match "fun" {
 }
 
 loki.source.file "fun" {
-	targets    = local.file_match.fun.targets
-	forward_to = []
+	targets               = local.file_match.fun.targets
+	forward_to            = []
+	legacy_positions_file = "/var/log/positions.yaml"
 }
diff --git a/internal/converter/internal/promtailconvert/testdata/file.river b/internal/converter/internal/promtailconvert/testdata/file.river
@@ -15,6 +15,7 @@ local.file_match "fun" {
 }
 
 loki.source.file "fun" {
-	targets    = local.file_match.fun.targets
-	forward_to = []
+	targets               = local.file_match.fun.targets
+	forward_to            = []
+	legacy_positions_file = "/var/log/positions.yaml"
 }
diff --git a/internal/converter/internal/promtailconvert/testdata/gce.river b/internal/converter/internal/promtailconvert/testdata/gce.river
@@ -9,6 +9,7 @@ local.file_match "fun" {
 }
 
 loki.source.file "fun" {
-	targets    = local.file_match.fun.targets
-	forward_to = []
+	targets               = local.file_match.fun.targets
+	forward_to            = []
+	legacy_positions_file = "/var/log/positions.yaml"
 }
diff --git a/internal/converter/internal/promtailconvert/testdata/globalpositions.river b/internal/converter/internal/promtailconvert/testdata/globalpositions.river
@@ -0,0 +1,43 @@
+discovery.file "fun" {
+	files = ["/etc/prometheus/targets/*.json"]
+}
+
+discovery.file "fun_2" {
+	files            = ["/etc/agent/targets/*.json"]
+	refresh_interval = "30m0s"
+}
+
+local.file_match "fun" {
+	path_targets = concat(
+		discovery.file.fun.targets,
+		discovery.file.fun_2.targets,
+	)
+}
+
+loki.source.file "fun" {
+	targets               = local.file_match.fun.targets
+	forward_to            = []
+	legacy_positions_file = "/good/positions.yml"
+}
+
+discovery.file "fun2" {
+	files = ["/etc/prometheus/targets2/*.json"]
+}
+
+discovery.file "fun2_2" {
+	files            = ["/etc/agent/targets2/*.json"]
+	refresh_interval = "30m0s"
+}
+
+local.file_match "fun2" {
+	path_targets = concat(
+		discovery.file.fun2.targets,
+		discovery.file.fun2_2.targets,
+	)
+}
+
+loki.source.file "fun2" {
+	targets               = local.file_match.fun2.targets
+	forward_to            = []
+	legacy_positions_file = "/good/positions.yml"
+}
diff --git a/internal/converter/internal/promtailconvert/testdata/globalpositions.yaml b/internal/converter/internal/promtailconvert/testdata/globalpositions.yaml
@@ -0,0 +1,21 @@
+positions:
+  filename: /good/positions.yml
+scrape_configs:
+  - job_name: fun
+    file_sd_configs:
+      - files:
+          - /etc/prometheus/targets/*.json
+        refresh_interval: 5m
+      - files:
+          - /etc/agent/targets/*.json
+        refresh_interval: 30m
+  - job_name: fun2
+    file_sd_configs:
+      - files:
+          - /etc/prometheus/targets2/*.json
+        refresh_interval: 5m
+      - files:
+          - /etc/agent/targets2/*.json
+        refresh_interval: 30m
+tracing: {enabled: false}
+server: {register_instrumentation: false}
diff --git a/internal/converter/internal/promtailconvert/testdata/kubernetes.river b/internal/converter/internal/promtailconvert/testdata/kubernetes.river
@@ -72,6 +72,7 @@ local.file_match "fun" {
 }
 
 loki.source.file "fun" {
-	targets    = local.file_match.fun.targets
-	forward_to = []
+	targets               = local.file_match.fun.targets
+	forward_to            = []
+	legacy_positions_file = "/var/log/positions.yaml"
 }
diff --git a/internal/converter/internal/promtailconvert/testdata/marathon.river b/internal/converter/internal/promtailconvert/testdata/marathon.river
@@ -21,6 +21,7 @@ local.file_match "fun" {
 }
 
 loki.source.file "fun" {
-	targets    = local.file_match.fun.targets
-	forward_to = []
+	targets               = local.file_match.fun.targets
+	forward_to            = []
+	legacy_positions_file = "/var/log/positions.yaml"
 }
diff --git a/internal/converter/internal/promtailconvert/testdata/mixed_pipeline.river b/internal/converter/internal/promtailconvert/testdata/mixed_pipeline.river
@@ -37,8 +37,9 @@ loki.process "uber_pipeline" {
 }
 
 loki.source.file "uber_pipeline" {
-	targets    = local.file_match.uber_pipeline.targets
-	forward_to = [loki.process.uber_pipeline.receiver]
+	targets               = local.file_match.uber_pipeline.targets
+	forward_to            = [loki.process.uber_pipeline.receiver]
+	legacy_positions_file = "/tmp/positions.yaml"
 }
 
 loki.source.api "uber_pipeline" {

diff --git a/internal/converter/internal/promtailconvert/testdata/nerve.river b/internal/converter/internal/promtailconvert/testdata/nerve.river
@@ -9,6 +9,7 @@ local.file_match "fun" {
 }
 
 loki.source.file "fun" {
-	targets    = local.file_match.fun.targets
-	forward_to = []
+	targets               = local.file_match.fun.targets
+	forward_to            = []
+	legacy_positions_file = "/var/log/positions.yaml"
 }
diff --git a/internal/converter/internal/promtailconvert/testdata/openstack.river b/internal/converter/internal/promtailconvert/testdata/openstack.river
@@ -42,6 +42,7 @@ local.file_match "prometheus1" {
 }
 
 loki.source.file "prometheus1" {
-	targets    = local.file_match.prometheus1.targets
-	forward_to = []
+	targets               = local.file_match.prometheus1.targets
+	forward_to            = []
+	legacy_positions_file = "/var/log/positions.yaml"
 }
diff --git a/internal/converter/internal/promtailconvert/testdata/pipeline_stages_cri_empty.river b/internal/converter/internal/promtailconvert/testdata/pipeline_stages_cri_empty.river
@@ -14,8 +14,9 @@ loki.process "example" {
 }
 
 loki.source.file "example" {
-	targets    = local.file_match.example.targets
-	forward_to = [loki.process.example.receiver]
+	targets               = local.file_match.example.targets
+	forward_to            = [loki.process.example.receiver]
+	legacy_positions_file = "/var/log/positions.yaml"
 }
 
 loki.write "default" {

diff --git a/internal/converter/internal/promtailconvert/testdata/pipeline_stages_drop.river b/internal/converter/internal/promtailconvert/testdata/pipeline_stages_drop.river
@@ -52,8 +52,9 @@ loki.process "example" {
 }
 
 loki.source.file "example" {
-	targets    = local.file_match.example.targets
-	forward_to = [loki.process.example.receiver]
+	targets               = local.file_match.example.targets
+	forward_to            = [loki.process.example.receiver]
+	legacy_positions_file = "/var/log/positions.yaml"
 }
 
 loki.write "default" {

diff --git a/internal/converter/internal/promtailconvert/testdata/pipeline_stages_match_nested.river b/internal/converter/internal/promtailconvert/testdata/pipeline_stages_match_nested.river
@@ -30,8 +30,9 @@ loki.process "example" {
 }
 
 loki.source.file "example" {
-	targets    = local.file_match.example.targets
-	forward_to = [loki.process.example.receiver]
+	targets               = local.file_match.example.targets
+	forward_to            = [loki.process.example.receiver]
+	legacy_positions_file = "/var/log/positions.yaml"
 }
 
 loki.write "default" {

diff --git a/internal/converter/internal/promtailconvert/testdata/pipeline_stages_part1.river b/internal/converter/internal/promtailconvert/testdata/pipeline_stages_part1.river
@@ -90,8 +90,9 @@ loki.process "example" {
 }
 
 loki.source.file "example" {
-	targets    = local.file_match.example.targets
-	forward_to = [loki.process.example.receiver]
+	targets               = local.file_match.example.targets
+	forward_to            = [loki.process.example.receiver]
+	legacy_positions_file = "/var/log/positions.yaml"
 }
 
 loki.write "default" {

diff --git a/internal/converter/internal/promtailconvert/testdata/pipeline_stages_part2.river b/internal/converter/internal/promtailconvert/testdata/pipeline_stages_part2.river
@@ -99,8 +99,9 @@ loki.process "example" {
 }
 
 loki.source.file "example" {
-	targets    = local.file_match.example.targets
-	forward_to = [loki.process.example.receiver]
+	targets               = local.file_match.example.targets
+	forward_to            = [loki.process.example.receiver]
+	legacy_positions_file = "/var/log/positions.yaml"
 }
 
 loki.write "default" {

diff --git a/...nal/converter/internal/promtailconvert/testdata/pipeline_stages_structured_metadata.river b/...nal/converter/internal/promtailconvert/testdata/pipeline_stages_structured_metadata.river
@@ -24,8 +24,9 @@ loki.process "example" {
 }
 
 loki.source.file "example" {
-	targets    = local.file_match.example.targets
-	forward_to = [loki.process.example.receiver]
+	targets               = local.file_match.example.targets
+	forward_to            = [loki.process.example.receiver]
+	legacy_positions_file = "/var/log/positions.yaml"
 }
 
 loki.write "default" {

diff --git a/internal/converter/internal/promtailconvert/testdata/sd_pipeline_example.river b/internal/converter/internal/promtailconvert/testdata/sd_pipeline_example.river
@@ -38,8 +38,9 @@ loki.process "funny_one" {
 }
 
 loki.source.file "funny_one" {
-	targets    = local.file_match.funny_one.targets
-	forward_to = [loki.process.funny_one.receiver]
+	targets               = local.file_match.funny_one.targets
+	forward_to            = [loki.process.funny_one.receiver]
+	legacy_positions_file = "/var/log/positions.yaml"
 }
 
 loki.write "default" {