fix(agent): Restore setup order of stateful plugins to Init() then Se…

…tState() (#16123)

agent/agent.go

@@ -106,6 +106,11 @@ func (a *Agent) Run(ctx context.Context) error {
 		time.Duration(a.Config.Agent.Interval), a.Config.Agent.Quiet,
 		a.Config.Agent.Hostname, time.Duration(a.Config.Agent.FlushInterval))
 
+	log.Printf("D! [agent] Initializing plugins")
+	if err := a.InitPlugins(); err != nil {
+		return err
+	}
+
 	if a.Config.Persister != nil {
 		log.Printf("D! [agent] Initializing plugin states")
 		if err := a.initPersister(); err != nil {
@@ -119,11 +124,6 @@ func (a *Agent) Run(ctx context.Context) error {
 		}
 	}
 
-	log.Printf("D! [agent] Initializing plugins")
-	if err := a.InitPlugins(); err != nil {
-		return err
-	}
-
 	startTime := time.Now()
 
 	log.Printf("D! [agent] Connecting outputs")

docs/specs/tsd-003-state-persistence.md

@@ -31,15 +31,17 @@ It is intended to
 
 The persistence will use the following steps:
 
+- Compute an unique ID for each of the plugin _instances_
+- Startup Telegraf plugins calling `Init()`, etc.
 - Initialize persistence framework with the user specified `statefile` location
   and load the state if present
 - Determine all stateful plugin instances by fulfilling the `StatefulPlugin`
   interface
-- Compute an unique ID for each of the plugin _instances_
 - Restore plugin states (if any) for each plugin ID present in the state-file
-- Startup Telegraf plugins calling `Init()`, etc.
 - Run data-collection etc...
-- On shutdown, query the state of all registered stateful plugins state
+- On shutdown, stopping all Telegraf plugins calling `Stop()` or `Close()`
+  depending on the plugin type
+- Query the state of all registered stateful plugins state
 - Create an overall state-map with the plugin instance ID as a key and the
   serialized plugin state as value.
 - Marshal the overall state-map and store to disk
@@ -85,7 +87,7 @@ for the overall state. On-disk, the overall state of Telegraf is stored as JSON.
 To restore the state of a plugin, the overall Telegraf state is first
 deserialized from the on-disk JSON data and a lookup for the plugin ID is
 performed in the resulting map. The value, if found, is then deserialized to the
-plugin's state data-structure and provided to the plugin before calling `Init()`.
+plugin's state data-structure and provided to the plugin after calling `Init()`.
 
 ## Is / Is-not
 

plugins/common/starlark/starlark.go

@@ -25,6 +25,7 @@ type Common struct {
 	StarlarkLoadFunc func(module string, logger telegraf.Logger) (starlark.StringDict, error)
 
 	thread     *starlark.Thread
+	builtins   starlark.StringDict
 	globals    starlark.StringDict
 	functions  map[string]*starlark.Function
 	parameters map[string]starlark.Tuple
@@ -97,8 +98,9 @@ func (s *Common) SetState(state interface{}) error {
 			return fmt.Errorf("state item %q cannot be set: %w", k, err)
 		}
 	}
+	s.builtins["state"] = s.state
 
-	return nil
+	return s.InitProgram()
 }
 
 func (s *Common) Init() error {
@@ -109,44 +111,48 @@ func (s *Common) Init() error {
 		return errors.New("both source or script cannot be set")
 	}
 
-	s.thread = &starlark.Thread{
-		Print: func(_ *starlark.Thread, msg string) { s.Log.Debug(msg) },
-		Load: func(_ *starlark.Thread, module string) (starlark.StringDict, error) {
-			return s.StarlarkLoadFunc(module, s.Log)
-		},
-	}
+	s.builtins = starlark.StringDict{}
+	s.builtins["Metric"] = starlark.NewBuiltin("Metric", newMetric)
+	s.builtins["deepcopy"] = starlark.NewBuiltin("deepcopy", deepcopy)
+	s.builtins["catch"] = starlark.NewBuiltin("catch", catch)
 
-	builtins := starlark.StringDict{}
-	builtins["Metric"] = starlark.NewBuiltin("Metric", newMetric)
-	builtins["deepcopy"] = starlark.NewBuiltin("deepcopy", deepcopy)
-	builtins["catch"] = starlark.NewBuiltin("catch", catch)
-
-	if err := s.addConstants(&builtins); err != nil {
+	if err := s.addConstants(&s.builtins); err != nil {
 		return err
 	}
 
-	// Insert the persisted state if any
-	if s.state != nil {
-		builtins["state"] = s.state
-	}
-
-	// Load the program. In case of an error we can try to insert the state
-	// which can be used implicitly e.g. when persisting states
-	program, err := s.sourceProgram(builtins)
-	if err != nil {
+	// Initialize the program
+	if err := s.InitProgram(); err != nil {
 		// Try again with a declared state. This might be necessary for
 		// state persistence.
 		s.state = starlark.NewDict(0)
-		builtins["state"] = s.state
-		p, serr := s.sourceProgram(builtins)
-		if serr != nil {
+		s.builtins["state"] = s.state
+		if serr := s.InitProgram(); serr != nil {
 			return err
 		}
-		program = p
+	}
+
+	s.functions = make(map[string]*starlark.Function)
+	s.parameters = make(map[string]starlark.Tuple)
+
+	return nil
+}
+
+func (s *Common) InitProgram() error {
+	// Load the program. In case of an error we can try to insert the state
+	// which can be used implicitly e.g. when persisting states
+	program, err := s.sourceProgram(s.builtins)
+	if err != nil {
+		return err
 	}
 
 	// Execute source
-	globals, err := program.Init(s.thread, builtins)
+	s.thread = &starlark.Thread{
+		Print: func(_ *starlark.Thread, msg string) { s.Log.Debug(msg) },
+		Load: func(_ *starlark.Thread, module string) (starlark.StringDict, error) {
+			return s.StarlarkLoadFunc(module, s.Log)
+		},
+	}
+	globals, err := program.Init(s.thread, s.builtins)
 	if err != nil {
 		return err
 	}
@@ -162,10 +168,8 @@ func (s *Common) Init() error {
 	// metrics. Tasks that require global state will not be possible due to
 	// this, so maybe we should relax this in the future.
 	globals.Freeze()
-
 	s.globals = globals
-	s.functions = make(map[string]*starlark.Function)
-	s.parameters = make(map[string]starlark.Tuple)
+
 	return nil
 }
 

plugins/inputs/tail/tail.go

@@ -30,10 +30,6 @@ var sampleConfig string
 
 var once sync.Once
 
-const (
-	defaultWatchMethod = "inotify"
-)
-
 var (
 	offsets      = make(map[string]int64)
 	offsetsMutex = new(sync.Mutex)