Add logback tracing to Java LS #2116
Conversation
|
Nice. If this works, we can hopefully close out redhat-developer/vscode-java#2375 (comment) Is it possible to combine |
No, it isn't. jdt.ls.start have to start first so can't be a fragment.
No, it couldn't. We have to call setProperty very early if exists ch.qos.logback |
|
Having a separate start plugin to set logback property looks like kind of hack. I don't get the point why we cannot do it in jdt.ls bundle. |
@testforstephen You may want to take a look at #2096 (comment) and #2116 (comment) |
|
Also, for the nicer solution of logging directly to Eclipse's logs, I think the logback framework (ch.qos.logback.classic?) only checks its classpath for custom appenders, so effectively that means we need to use |
snjeza
changed the title
|
test this please |
1 similar comment
|
test this please |
|
Just one observation about the logging. Importing a project like eclipse/lemminx I see about 15 messages going into the server logs. However, I remember seeing many more when I was able to enable verbose/debug logging. For example : .out-jdt.ls-20220610140605.log It's a bit excessive, but is it possible to have control over the DEBUG messages also ? i think they're the ones that are currently missing. I thought adding a |
|
|
@rgrunber Could you, please, check again? |
|
test this please |
3 similar comments
|
test this please |
|
test this please |
|
test this please |
|
Works for me when I just tried disabling the Note that, I don't mind regressing on having the ability to supply a custom logback file through system properties. It was a nice-to-have feature, but ultimately, the most important thing is just being able to log the data with some sane defaults (we can provide). diffdiff --git a/org.eclipse.jdt.ls.core/META-INF/MANIFEST.MF b/org.eclipse.jdt.ls.core/META-INF/MANIFEST.MF
index 00190061..c53b74b2 100644
--- a/org.eclipse.jdt.ls.core/META-INF/MANIFEST.MF
+++ b/org.eclipse.jdt.ls.core/META-INF/MANIFEST.MF
@@ -30,7 +30,10 @@ Require-Bundle: org.eclipse.core.runtime;bundle-version="3.12.0",
org.eclipse.xtext.xbase.lib,
org.eclipse.core.filesystem;bundle-version="1.7.0",
org.eclipse.jdt.apt.pluggable.core;bundle-version="1.2.0";resolution:=optional,
- org.eclipse.m2e.apt.core;bundle-version="1.3.0";resolution:=optional
+ org.eclipse.m2e.apt.core;bundle-version="1.3.0";resolution:=optional,
+ ch.qos.logback.classic;bundle-version="1.2.3",
+ ch.qos.logback.core;bundle-version="1.2.3",
+ org.slf4j.api;bundle-version="1.7.30"
Export-Package: org.eclipse.jdt.ls.core.internal;x-friends:="org.eclipse.jdt.ls.tests,org.eclipse.jdt.ls.tests.syntaxserver",
org.eclipse.jdt.ls.core.internal.codemanipulation;x-friends:="org.eclipse.jdt.ls.tests",
org.eclipse.jdt.ls.core.internal.commands;x-friends:="org.eclipse.jdt.ls.tests",
diff --git a/org.eclipse.jdt.ls.core/build.properties b/org.eclipse.jdt.ls.core/build.properties
index 829276d3..3ca486ad 100644
--- a/org.eclipse.jdt.ls.core/build.properties
+++ b/org.eclipse.jdt.ls.core/build.properties
@@ -7,6 +7,8 @@ bin.includes = META-INF/,\
lib/remark-1.2.0.jar,\
lifecycle-mapping-metadata.xml,\
plugin.properties,\
+ logback.xml,\
+ emptylogback.xml,\
gradle/checksums/checksums.json,\
gradle/checksums/versions.json
src.includes = src/
diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaLanguageServerPlugin.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaLanguageServerPlugin.java
index 33fea334..e85133e9 100644
--- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaLanguageServerPlugin.java
+++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaLanguageServerPlugin.java
@@ -24,9 +24,12 @@ import java.io.PrintStream;
import java.net.Authenticator;
import java.net.InetSocketAddress;
import java.net.PasswordAuthentication;
+import java.net.URL;
import java.nio.channels.AsynchronousServerSocketChannel;
import java.nio.channels.AsynchronousSocketChannel;
import java.nio.channels.Channels;
+import java.nio.file.Files;
+import java.nio.file.StandardCopyOption;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.concurrent.ExecutionException;
@@ -81,11 +84,23 @@ import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
import org.osgi.framework.BundleException;
import org.osgi.util.tracker.ServiceTracker;
+import org.slf4j.ILoggerFactory;
+import org.slf4j.LoggerFactory;
+import org.slf4j.helpers.SubstituteLoggerFactory;
import com.google.common.base.Throwables;
+import ch.qos.logback.classic.LoggerContext;
+import ch.qos.logback.classic.joran.JoranConfigurator;
+import ch.qos.logback.core.joran.spi.JoranException;
+import ch.qos.logback.core.util.StatusPrinter;
+
public class JavaLanguageServerPlugin extends Plugin {
+ private static final String LOGBACK_CONFIG_FILE_PROPERTY = "logback.configurationFile";
+ private static final String LOGBACK_DEFAULT_FILENAME = "logback.xml";
+ private static final String LOGBACK_EMPTY_FILENAME = "emptylogback.xml";
+
private static final String JDT_UI_PLUGIN = "org.eclipse.jdt.ui";
public static final String MANUAL = "Manual";
public static final String DIRECT = "Direct";
@@ -173,6 +188,22 @@ public class JavaLanguageServerPlugin extends Plugin {
JavaLanguageServerPlugin.pluginInstance = this;
setPreferenceNodeId();
+ if (System.getProperty(LOGBACK_CONFIG_FILE_PROPERTY) == null) {
+ File stateDir = getStateLocation().toFile();
+ String fileName = isDebug ? LOGBACK_DEFAULT_FILENAME : LOGBACK_EMPTY_FILENAME;
+ File configFile = new File(stateDir, fileName);
+ if (!configFile.isFile()) {
+ try (InputStream is = bundleContext.getBundle().getEntry(fileName).openStream(); FileOutputStream fos = new FileOutputStream(configFile)) {
+ Files.copy(is, configFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
+ } catch (Exception e) {
+ logException(e.getMessage(), e);
+ }
+ }
+ loadConfiguration(configFile.toURI().toURL());
+ // ch.qos.logback.classic.util.ContextInitializer.CONFIG_FILE_PROPERTY
+ // System.setProperty(LOGBACK_CONFIG_FILE_PROPERTY, configFile.getAbsolutePath());
+ }
+
if (JDTEnvironmentUtils.isSyntaxServer()) {
disableServices();
preferenceManager = new PreferenceManager();
@@ -200,6 +231,42 @@ public class JavaLanguageServerPlugin extends Plugin {
}
}
+ public static void loadConfiguration(URL configFile) throws JoranException {
+ LoggerContext lc = getLoggerContext();
+ if (lc == null) {
+ logError("Cannot initialize logback");
+ return;
+ }
+ lc.reset();
+ JoranConfigurator configurator = new JoranConfigurator();
+ configurator.setContext(lc);
+ configurator.doConfigure(configFile);
+ StatusPrinter.printInCaseOfErrorsOrWarnings(lc);
+ // logJavaProperties(LoggerFactory.getLogger(JavaLanguageServerStart.class));
+ }
+
+ private static LoggerContext getLoggerContext() {
+ ILoggerFactory loggerFactory = LoggerFactory.getILoggerFactory();
+
+ for (int i = 0; loggerFactory instanceof SubstituteLoggerFactory && i < 100; i++) {
+ try {
+ Thread.sleep(50);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt(); // ignore but re-interrupt
+ }
+ loggerFactory = LoggerFactory.getILoggerFactory();
+ }
+
+ if (loggerFactory instanceof LoggerContext) {
+ return (LoggerContext) loggerFactory;
+ }
+ String msg = loggerFactory == null ? // Is null possible?
+ "SLF4J logger factory is null" //$NON-NLS-1$
+ : "SLF4J logger factory is not an instance of LoggerContext: " + loggerFactory.getClass().getName(); // $NON-NLS-1$
+ logInfo(msg);
+ return null;
+ }
+
private void disableServices() {
try {
ProjectsManager.setAutoBuilding(false);
diff --git a/org.eclipse.jdt.ls.product/languageServer.product b/org.eclipse.jdt.ls.product/languageServer.product
index 404c10ea..d8ccede9 100644
--- a/org.eclipse.jdt.ls.product/languageServer.product
+++ b/org.eclipse.jdt.ls.product/languageServer.product
@@ -57,7 +57,6 @@
<plugin id="org.eclipse.jdt.launching.macosx"/>
<plugin id="org.eclipse.jdt.ls.core"/>
<plugin id="org.eclipse.jdt.ls.logback.appender" fragment="true"/>
- <plugin id="org.eclipse.jdt.ls.start"/>
<plugin id="org.eclipse.m2e.apt.core"/>
<plugin id="org.eclipse.m2e.core"/>
<plugin id="org.eclipse.m2e.jdt"/>
@@ -77,7 +76,6 @@
<plugin id="org.eclipse.equinox.common" autoStart="true" startLevel="2" />
<plugin id="org.eclipse.jdt.core" autoStart="false" startLevel="0" />
<plugin id="org.eclipse.jdt.launching" autoStart="false" startLevel="0" />
- <plugin id="org.eclipse.jdt.ls.start" autoStart="true" startLevel="0" />
</configurations>
<preferencesInfo> |
Could you try to create a vsix with Java 17 jre and start a maven project in a new workspace? See #2096 (comment)
You can try to set -Dlogback.configurationFile=<your_logback.xml> jvmargs. It should work. |
You're right. It would break that case. |
| private static final String LOGBACK_CONFIG_FILE_PROPERTY = "logback.configurationFile"; | ||
| private static final String LOGBACK_DEFAULT_FILENAME = "logback.xml"; | ||
| private static final String LOGBACK_EMPTY_FILENAME = "emptylogback.xml"; | ||
| public static final String PLUGIN_ID = "org.eclipse.jdt.ls.start"; |
There was a problem hiding this comment.
If we have to create a new bundle to solve this problem, I would rename it from "org.eclispe.jdt.ls.start" to "org.eclipse.jdt.ls.logback", and make this bundle only used for handling logback logic.
| try { | ||
| Platform.getBundle(CORE_PLUGIN_ID).start(Bundle.START_TRANSIENT); | ||
| } catch (BundleException e) { | ||
| logException(e.getMessage(), e); | ||
| } |
There was a problem hiding this comment.
Why do we need to start jdt.ls.core bundle manually here? It looks unnecessary for me.
There was a problem hiding this comment.
You are right. I have removed it.
|
@rgrunber @testforstephen I have updated the PR. I have also updated the target platform to 4.24 because 4.24-I-builds doesn't exist. |
snjeza
force-pushed
the
issue-2108
branch
3 times, most recently
from
0154920
to
e57bd57
Compare
|
@testforstephen , so currently, the m2e logs are only appended to the on loading a project like spring-petclinic : |
|
test this please |
Good to know it. Since it won't append to the .log file by default, so it's not a problem for language server daemon. |
|
When I execute "Java: Clean Java Language Server Workspace" commad and reload window, it reports the following exception. |
|
@testforstephen I can reproduce the issue on Windows. I have fixed it. Could you, please, try again. |
|
With the latest changes, I didn't see the exception above. But I found another issue. When I removed |
@testforstephen I can't reproduce the issue. Could you attach your server log? |
It turns out I have enabled jdt.ls.debug somewhere. After I cleaned it, no m2e log is printed to .log file. |
|
@rgrunber @testforstephen I have updated the PR.
The org.eclipse.jdt.ls.logback plugin is not needed. |
|
I think with |
| Bundle-Version: 1.13.0.qualifier | ||
| Fragment-Host: ch.qos.logback.classic | ||
| Automatic-Module-Name: org.eclipse.jdt.ls.logback.appender | ||
| Bundle-RequiredExecutionEnvironment: JavaSE-11 |
There was a problem hiding this comment.
it should be JavaSE-17
Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
Fixes #2108
You can test in the following way
You can override it using -Dlogback.configurationFile=<your-logback.xml>
Notice: Now, m2e.core requires Java-17
Java LS tests fail because of that.
Signed-off-by: Snjezana Peco snjezana.peco@redhat.com