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

Change the default value for $otelcol::processors::order to 10 #22

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cyberkov
Copy link
Contributor

@cyberkov cyberkov commented May 2, 2024

Pull Request (PR) description

According to the documentation it would make sense to be able to declare processors that are always first.
Therefore I'd suggest to raise the default order value to 10.

One could skip the order parameter in day-to-day operations and would still be able to have them in the right order.

This Pull Request (PR) fixes the following issues

@cyberkov cyberkov added the enhancement New feature or request label May 2, 2024
@smortex
Copy link
Member

smortex commented May 2, 2024

This feels like this can unexpectedly change the behavior for some users? If so, this should rather be labelled as backwards-incompatible.


REFERENCE.md is outdated

Please update REFERENCE.md with

$ bundle exec rake strings:generate:reference

Add it to the PR and CI should continue.

@smortex smortex changed the title set the default order for processors to 10 Change the default value for $otelcol::processors::order to 10 May 2, 2024
Copy link
Member

@smortex smortex left a comment

Choose a reason for hiding this comment

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

LGTM! I am not familiar with this module so will not merge this in case you want to do a minor release before introducing breaking changes.

@smortex smortex removed the enhancement New feature or request label May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants