Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aggregators should not re-run processors #7993

Closed
Tracked by #9478
ssoroka opened this issue Aug 16, 2020 · 6 comments · Fixed by #14882
Closed
Tracked by #9478

aggregators should not re-run processors #7993

ssoroka opened this issue Aug 16, 2020 · 6 comments · Fixed by #14882
Assignees
Labels
area/agent area/execd Issues related to execd or plugins that would be better suited to be used through execd area/starlark bug unexpected problem or unintended behavior size/l 1 week or more effort

Comments

@ssoroka
Copy link
Contributor

ssoroka commented Aug 16, 2020

Relevant telegraf.conf:

[[inputs.file]]
  files = ["file.txt"] # contains `data 1`
  data_format = "influx"

[[processors.starlark]]
  # Multiply any float fields by 10
  source = '''
def apply(metric):
    for k, v in metric.fields.items():
        if type(v) == "float":
            metric.fields[k] = v * 10
    return metric
'''

# Keep the aggregate min/max of each metric passing through.
[[aggregators.minmax]]
  period = "30s"
  drop_original = false

Steps to reproduce:

run config like above. Run aggregator with any non-idempotent processor

Expected behavior:

processor runs once, outputting data 10

Actual behavior:

processor runs twice, outputting data 100

Additional info:

This is an old feature of aggregators, and it's due to the fact that aggregators re-run all processors. This was probably figured to be a good idea since you may want to modify the output of aggregators, but it's 1. wasteful, and 2. breaks any processors that can't be applied twice to the same field, eg x * 10 where x=1 gives you 10 after one run, but 100 after two runs; it's not idempotent. Adding an aggregator will break any of these processors.

The solution here would be to convert aggregators to be processors themselves. This will let you order them across both processors and aggregators, and resolve the problem in a very sensible way. Aggregators are a special case of processors, and with the new streaming processor support, they fit perfectly into the processor model.

@ssoroka ssoroka added bug unexpected problem or unintended behavior area/agent labels Aug 16, 2020
@ssoroka ssoroka self-assigned this Aug 19, 2020
@sjwang90 sjwang90 added this to the 1.16.0 milestone Sep 14, 2020
@sjwang90 sjwang90 modified the milestones: 1.16.0, 1.15.4, Planned Oct 19, 2020
@ssoroka ssoroka added area/execd Issues related to execd or plugins that would be better suited to be used through execd area/starlark labels Nov 3, 2020
@iot-monitor-net
Copy link

Took me some time to figure out what was wrong, as I ran into this problem as well. Would be great to have this resolved.

@sjwang90 sjwang90 removed this from the Planned milestone Jan 29, 2021
@ssoroka
Copy link
Contributor Author

ssoroka commented Mar 1, 2021

This will be part of Telegraf 2.0

@jimmyseto
Copy link
Contributor

@ssoroka - is there a workaround for this in 1.17.x by any chance? we can use metric filtering to bypass processors the second time around. but, if we are using execd, for example, and we don't want a second instance of that program to spin up, is there way to prevent that from happening?

@sjwang90 sjwang90 added this to the 2.0.0 milestone Apr 14, 2021
@ssoroka
Copy link
Contributor Author

ssoroka commented Jul 2, 2021

If you're using aggregators you can't prevent a second copy from spinning up in 1.x

@sjwang90
Copy link
Contributor

Behavior when fix is implemented

 +------------+                     Processors and aggregators can be ordered                     +--------+
 | Input      +---+                        and chained arbitrarily                           +--->+ Output |
 +------------+   |                                                                          |    +--------+
                  |                                                                          |
 +------------+   |   +-----------+    +------------+      +-----------+    +------------+   |    +--------+
 | Input      +------>+ Processor +--->+ Aggregator +----->+ Processor +--->+ Aggregator +-->---->+ Output |
 +------------+   |   +-----------+    +------------+      +-----------+    +------------+   |    +--------+
                  |                                                                          |
 +------------+   |                                                                          |    +--------+
 | Input      +---+                                                                          +--->+ Output |
 +------------+   |                                                                          |    +--------+
                  |                                                                          |
 +------------+   |                                                                          |    +--------+
 | Input      +---+                                                                          +--->+ Output |
 +------------+                                                                                   +--------+

@redbaron
Copy link
Contributor

redbaron commented May 6, 2023

@ssoroka - is there a workaround for this in 1.17.x by any chance? we can use metric filtering to bypass processors the second time around. but, if we are using execd, for example, and we don't want a second instance of that program to spin up, is there way to prevent that from happening?

I haven't tried myself, but adding marker tag in the aggregator, then tagdrop in processor and tagexclude in output should prevent loops

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/agent area/execd Issues related to execd or plugins that would be better suited to be used through execd area/starlark bug unexpected problem or unintended behavior size/l 1 week or more effort
Projects
None yet
7 participants