influxdata / telegraf

Agent for collecting, processing, aggregating, and writing metrics, logs, and other arbitrary data.
https://influxdata.com/telegraf
MIT License
14.63k stars 5.58k forks source link

Support canceling plugins using context.Context #6613

Closed goller closed 1 year ago

goller commented 4 years ago

Feature Request

I'd like to see support for canceling plugins including non-service plugins using context.Context.
My motivating example came from the SNMP plugin where it can take a long time for each SNMP field to be timed-out.

Proposal:

Follow the pattern in the go HTTP library:

diff --git a/accumulator.go b/accumulator.go
index 1ea5737a..0394c398 100644
--- a/accumulator.go
+++ b/accumulator.go
@@ -1,11 +1,16 @@
 package telegraf

 import (
+   "context"
    "time"
 )

 // Accumulator allows adding metrics to the processing flow.
 type Accumulator interface {
+   // Context returns the existing context on the Accumlator or Background.
+   Context() context.Context
+   WithContext(ctx context.Context) Accumulator
+
    // AddFields adds a metric to the accumulator with the given measurement
    // name, fields, and tags (and timestamp). If a timestamp is not provided,
    // then the accumulator sets it to "now".

Telegraf already sets a cancel on various signals here: https://github.com/influxdata/telegraf/blob/16784bca556be330a00bbda4881238701d1f5e59/cmd/telegraf/telegraf.go#L91

If one were to SIGINT, SIGTERM',SIGHUPthen the plugins could be canceled if they use the context of theAccumulator`.

In a brief survey it was common for the Accumulator to be passed into the specific implementation of the plugin.

danielnelson commented 4 years ago

I don't think we should add the Context to the Accumulator as it is only available in inputs and aggregators. Instead the Context should be passed in to the Start/Gather functions. A transition could be made by creating a new plugin interface and supporting both variations.

danielnelson commented 4 years ago

Obviously what I'm proposing is more work, in the meantime we have been creating a background/todo context in the Start function and cancelling it in Stop.

powersj commented 1 year ago

Working going on around this in #13913, so closing this one as #13913 has more info.