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

ShortenedThrowableConverter: <exclude> and <exclusions> are mutually exclusive #876

Closed
brenuart opened this issue Oct 11, 2022 · 2 comments · Fixed by #881
Closed

ShortenedThrowableConverter: <exclude> and <exclusions> are mutually exclusive #876

brenuart opened this issue Oct 11, 2022 · 2 comments · Fixed by #881
Labels
warn/behavior-change Breaking change of publicly advertised behavior
Milestone

Comments

@brenuart
Copy link
Collaborator

Suppose the following configuration:

<throwableConverter class="net.logstash.logback.stacktrace.ShortenedThrowableConverter">
    <exclude>pattern1</exclude>
    <exclusions>${STACK_EXCLUSIONS}</exclusions>
    <exclude>pattern2</exclude>
</throwableConverter>

Using the <exclusions> with an environment variable is an easy way to configure additional exclusion patterns in addition to those already configured in the XML file. However, it happens that <exclusions> clears the exclusions already configured and replace them with new values. This sample configuration results in only TWO exclusion patterns with the first one being ignored...

To avoid confusion the <exclusions> keyword should ADD new patterns instead of replacing those already defined.

@brenuart
Copy link
Collaborator Author

@philsttr What do you think of this?

@philsttr
Copy link
Collaborator

philsttr commented Oct 12, 2022

Sounds good.

I think there is a low probability that anyone is using both.

Worth calling out as a slight backwards incompatibility in the release notes. But I don't think it requires a major version bump, since this seems more like a bugfix.

@philsttr philsttr added the warn/api-change Breaking change with compilation or xml configuration errors label Oct 12, 2022
@brenuart brenuart added this to the 7.3 milestone Oct 16, 2022
brenuart added a commit that referenced this issue Oct 16, 2022
…exclusive (#881)

* <exclusions> and <truncateAfters> should ADD to the list instead of replacing content
* Also add a common utility method in StringUtils to convert a comma delimited list of strings into an array.
* Rename setExclusions() into addExclusions() to better reflect the intend

Related to issue #876
@philsttr philsttr added warn/behavior-change Breaking change of publicly advertised behavior and removed warn/api-change Breaking change with compilation or xml configuration errors labels Feb 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
warn/behavior-change Breaking change of publicly advertised behavior
Projects
None yet
2 participants