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

Lemminx extension logging can't be enabled with current Lemminx Eclipse (and IDEA?) integration #138

Closed
scottkurz opened this issue Nov 21, 2022 · 4 comments · Fixed by #162
Labels
LemMinX Liberty extension to Eclipse Lemminx should-fix

Comments

@scottkurz
Copy link
Member

scottkurz commented Nov 21, 2022

Though we have java.util.logging instrumentation in the lemminx-liberty extension, there doesn't seem to be a way to enable it via Liberty Tools Eclipse integration.

Some of the reason is discussed here: eclipse-wildwebdeveloper/wildwebdeveloper#974.

I was able to prototype a couple workarounds in the near term:

IDEA 1

  1. Programmatically add ConsoleHandler to the root Logger, e.g. in a static/init block: Logger.getLogger("").addHandler(new ConsoleHandler());
  2. If we decide this should only be done conditionally, we can key off of a sysprop with a name like: io.openliberty.tools.lcls.logging.proxy=true. Why "proxy"? Well, as it turns out, the Eclipse WWD integration will pass through properties with "proxy" in the name from the IDE config (e.g. eclipse.ini) to the XML LSP JVM. So that would "work".
  3. "Promote" everything we want to see to INFO level. (Or do this programmatically but only conditionally along with the enablement itself in 2.)

Note this could be noisy, I haven't tried it. On the other hand, I'm not sure we've got the perfect balance as-is...e.g. the log message "Using cached Liberty schema file located at: " + serverXSDFile.toString()) gets logged an awful lot. Not sure how much time's been spent getting the message levels right.

IDEA 2

  1. Tell users to run with env var LEMMINX_DEBUG=true
  2. Replace JUL calls with System.err.println(msg) (we can't just use System.out since Lemminx sets that to a NoOp stream).
  3. As with IDEA 1, if this is conditional we need to use a sysprop with 'proxy' in the name

The 2nd ends up a good bit noisier since we're opening this up to all of Lemminx, e.g. the m2e in Eclipse gives quite a few stack traces.

@scottkurz
Copy link
Member Author

Just updating with a follow-up from the eclipse-wildwebdeveloper/wildwebdeveloper#974 discussion... there seems to be a PR eclipse-wildwebdeveloper/wildwebdeveloper#1040 intended to improve logging.

@evie-lau evie-lau added the LemMinX Liberty extension to Eclipse Lemminx label Jan 23, 2023
@scottkurz
Copy link
Member Author

I look a look at the PR in the last comment. It seems to give us a way to enable every log message at INFO level or higher, at least in Eclipse. If that's good enough then it'd seem the TODO would be to promote everything to at least INFO trace level (the FINE would never get logged).

I don't think that's going to be good enough long term but we can work through that with Lemminx, WWD, etc. It'd be a step forwards in Eclipse at least.

@cherylking
Copy link
Member

cherylking commented Feb 28, 2023

the TODO would be to promote everything to at least INFO trace level (the FINE would never get logged).

@scottkurz I was looking through our logging and I only see us references to LOGGER.info, LOGGER.warning and LOGGER.error. I saw no references to FINE. Is there an action to be taken in LCLS for this issue?

Updated: Just found LOGGER.fine in four files. I will use this issue to update those to LOGGER.info.

@scottkurz
Copy link
Member Author

scottkurz commented Feb 28, 2023

That seems like a good useful step. There's been some work and issues in the other projects but not sure there's a need to keep this current issue open beyond that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LemMinX Liberty extension to Eclipse Lemminx should-fix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants