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

processors should run in the order they appear in the config #8016

Closed
Tracked by #9478
ssoroka opened this issue Aug 20, 2020 · 10 comments · Fixed by #12113
Closed
Tracked by #9478

processors should run in the order they appear in the config #8016

ssoroka opened this issue Aug 20, 2020 · 10 comments · Fixed by #12113
Assignees
Labels
area/agent bug unexpected problem or unintended behavior size/m 2-4 day effort

Comments

@ssoroka
Copy link
Contributor

ssoroka commented Aug 20, 2020

Without specifying order, processors are loaded and run in a random order. This makes no sense, as the order they're specified in the .toml file is correct most of the time.

Relevant telegraf.conf:

eg this .conf needs order to work properly

[agent]
  interval = "1s"
  flush_interval = "1s"
  omit_hostname = true

[[inputs.file]]
  name_override = "filesystems_raw"
  files = ["./json.txt"]
  data_format = "value"
  data_type = "string"

[[processors.parser]]
  metric_name = "filesystems"
  order = 1
  namepass = "filesystems_raw"
  drop_original = false
  parse_fields = ["value"]
  data_format = "json"
  json_strict = true
  json_query = "filesystems./"
  tag_keys = [
    "mount",
    "device"
  ]

[[processors.parser]]
  metric_name = "filesystems"
  order = 2
  namepass = "filesystems_raw"
  drop_original = false
  parse_fields = ["value"]
  data_format = "json"
  json_strict = true
  json_query = "filesystems./usr"
  tag_keys = [
    "mount",
    "device"
  ]

[[processors.strings]]
  order = 3
  namepass = "filesystems_raw"
  fielddrop = ["value"] # deletes the metric

[[outputs.file]]
  namedrop = "filesystems_raw"
  files = ["stdout"]

Expected behavior:

  • order processors appear in .conf file used as default
  • order = n still exists and not deprecated and can override default config order

Actual behavior:

  • order is random unless specified
@ssoroka ssoroka added bug unexpected problem or unintended behavior area/agent labels Aug 20, 2020
@mertbakir
Copy link

I think this is very important. I couldn't figure out how to use processor enum | converter. I have a numeric field that I want to store string values as a tag according to the numeric value in the field.

For example I want to convert. field: 1 into tag: "one" field: 2 > tag: "two"

@sjwang90 sjwang90 added this to the 1.16.0 milestone Sep 14, 2020
@ssoroka
Copy link
Contributor Author

ssoroka commented Oct 13, 2020

@mertbakir you can add order = n to each processor to force them to run in a certain order. This PR is for the convenience of not having to set that manually.

@sjwang90 sjwang90 modified the milestones: 1.16.0, 1.15.4, Planned Oct 19, 2020
@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

@sjwang90
Copy link
Contributor

sjwang90 commented Aug 13, 2021

@ssoroka Would we deprecate the order = n when this gets implemented in a Telegraf 2.0?

@MartinLesterSynamedia
Copy link

MartinLesterSynamedia commented Aug 19, 2021

I agree file order should be the default as random processor order is highly unexpected, generates hard to find intermittent bugs, creates data in the DB that can be hard to correct, and is not highlighted clearly in the docs. It really needs some big red flashing warnings!

I even raised this SO and got no response which to me implies it is not widely known about. I eventually stumbled on the order option in the docs by accident which is the first time I saw the phrase "processor execution order will be random".

I don't think this can wait till 2.0. I think this is a bug generator that needs fixing asap. order feels like a sticking plaster solution that comes from maps not being ordered. As @sjwang90 suggests if ordering is implicit from the file, then when would you use the order option? AFAIK There are no loops or conditionals when processing the config files so I am struggling to even conceive of a use case where file order is not the order you would want your processors to run in.

@1tft
Copy link

1tft commented Sep 7, 2021

Regarding

This makes no sense, as the order they're specified in the .toml file is correct most of the time.

I have to remark, telegraf config is often spread over many files (because of own configgenerator etc.), so execution order is not (easy) comprehensible and so you have always think about correct processing order (using order = x or tagpass).

@afletch
Copy link

afletch commented Apr 19, 2022

Regarding

This makes no sense, as the order they're specified in the .toml file is correct most of the time.

I have to remark, telegraf config is often spread over many files (because of own configgenerator etc.), so execution order is not (easy) comprehensible and so you have always think about correct processing order (using order = x or tagpass).

This is absolutely the case in any logically-designed configuration layout. If processors are to be run in the order they appear in files then it would also be necessary to load files in some sane order, for example to at least have the chance to use numerical prefixes on files to determine ordering.

@philomory
Copy link
Contributor

Yeah, even though I think this would be a helpful convenience item for simple configurations, this is absolutely not a replacement for the order parameter as a whole, and the order parameter should definitely not be deprecated.

@zenonian
Copy link

zenonian commented Aug 2, 2022

A related issue is that a telegraf config command should default to writing the configuration stanza categories in this order:
global_tags:agent:inputs:processors:aggregators:outputs
Right now, we get this order instead:
global_tags:agent:outputs:processors:aggregators:inputs
which is of course quite confusing. Possibly, one might have the --section-filter option override the default order of stanza categories.

Within each stanza category, of course, the order of generation should be controlled by the plugin order in the associated command-line option, such as --processor-filter.

@sspaink sspaink self-assigned this Sep 28, 2022
@sspaink sspaink added the size/m 2-4 day effort label Sep 28, 2022
@matthijskooijman
Copy link
Contributor

I don't think this can wait till 2.0. I think this is a bug generator that needs fixing asap. order feels like a sticking plaster solution that comes from maps not being ordered. As @sjwang90 suggests if ordering is implicit from the file, then when would you use the order option? AFAIK There are no loops or conditionals when processing the config files so I am struggling to even conceive of a use case where file order is not the order you would want your processors to run in.

Agreed, I also ran into this issue today. I saw the order config, but it seemed sensible that that was only to customize order in some cases, with the default being file order, which is apparently not the case.

Fixing this to default to file order (i.e. when order is equal, order by file order) would be a sensible default. Since the current ordering is, I believe, random, implementing this does not pose a backward compatibility problem, since order-by-file is just one of the random orders that you could get right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/agent bug unexpected problem or unintended behavior size/m 2-4 day effort
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants