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

fix(agent): Respect processor order in file #13614

Merged
merged 3 commits into from
Jul 14, 2023

Conversation

srebhan
Copy link
Contributor

@srebhan srebhan commented Jul 13, 2023

resolves #12849

Processor and aggregators are sensitive to order changes in Telegraf as their order defines the actual processing done and the metrics that are taken into account. When loading the configuration files, the order of those plugins is preserved taking the order option into account. However, the agent itself sorts the processors again before actually setting up the processing pipeline. While this on itself does not cause an issue, there is a subtle twist as the "order" is reversed using the sorting. This is necessary because the agent constructs the pipeline from output to input and thus needs to start with the "last" processor.
However, the sorting only affects the processors with an explicit order setting, leaving all other processors in the initial order. As a consequence, those processors will then process in reverse order wrt the appearance in the config file...

This PR removes the superfluous sorting in the agent and takes the config-order as-is. It then adds the processors from output-to-input, i.e. it truly iterates the processors in reverse order, thus keeping the processing order equivalent to the appearance in the file while respecting explicit order statements.

@telegraf-tiger telegraf-tiger bot added the fix pr to fix corresponding bug label Jul 13, 2023
@srebhan srebhan added area/agent ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. labels Jul 14, 2023
Copy link
Contributor

@powersj powersj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for putting up the PR!

@powersj powersj merged commit a72b859 into influxdata:master Jul 14, 2023
24 checks passed
@github-actions github-actions bot added this to the v1.27.3 milestone Jul 14, 2023
powersj pushed a commit that referenced this pull request Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/agent fix pr to fix corresponding bug ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New metrics created by starlark do not pass to downstream processors
2 participants