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

setting to exclude MDC keys from being sent to Elastic #57

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

PeterFokkinga
Copy link

useful when a particular key (or keys) may contain a lot of text that is of no particular use in your Elastic index, or when you want to explicitly add the key as an object property (see PR #56 for that)

keys are matched case sensitive, without wildcard support

useful when a particular key (or keys) may contain a lot of text that is of no particular use in your Elastic index, or when you want to explicitly add the key as an object property

keys are matched case sensitive, without wildcard support
@remk
Copy link

remk commented Jun 28, 2019

It would help us to exclude some MDC keys which conflict with some logback properties.
Since elastic search 6.X rejects duplicate keys. This PR seems to be a nice solution to our problem ?

Is there a plan to merge it ?

}
}
return result;
Copy link

Choose a reason for hiding this comment

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

Is there a reason this list is re-allocated and populated anew every time a log entry is written? Why is this not parsed once when reading the config and then re-used on subsequent accesses? Looks like this could incur a performance overhead.

List instead of a map or set is probably fine, because this will likely only matter when there are hundreds of items in the collection and calculating the hash code of each tested key can be avoided.

import com.internetitem.logback.elasticsearch.util.*;

import java.io.IOException;
import java.util.*;
Copy link

Choose a reason for hiding this comment

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

New to the project, but I don't think star-imports are used anywhere else (and haven't been used before in this file).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants