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

Support for Spring Boot log groups #102

Closed
martinlippert opened this Issue Oct 4, 2018 · 16 comments

Comments

Projects
None yet
4 participants
@martinlippert
Copy link
Member

martinlippert commented Oct 4, 2018

(migrated this over from spring-projects/spring-ide#325)

Spring Boot 2.1 introduced the concept of log groups with two out-of-the-box group: web, sql.

It would be nice if IJ had a way to auto-complete available groups on logging.levels. alongside the usual package browser.

If an additional group is created in the project (via logging.groups.xyz), auto-completing xyz would be very nice as well.

Please note that this is represented by the logger-name hint whose description has been matched as of Spring Boot 2.1.

@snicoll

This comment has been minimized.

Copy link
Member

snicoll commented Oct 10, 2018

In the meantime we've improved the definition of the logger-name value hint so that it can be used for both logging.levels keys and logging.groups values.

See spring-projects/spring-boot#14748 for more details

@spring-issuemaster

This comment has been minimized.

Copy link

spring-issuemaster commented Nov 9, 2018

(comment in Pivotal Tracker added by Kris De Volder:)

Actualy, because of this:
spring-projects/spring-boot#14740

This already automatically works for the 'built-in' log groups.

I did notice however that the descriptions from the metadata seem to be getting lost. (In the completion side panel where the description should be it is showing 'null').

@spring-issuemaster

This comment has been minimized.

Copy link

spring-issuemaster commented Nov 14, 2018

(comment in Pivotal Tracker added by Kris De Volder:)

I got this mostly working. With a few caveats.

  • it is only harvesting user-defined logging groups from resources on disk. So it will not be able to know about logging groups in the current file unless that file is saved first. This might be improved, but its a bit tricky.

  • Because lsp4e has no support for flie change events the 'AdHocPopertyIndex' on which this is based is not invalidated when a file is changed. So it may require a server restart to force a re-parse.

Thinking about it now, perhaps I could improve on this slioghtly by using document save events as 'dirty index' trigger. So I'll give that a try.

@spring-issuemaster spring-issuemaster added finished and removed started labels Nov 14, 2018

@spring-issuemaster

This comment has been minimized.

Copy link

spring-issuemaster commented Nov 14, 2018

(comment in Pivotal Tracker added by Kris De Volder:)

I added some logic to clear the index on document saves, this somewhat compensates for the lack of file system change events in lsp4e.

I found another bug/improvement which I should probably try to fix. But I'll treat it as a separate issue:

https://www.pivotaltracker.com/story/show/161964105

@kdvolder

This comment has been minimized.

Copy link
Member

kdvolder commented Nov 16, 2018

@snicoll

In the meantime we've improved the definition of the logger-name value hint so that it can be used for both logging.levels keys and logging.groups values.

I think you are referring to this: spring-projects/spring-boot#14740
Right?

But I think this is incomplete. (Or perhaps it is intentional?). With support implemented in STS now we are getting completions suggestion of 'sql' and 'web' in this example (the '<*>' denotes cursor where we ask for completion)

logging.level.<*> 

But in this context:

logging.group.whatever=<*>

We are only getting suggestions for package and class names (so 'sql' and 'web' are not suggested here).

I think the reason is that you have only added 'sql' and 'web' value hints to logging.level.keys. But not logging.group.values.

@spring-issuemaster

This comment has been minimized.

Copy link

spring-issuemaster commented Nov 16, 2018

(comment in Pivotal Tracker added by Kris De Volder:)

Delivering this. Tested with latest snapshot. It is working.

However, response to some completions is pretty slow sometimes, even with the caching support we have built-into it.

I am creating a ticket to try and optimise or tweak how we cache things to make this a bit more responsive.

@spring-issuemaster

This comment has been minimized.

Copy link

spring-issuemaster commented Nov 16, 2018

(comment in Pivotal Tracker added by Kris De Volder:)

Was going to delever this. But though its working, it feels very slow.

I actually remember it being a little faster before and I have a hunch of a last minute change I did to eliminate some duplicate suggestions may have something to do with this. Putting back to started state to explore this.

@snicoll

This comment has been minimized.

Copy link
Member

snicoll commented Nov 16, 2018

This is intentional. Meta-groups aren't supported.

@spring-issuemaster

This comment has been minimized.

Copy link

spring-issuemaster commented Nov 16, 2018

(comment in Pivotal Tracker added by Kris De Volder:)

Hmmm... not sure why but in runtime workbench it works a lot snappier it seems. I thought maybe its memory issue but attching to the language server it doesn't seem overly stressed by GC at all.

@spring-issuemaster

This comment has been minimized.

Copy link

spring-issuemaster commented Nov 16, 2018

(comment in Pivotal Tracker added by Kris De Volder:)

I've seen this stacktrace in server logs a couple times (pasted below).

It seems to correlate with when CA feels very slow.

I'm guessing it may have something to do with the problem. This method times out after 10s. So that at least means something was being blocked for 10s.

My current theory is that its not the loggerName provider that is slow, but something around resolving javadocs is either really slow, or being deadlocked somehow. And when this happens our server becomes unresponsive until that times out.

As I think this has not much todo with logger name provider at all, I am delivering this ticket and will log a ticket instead to try and figure out what is happening there and if/how we can avoid this problem.

12:40:28.137 [SimpleLanguaserver main thread] ERROR o.s.i.v.c.j.JdtLsJavadocProvider - 
java.util.concurrent.TimeoutException: null
	at java.util.concurrent.CompletableFuture.timedGet(CompletableFuture.java:1771)
	at java.util.concurrent.CompletableFuture.get(CompletableFuture.java:1915)
	at org.springframework.ide.vscode.commons.javadoc.JdtLsJavadocProvider.javadoc(JdtLsJavadocProvider.java:62)
	at org.springframework.ide.vscode.commons.javadoc.JdtLsJavadocProvider.getJavadoc(JdtLsJavadocProvider.java:72)
	at org.springframework.ide.vscode.commons.jandex.TypeImpl.getJavaDoc(TypeImpl.java:66)
	at org.springframework.ide.vscode.boot.metadata.util.PropertyDocUtils.documentJavaElement(PropertyDocUtils.java:77)
	at org.springframework.ide.vscode.boot.metadata.hints.StsValueHint.lambda$0(StsValueHint.java:128)
	at org.springframework.ide.vscode.commons.util.Renderables$11.renderAsMarkdown(Renderables.java:273)
	at org.springframework.ide.vscode.commons.util.Renderable.toMarkdown(Renderable.java:24)
	at org.springframework.ide.vscode.commons.languageserver.completion.VscodeCompletionEngineAdapter.toMarkdown(VscodeCompletionEngineAdapter.java:211)
	at org.springframework.ide.vscode.commons.languageserver.completion.VscodeCompletionEngineAdapter.resolveItem(VscodeCompletionEngineAdapter.java:189)
	at org.springframework.ide.vscode.commons.languageserver.completion.VscodeCompletionEngineAdapter.access$0(VscodeCompletionEngineAdapter.java:186)
	at org.springframework.ide.vscode.commons.languageserver.completion.VscodeCompletionEngineAdapter$LazyCompletionResolver.lambda$0(VscodeCompletionEngineAdapter.java:71)
	at org.springframework.ide.vscode.commons.languageserver.completion.VscodeCompletionEngineAdapter$LazyCompletionResolver.resolveNow(VscodeCompletionEngineAdapter.java:84)
	at org.springframework.ide.vscode.commons.languageserver.completion.VscodeCompletionEngineAdapter.resolveCompletion(VscodeCompletionEngineAdapter.java:279)
	at org.springframework.ide.vscode.commons.languageserver.util.SimpleTextDocumentService.lambda$5(SimpleTextDocumentService.java:295)
	at reactor.core.publisher.MonoCallable.call(MonoCallable.java:92)
	at reactor.core.publisher.FluxSubscribeOnCallable$CallableSubscribeOnSubscription.run(FluxSubscribeOnCallable.java:225)
	at reactor.core.scheduler.SchedulerTask.call(SchedulerTask.java:50)
	at reactor.core.scheduler.SchedulerTask.call(SchedulerTask.java:27)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:180)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
@kdvolder

This comment has been minimized.

Copy link
Member

kdvolder commented Nov 16, 2018

@snicoll

This is intentional. Meta-groups aren't supported.

Okay good. Unfortunately, I actually did do the extra effort to include user-defined logging group names as suggestions for the 'logger-name' provider. It seemed to me you were suggesting this was good idea. So that means when we do this:

logging.group.whatever=blah
logging.level.<*>

Then 'whatever' will be one of the suggestions. That I suppose is good but...unfortunately it will also be a suggestion here:

logging.group.whatever=blah
logging.group.another=<*>

This I take it, is bad. So it seems that we want two different interpretations for the 'logger-name' provider, or I have to add some kind of a special case to exclude the group names from one context but not another. Perhaps logger-name provider should get a parameter to indicate whether or not group-names should be included?

@snicoll

This comment has been minimized.

Copy link
Member

snicoll commented Nov 17, 2018

Perhaps logger-name provider should get a parameter to indicate whether or not group-names should be included?

There is a parameter. Please review the documentation and the metadata. This is also mentioned in the first comment of this issue.

@kdvolder

This comment has been minimized.

Copy link
Member

kdvolder commented Nov 19, 2018

Thanks I missed that somehow. Support for that is also implemented now.

@spring-issuemaster

This comment has been minimized.

Copy link

spring-issuemaster commented Nov 22, 2018

(comment in Pivotal Tracker added by Martin Lippert:)

The content-assist is indeed sometimes super super slow, feels to me like it is re-computing the whole index and/or re-scanning everything in the language server.

I have a workspace with quite a number of projects and the performance of the content-assist turns it not usable at all anymore in those situations.

@spring-issuemaster

This comment has been minimized.

Copy link

spring-issuemaster commented Nov 22, 2018

(comment in Pivotal Tracker added by Martin Lippert:)

I filed this #162151564 for the performance issue.

@spring-issuemaster

This comment has been minimized.

Copy link

spring-issuemaster commented Nov 22, 2018

(comment in Pivotal Tracker added by Kris De Volder:)

I filed this before https://www.pivotaltracker.com/story/show/162024238

I think some javadocs for completions are getting stuck and blocking everything (even the next content assist request is blocked if the previous one is still stuck). Stack traces in the 'stuck and waiting for completions' state I collected allways show a thread waiting for javadoc from the JDT side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.