search

eclipse-sisu / sisu-project

Sisu Inject
https://www.eclipse.org/sisu
Eclipse Public License 2.0
17 stars 15 forks source link

Raise problem reporting logs to DEBUG #36

Closed gnodet closed 2 years ago

gnodet commented 2 years ago

When a LinkageError happens during the instantiation of a bean, the error is logged at TRACE level, which is not very often displayed, especially in maven. This happens if a transitive dependency is missing in a maven extension for example, at the following location https://github.com/eclipse/sisu.inject/blob/ac8b554b9dabdfc41cebea081a9338dc99ad4864/org.eclipse.sisu.inject/src/org/eclipse/sisu/wire/DependencyAnalyzer.java#L238-L245

I suggest to raise a bit the level where potential problems are logged to DEBUG. At least, it's easier to see them and tone down rather than to have no way to figure out where the problem comes from when a bean is not instantiated.

See https://github.com/apache/maven/pull/619 and https://issues.apache.org/jira/browse/MNG-7342

michael-o commented 2 years ago

This is an antipattern to make log levels configurable. It does not solve the actual problem. Only the symptom.

gnodet commented 2 years ago

This is an antipattern to make log levels configurable. It does not solve the actual problem. Only the symptom.

Yes, that's not what I did in the PR, let me change the title of the PR.

michael-o commented 2 years ago

This is an antipattern to make log levels configurable. It does not solve the actual problem. Only the symptom.

Yes, that's not what I did in the PR, let me change the title of the PR.

Then, I agree, but it should be at least WARN. LinkageError is severe. If Sisu cannot continue, then it must be ERROR.

mcculls commented 2 years ago

The reason it's not logged as WARN or ERROR is because the container cannot tell whether the missing dependency is expected to always be there. It may be an optional dependency that is only there in certain deployments.

For a concrete example a while back Nexus 2 shipped with different component implementations for non-HA and HA modes. The HA implementations built on top of Hazelcast. When Nexus 2 was deployed with Hazelcast the HA implementations took precedence, whereas when it was deployed without Hazelcast the HA implementations didn't load and Nexus 2 fell back to use the non-HA implementations. Logging a WARN or ERROR in that scenario would be incorrect, because it was an optional dependency and the absence of it was not an error.

Likewise you could have different component implementations for different JDK versions - the absence of a particular class would again not be an error if that component was only expected to load when running a particular JDK.

michael-o commented 2 years ago

Hard nut to crack

mcculls commented 2 years ago

Bumping the level to DEBUG for these calls makes sense though, so I'll get that merged today