mikehardy / google-analytics-java

Open Source license compatible Java API for Google Analytics
9 stars 3 forks source link

Conflict between SLF4J backends caused by slf4j-simple dependency #20

Closed robertvazan closed 5 years ago

robertvazan commented 5 years ago

I saw that you are adding test dependency for slf4j-simple to get logging in junit tests:

        <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>slf4j-simple</artifactId>
            <version>${slf4j-version}</version>
            <scope>test</scope>
        </dependency>

I thought this is a good idea and I tried it in my own projects only to find out this dependency gets propagated to other projects despite the test scope. Once propagated, slf4j-simple may take priority over the SLF4J backend chosen by the application. Backend priority is influenced by dependency order in pom.xml. I am observing this behavior in Eclipse with enabled resolution of dependencies in workspace if that matters.

Do you have any idea why is this happening? Any way to fix it? If not fixed, this will make some people wonder why did their logging setup suddenly stop working after they added dependency on this project.

mikehardy commented 5 years ago

Interesting - thank you very much for posting this. I hadn't noticed it taking over AnkiDroid's slf4j-timber binding, but I definitely don't want to pollute anyone's logging configuration, those are painful problems to figure out.

I'll just remove this and figure out a different solution for local logging for the library while under test

mikehardy commented 5 years ago

Worth noting that if you discover a root cause for this please let me know, as the solutions for logging while under test are unattractive otherwise - I believe I need to have the library local via a non-maven mechanism and put it on the classpath on the command-line. All possible with scripting but introduces platform scripting issues etc. In the meantime, with it gone I won't have to worry about blowing up someone's config...

robertvazan commented 5 years ago

Safe choice. I am going to risk it and keep this solution in my own projects for now, using dependency order in pom.xml to pick the right SLF4J backend in my own apps. I suspect Eclipse's maven integration just works incorrectly when workspace dependency resolution is enabled. When I make a release of one of my libraries, I will disable workspace dependency resolution and see whether the problem persists. I cannot believe that regular maven (or even Eclipse's maven integration) goes out of its way to download test dependecies from Maven Central.

robertvazan commented 5 years ago

I have tested this now with an older version of my library (resolved from Maven Central) and the current version (resolved in Eclipse workspace). Both versions behave the same. The test-only logger is picked up shortly after I choose library version in pom.xml, save the pom.xml, and launch the application. If I wait a few seconds, the application-specified logger is picked.

There is obviously something strange going on, but I cannot reproduce it beyond one rare case of a race rule in Eclipse. The question is why was I able to reproduce it more reliably before? It turns out this bug is visible after any change to pom.xml. In my previous tests, I experimented with dependency order to figure out what is causing it and I unknowingly triggered the bug all the time.

So the conclusion is that including slf4j-simple in tests is safe except for the rare case when pom.xml was just modified. The race rule is likely a bug in Eclipse. It should be reported, but Eclipse's bug tracker is down at the moment. I might try to report the bug later.

mikehardy commented 5 years ago

It should be reported, but Eclipse's bug tracker is down at the moment. I might try to report the bug later.

:laughing: - still good information though - if you manage to get an upstream issue filed and cross-link it here I can track also, and re-add that dependency. It's still by far the easiest way to log during development on the library and I don't have a workaround yet.

Thanks for continuing to dig in - these racy issues are my least favorite things to try to nail down

robertvazan commented 5 years ago

Eclipse's bug tracker is online again. I have submitted a bug report for this:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=540534

mikehardy commented 5 years ago

Great - I'm cc'd on that bug now. We'll see how it goes - I wonder if a very simple repro for them that had a custom build task that did nothing but sleep (to let the bad side of the race win) might help, but I'm just naively brainstorming there...