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

Add log4j2 xml-based config support #900

Merged

Conversation

randomstatistic
Copy link
Contributor

This allows the use of the log4j2 InstrumentedAppender without needing code-based config. ie:

<?xml version="1.0" encoding="UTF-8"?>
<Configuration status="INFO" name="log4j2-config" packages="com.codahale.metrics.log4j2">
    <Appenders>
        <MetricsAppender name="metrics" registryName="shared-metrics-registry"/>
    </Appenders>
    <Loggers>
        <Root level="INFO">
            <AppenderRef ref="metrics" />
        </Root>
    </Loggers>
</Configuration>

For the most part, this was simply a matter of adding the right log4j2 plugin hooks.

However, I ended up adding a new constructor because the concept of the appender name and the metrics prefix were tightly coupled. This was an irritation because the appender name is used as the logger reference in the XML config.
Also, since the test I added was actually initializing a logging system, I removed the test parallelism for the metrics-log4j2 suite to avoid potential race conditions among the two test files.

Please note that this is a patch against 3.1-maintenance. I wasn't sure where most development is occurring and what the best place to put it would be.

@randomstatistic
Copy link
Contributor Author

Hm, apparently my change to the documentation caused the ExponentiallyDecayingReservoirTest to fail, and only on Java7. That... seems unlikely.
Are the tests flaky on Travis/3.1-maintenance?

@ryantenney
Copy link
Member

Some tests are timing dependent and can fail randomly. Can you rerun the build? If not I'll rerun it tomorrow.

@randomstatistic
Copy link
Contributor Author

My phone and I can't find how to trigger that without pushing a commit. I can do that, but not until, well, probably tomorrow.

@ghost
Copy link

ghost commented Feb 1, 2016

would love to see this make it in - it's awkward to mix xml based logger config + InstrumentedAppender

@ryantenney
Copy link
Member

@randomstatistic Would you mind submitting a pull request to the master branch as well?

ryantenney added a commit that referenced this pull request Feb 1, 2016
Add log4j2 xml-based config support
@ryantenney ryantenney merged commit a3f716b into dropwizard:3.1-maintenance Feb 1, 2016
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.

None yet

2 participants