promtail: Fix issue with dropping logs when a file based SD target's labels are updated

CHANGELOG.md

@@ -36,6 +36,7 @@
 
 ##### Fixes
 * [6766](https://github.com/grafana/loki/pull/6766) **kavirajk**: fix(logql): Make `LabelSampleExtractor` ignore processing the line if it doesn't contain that specific label. Fixes unwrap behavior explained in the issue https://github.com/grafana/loki/issues/6713
+* [7016](https://github.com/grafana/loki/pull/7016) **chodges15**: Fix issue with dropping logs when a target's labels are updated
 
 ##### Changes
 

clients/pkg/promtail/targets/file/filetargetmanager.go

@@ -8,8 +8,6 @@ import (
 	"sync"
 
 	"github.com/bmatcuk/doublestar"
-	"gopkg.in/fsnotify.v1"
-
 	"github.com/go-kit/log"
 	"github.com/go-kit/log/level"
 	"github.com/prometheus/client_golang/prometheus"
@@ -19,6 +17,7 @@ import (
 	"github.com/prometheus/prometheus/discovery/targetgroup"
 	"github.com/prometheus/prometheus/model/labels"
 	"github.com/prometheus/prometheus/model/relabel"
+	"gopkg.in/fsnotify.v1"
 
 	"github.com/grafana/loki/clients/pkg/logentry/stages"
 	"github.com/grafana/loki/clients/pkg/promtail/api"
@@ -273,6 +272,25 @@ func (s *targetSyncer) sync(groups []*targetgroup.Group, targetEventHandler chan
 	targets := map[string]struct{}{}
 	dropped := []target.Target{}
 
+	// process target removal first to avoid cleaning up fileEventWatchers that are needed by a new or modified target
+	for key, target := range s.targets {
+		if _, ok := targets[key]; !ok {
+			level.Info(s.log).Log("msg", "Removing target", "key", key)
+			target.Stop()
+			s.metrics.targetsActive.Add(-1.)
+			delete(s.targets, key)
+
+			// close related file event watcher
+			k := target.path
+			if _, ok := s.fileEventWatchers[k]; ok {
+				close(s.fileEventWatchers[k])
+				delete(s.fileEventWatchers, k)
+			} else {
+				level.Warn(s.log).Log("msg", "failed to remove file event watcher", "path", k)
+			}
+		}
+	}
+
 	for _, group := range groups {
 		for _, t := range group.Targets {
 			level.Debug(s.log).Log("msg", "new target", "labels", t)
@@ -356,23 +374,6 @@ func (s *targetSyncer) sync(groups []*targetgroup.Group, targetEventHandler chan
 		}
 	}
 
-	for key, target := range s.targets {
-		if _, ok := targets[key]; !ok {
-			level.Info(s.log).Log("msg", "Removing target", "key", key)
-			target.Stop()
-			s.metrics.targetsActive.Add(-1.)
-			delete(s.targets, key)
-
-			// close related file event watcher
-			k := target.path
-			if _, ok := s.fileEventWatchers[k]; ok {
-				close(s.fileEventWatchers[k])
-				delete(s.fileEventWatchers, k)
-			} else {
-				level.Warn(s.log).Log("msg", "failed to remove file event watcher", "path", k)
-			}
-		}
-	}
 	s.droppedTargets = dropped
 }
 

clients/pkg/promtail/targets/file/filetargetmanager_test.go

@@ -527,3 +527,53 @@ func TestDeadlockStartWatchingDuringSync(t *testing.T) {
 	ftm.Stop()
 	ps.Stop()
 }
+
+func TestLabelSetUpdate(t *testing.T) {
+	client := fake.New(func() {})
+	defer client.Stop()
+
+	targetEventHandler := make(chan fileTargetEvent)
+	defer func() {
+		close(targetEventHandler)
+	}()
+
+	syncer := &targetSyncer{
+		metrics:           NewMetrics(nil),
+		log:               log.NewLogfmtLogger(log.NewSyncWriter(os.Stderr)),
+		positions:         nil,
+		entryHandler:      client,
+		hostname:          "localhost",
+		fileEventWatchers: make(map[string]chan fsnotify.Event),
+		targets:           make(map[string]*FileTarget),
+		targetConfig: &Config{
+			SyncPeriod: time.Hour,
+		},
+	}
+
+	var target = model.LabelSet{
+		hostLabel: "localhost",
+		pathLabel: "baz",
+		"job":     "foo",
+	}
+
+	syncer.sync([]*targetgroup.Group{
+		{
+			Targets: []model.LabelSet{target},
+		},
+	}, targetEventHandler)
+	syncer.sendFileCreateEvent(fsnotify.Event{Name: "baz"})
+
+	require.Equal(t, 1, len(syncer.targets))
+	require.Equal(t, 1, len(syncer.fileEventWatchers))
+
+	target["job"] = "bar"
+	syncer.sync([]*targetgroup.Group{
+		{
+			Targets: []model.LabelSet{target},
+		},
+	}, targetEventHandler)
+
+	require.Equal(t, 1, len(syncer.targets))
+	require.Equal(t, 1, len(syncer.fileEventWatchers))
+
+}