Skip to content

Add environment variable arbiter #682

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

Closed
wants to merge 4 commits into from
Closed

Add environment variable arbiter #682

wants to merge 4 commits into from

Conversation

MouettE-SC
Copy link
Contributor

This is a simple arbiter using environment variables functionnally based on the existing system property arbiter (on which I corrected some typos).

I tried to write unit tests for it but injecting environment variables in junit is not easy. I tried system-rules but it didn't work so I did not include it in this PR. I tested the arbiter in my application and it seems to work fine.

@rgoers
Copy link
Member

rgoers commented Jan 5, 2022

Thanks for this. I should be able to create the unit tests for it.

@MouettE-SC
Copy link
Contributor Author

MouettE-SC commented Jan 5, 2022

Thanks for this. I should be able to create the unit tests for it.

Great ! I've added the tests I wrote, based on the current system property ones with system-rules which does not work on my machine. That way test structure is already done.

@MouettE-SC
Copy link
Contributor Author

Sorry, forgot to comment that I was able to create working tests for the environment arbiter using system-lambda. Hope you'll find them satisfying !

@MouettE-SC
Copy link
Contributor Author

Let me know if there is anything I can do to help, like opening an issue on Jira, writing the documentation part or anything, I'll be happy to !

@Zhnz
Copy link

Zhnz commented Mar 10, 2022

Wouldn't be using a PluginFactory instead of a PluginBuilderFactory way shorter? I think that would be better. I implemented my own custom EnvironmentArbiter this way (because this one still isn't merged :P)


public static class Builder implements org.apache.logging.log4j.core.util.Builder<EnvironmentArbiter> {

public static final String ATTR_VARIABLE_NAME = "variableName";
Copy link

Choose a reason for hiding this comment

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

Why not stick with the names used by the SystemPropertyArbiter ("propertyName" and "propertyValue") ?

@jbisotti
Copy link

What is the status of the PR? Will it be merged soon? This is exactly what I need.

@ppkarwasz ppkarwasz self-assigned this Feb 15, 2023
@ppkarwasz
Copy link
Contributor

@jbisotti,

I am afraid this PR fell out of our long todo list. Since we are cutting 2.20.0 right now, I am afraid it will not make it into the next release, but I'll take care of this as soon as we are done.

BTW: you can repackage the class into a separate JAR and add it to any Log4j2 release.

@MouettE-SC
Copy link
Contributor Author

I've rebased the release branch on this given the fact that it is kinda old. I didn't manage to run the whole tests yet.

@vy vy deleted the branch apache:release-2.x February 28, 2023 15:02
@vy vy closed this Feb 28, 2023
@MouettE-SC MouettE-SC deleted the release-2.x branch February 28, 2023 15:32
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.

7 participants