@Conditional… and @Bean highlighting in Java editor displays irrelevant information

[wilkinsona]

I have a couple of workspaces, one with Spring Boot 2.2's source code mounted and another with the Initializr and start.spring.io source code mounted. In the latter, which depends on Spring Boot 2.1, I'm running StartApplication. Back in my Spring Boot 2.2 workspace, I'm seeing green highlighting of @Bean and @Conditional…. Hovering over them, I can see that they're using information from the StartApplication process to decide what to highlight. Unfortunately, this information is confusing and misleading in my case. The source of information is using a different version of Boot and it's running in a completely separate workspace so I would not expect it to be used.

[kdvolder]

This is sort of a known problem/limitation. The language server that creates the hovers scrapes every process on the local machine regardless of how the process was created. This has benefits and drawbacks. The biggest benefit is that you can run things in different ways, and get the hovers displaying in your editor regardless. E.g. you can run the boot app from the CLI using mvn or gradle run instead of the IDE. Or similarly, you could launch your boot app from the Eclipse IDE, then open some of the source code in vscode, and still get the hovers.

The drawback is that, as you noticed, it may display information when it shouldn't. This would only happen when the live data 'appears' to match the source code you are looking at. This typically means that at least the code you are looking at is actually for the same thing you are looking at, but it may be for a different version.

We have discussed about this amongst ourselves a few times. Unfortunately this is not an easy problem to solve (unless we explicitly limit the processes that are being scraped to just the processes launched from the IDE, in which case we completely control the process and know exactly where it came from). We finally decided that it is probably better to err on the side of displaying too much information (even potentially irrelevant information) versus making it hard to get the the information displaying at all for certain common use cases.

[kdvolder]

BTW: there may be an opportunity for spring boot actuator to help solve this hard problem. E.g. perhaps actuator could expose some metadata through an endpoint that could be used somehow by the tooling to determine precise version / origin of the source code that the process is running. The missing link here is ... when STS sees a process, it needs a reliable way to match it agains source code in the editor and determine whether they are from 'the same version of the project / source code'.

[wilkinsona]

We finally decided that it is probably better to err on the side of displaying too much information (even potentially irrelevant information) versus making it hard to get the the information displaying at all for certain common use cases.

For the way I work, I'd much rather have limited information that I know is accurate and that I can trust than more information that may be inaccurate. As soon as I realise that the information may be inaccurate, I have to either ignore it to avoid being misled or treat it with suspicion and spend time figuring out if it's accurate before trusting it. My feeling at the moment is that the information is inaccurate more often than not so I'd like to be able to ignore it as easily as possible. To that end, is it possible to turn this functionality off?

BTW: there may be an opportunity for spring boot actuator to help solve this hard problem. E.g. perhaps actuator could expose some metadata through an endpoint that could be used somehow by the tooling to determine precise version / origin of the source code that the process is running.

I don't think you can rely on Actuator for a solution, unfortunately. If you want to be able to gather information from any Spring Boot app, and not just those launched by STS, there's no guarantee that the necessary endpoint will be enabled or exposed. It is increasingly likely that it won't be in 2.2 where JMX is disabled by default for performance reasons and the endpoints exposed over HTTP will remain as they were in 2.1 (just health and info).

The language server that creates the hovers scrapes every process on the local machine regardless of how the process was created.

If you are in this business already, I wonder if you could look at the -classpath or -jar arguments passed into the JVM and check the locations to which they point to identify the versions of its dependencies.

[martinlippert]

@wilkinsona first of all, thanks for the feedback and input, much appreciated.

To that end, is it possible to turn this functionality off?

Yes. You can enable/disable this feature in the Preferences -> Language Servers -> Spring Language Servers -> Spring Boot Language Server -> Spring Boot Java, there is a checkbox for Live Boot Hint Decorators.

I don't think you can rely on Actuator for a solution, unfortunately. If you want to be able to gather information from any Spring Boot app, and not just those launched by STS, there's no guarantee that the necessary endpoint will be enabled or exposed. It is increasingly likely that it won't be in 2.2 where JMX is disabled by default for performance reasons and the endpoints exposed over HTTP will remain as they were in 2.1 (just health and info).

We indeed rely on the actuators being on the classpath and exposing the necessary information via JMX endpoints. This is the way we get the detailed information about the beans, the injections, conditions, environments, etc. from the Spring Boot app process.

We are aware of the change in Boot 2.2 where JMX is disabled by default (as discussed in the related GitHub issue about that change) and allow users of STS to active/deactivate JMX in the launch configs. For processes that are started out of STS (command line, VSCode Java launcher, maven, etc) we indeed rely on the user to configure the JMX exposure themselves.

If you are in this business already, I wonder if you could look at the -classpath or -jar arguments passed into the JVM and check the locations to which they point to identify the versions of its dependencies.

We tried hard and evaluated various ways to match the found processes to projects in your workspace. We tried to use the -classpath and -jar settings of the process to match that against the projects output folders in your IDE, but that breaks for processes launched with the thin jar launcher or via the maven build. So that works for some cases, not for others.

The decision made then was to rely on the type information (in the beans case) and match that against the types in the project, assuming that it is unlikely that a process running on your machine uses the same exact types (incl. package identifiers) as the project in your workspace, but the project doesn't correspond to the running process.

Maybe it is worth to re-think this design decision, especially in the context of types defined in libraries and/or use additional data (for example versions), as you mentioned. Definitely worth to think about that.

As a last resort we added the option to let the user exactly match the running process to a project in your workspace by setting a system property to your running app via: -Dspring.boot.project.name=<name-of-your-project-in-your-workspace>. If the language server finds that property on the running process, it matches it against projects in your workspace with that exact same name exclusively. That way you could specify that your running process belongs to a specific project in your workspace.

However, this wouldn't help in your setting, I guess, where you have multiple workspaces open with the same projects, so you would have the same project names.

I still like this feature "just working out-of-the-box" for the standard use case, but the "magic" behind the scenes should be more visible and configurable for the user. Maybe a simple view in Eclipse would do the trick where it shows a list of the running and identified processes on the machine and to which projects it connected those processes for the hover information. And then let the user enable/disable this for individual processes-projects combinations or let them change this association, so that they can define themselves to which project a specific process belongs. WDYT?

[wilkinsona]

I agree that it can be a useful feature and that there's value in it working out-of-the-box, but only if it can provide reliable and accurate information. If requiring zero setup compromises the accuracy of the information, then I think some setup should be required. I'd much rather see the information by default and be delighted by its usefulness and accuracy in a limited set of cases than see it all the time and be annoyed by its inaccuracy. The former leaves me wanting to learn more about how I can enable it in other cases while retaining accuracy, the latter leaves me wanting to turn the feature off.

[kdvolder]

To that end, is it possible to turn this functionality off?

Yes. Look around the language server preference and disable the "Live Boot Hint Decorators". You can find this under:

"Language Servers >> Spring Language Servers >> Spring Boot Java".

but only if it can provide reliable and accurate information

As I said this particular problem would be best solvable by us collaborating on finding ways to make this project <-> process relationship more easily discoverable by the tools (or anyone for that matter) via the actuator. I don't think it is really that far fetched that it would be useful in a production environment to be able to access a actuator endpoint and get some data back that unambgiuously tells you exactly what source code and version you are running there.. is it? In fact I imagine this might be quite valuable in a production environment. If so, and if it could be done while also keeping in mind supporting the fixing of your valid complaint about the live hovers accuracy, I think that would be a win/win.

[kdvolder]

there's no guarantee that the necessary endpoint will be enabled or exposed.

That is not such a big problem tough, if there is no data then the data will not be inaccurate and you won't therefore see any inaccurate data in your editor.

The point is, if the endpoint exists and is enabled then we can make things work better. Right now all we can do is make some wild guesses... essentially.... so it will never be accurate.

[wilkinsona]

I don't think it is really that far fetched that it would be useful in a production environment to be able to access a actuator endpoint and get some data back that unambgiuously tells you exactly what source code and version you are running there.. is it?

No, not at all. We have the info endpoint and git.properties combined with the app's git repository for this use case. A downside is that there's one step of indirection involved to get to, for example, the versions of dependencies that are being used. On the other hand a positive is that it lets you know everything about the app, not just what we've predicted you might want to know via a separate endpoint. It's also arguably more secure and would be far less damaging if accidentally made publicly accessible. Without access to the git repository, a git commit hash is meaningless to an attacker. By contrast a list of dependencies and their versions very quickly lets an attacker know where to focus their efforts to find an RCE.

An endpoint that displays more information directly was considered but we've seen very little demand for it. I suspect that's because the git endpoint is meeting people's needs. We can reconsider if that changes in the future for a significant number of users.

The point is, if the endpoint exists and is enabled then we can make things work better. Right now all we can do is make some wild guesses... essentially.... so it will never be accurate.

I realise I am repeating myself, but could you consider limiting the functionality to situations where the application has been launched in a way that does not require you to make wild guesses? Perhaps by setting the -Dspring.boot.project.name=<name-of-your-project-in-your-workspace> system property for apps that you launch (this may well already happen) and encourage users to set it themselves for apps that they launch.

@martinlippert said above that "this wouldn't help in your setting, I guess, where you have multiple workspaces open with the same projects, so you would have the same project names". It would, in fact, help in my setting as I had all of the Boot 2.2 projects in one workspace and all of the Initializr projects in another.

The problem I have right now is that I can either turn the feature off and lose it completely, or I can leave it on and live with its inaccuracies. I can increase the accuracy by using the spring.boot.project.name system property, but I'll still get inaccurate information for process where I have not set that property for whatever reason. Unfortunately that will leave me in a situation where I have to be suspicious of the information and double-check if it's likely to be accurate before using it. That friction is likely to leave me turning it off completely.

My ideal would be that, out-of-the-box, the information is only displayed when it is accurate, with users being guided as to how they can turn it on for processes not started by the Tools. The next best thing would be a configuration option that lets me opt in to the information being accurate by requiring the system property.

[wilkinsona]

We tried hard and evaluated various ways to match the found processes to projects in your workspace. We tried to use the -classpath and -jar settings of the process to match that against the projects output folders in your IDE, but that breaks for processes launched with the thin jar launcher or via the maven build. So that works for some cases, not for others.

The thin launcher is still very much a niche so I think it could be ignored and we'd still end up with something that works for the vast majority of Spring Boot users.

For apps launched with java -jar or with -classpath (bootRun with Gradle or spring-boot:run with Maven, for example), I wonder if you could look for a compiled class annotated with @SpringBootApplication and use this to tie the process back to a project in the IDE? This might take a little while depending on the size of the application's classpath, but hopefully it could be done in the background and only once per process. A shortcut for that search in some cases would be to look for a MANIFEST.MF and check the Start-Class attribute.

[philwebb]

I wonder if we've got two distinct use-cases and if we separated them it might make things easier. I feel like this is similar to "debug" and "remote debug". When I launch an application from the IDE it would make sense to directly support the hovers. For an existing running application, perhaps I should actively attach to a process. After all, it's perfectly possible that the exact same application could be running more than once.

[kdvolder]

Thanks for all the suggestions. Please forgive me for not responding to all the different comments right now. A lot of food for thought there but it will take some time to discuss with the team and digest.

However, let me just pick up one comment that peeked my interest as a potential solution to explore.

No, not at all. We have the info endpoint and git.properties combined with the app's git repository for this use case.

That sounds interesting, how does it work exactly? Would it be possible for STS to access an actuator endpoint and find out the exact commit hash for the code that is running there? If so, I think that may have potential because commit hashes might exactly be the kind of 'unique id' that we could use to match up processes with the contents of source code in the workspace (if the source code is in the workspace, we should be able to find out the commit hash!)

It is actually this sort of solution I had in mind. An endpoint that serves up some kind of 'unique id' for the source code / version that we can match with the code in the user's workspace. Maybe the commit hash would work. I would still consider that a bit of 'hack' and it would only work if the code is in git. So I would still prefer we can work together constructively on getting something that is explicitly designed and added to actuator for the purpose we intend to use if for.

[philwebb]

Unfortunately I think trying to use the git.properties will be too brittle. The properties gets written by a Maven or Gradle plugin so the user needs to actively add opt-in to that. Furthermore, it's possible to customize the format so I don't think the IDE can assume anything about the result that will be returned.

You can real about more about the feature here.

[kdvolder]

Okay yes, it sounds like the git.properties isn't going to work. Should I raise a ticket with boot/actuator about adding a more purposefully designed endpoint then, or is this just a non-starter?

I am afraid though that without some kind of help from the process there aren't really a lot of good options to deal with this better than we already are.

Other thought... maybe if whatever mechanic we use (the git properties or something else) to 'make this better' is 'opt in' that is actually okay. I.e. if the 'info' we expect is there.... then we use it to implement a more precise matching. And if it isn't there then we could just fallback on the inaccurate matching we are doing now.

That way people like Andy who complain (legitimately!) that the matching is not accurate enough to their tastes can just opt in to provide the right info on the info endpoint and thus enable more accurate process matching support).

[wilkinsona]

Should I raise a ticket with boot/actuator about adding a more purposefully designed endpoint then, or is this just a non-starter?

An Actuator endpoint is a non-starter. Actuator is designed to provide production-ready features so it isn't the right place for development-specific features. We could add something elsewhere, though.

I am afraid though that without some kind of help from the process there aren't really a lot of good options to deal with this better than we already are.

IMO, things would be better if the information was accurate by default, even if that meant there was (much) less of it. The feature's being undermined by its inaccuracy at the moment which is really hurting its usefulness.

then we use it to implement a more precise matching. And if it isn't there then we could just fallback on the inaccurate matching we are doing now.

Repeating myself, but I really think you should fall back to not showing anything.

can just opt in to provide the right info on the info endpoint and thus enable more accurate process matching support).

I think this should be the other way round to. I'd be very happy if it works as Phil has described above:

I feel like this is similar to "debug" and "remote debug". When I launch an application from the IDE it would make sense to directly support the hovers. For an existing running application, perhaps I should actively attach to a process. After all, it's perfectly possible that the exact same application could be running more than once.

An alternative to actively attaching to the process would be to document the setting(s) that are needed to allow STS to attach to the process without the user doing anything. These would be the same settings as STS sets when it launches the process. The key thing would be that information is only shown for processes where the accuracy can be guaranteed.

[kdvolder]

IMO, things would be better if the information was accurate by default, even if that meant there was (much) less of it. The feature's being undermined by its inaccuracy at the moment which is really hurting its usefulness.

We've been over this and understand your point of view you don't have to keep repeating it.

Repeating myself, but I really think you should fall back to not showing anything.

We should discuss this with the team, but I think there are some use-cases where that literally means the feature will be very hard for a user to get working, if it is even possible at all. These are use-cases we care enough about that it really isn't clear that this is a good idea. On top of that, it would also be a drastic departure of how things work now and would entail big/complex changes into our code base all over the place.

So please, stop repeating the argument and please just trust that we have already heard your point of view, we understand it and we actually consider it a valid point. In the end though we will have to do what we think is best all things considered.

And yes, some kind of 'active attach' to local processes is also a great suggestion and possible solution we should consider. This is also not trivial to implement/design however especially considering that it would probably need separate implementations for each IDE environment that uses the language server.

Anyhow, I thank you all for the provided suggestions. But we cannot reach a final decision on this right now and so probably its not that useful to continue repeating the same arguments again.

[wilkinsona]

Thank you for acknowledging the point. It was impossible to know if it had been made clearly when all we had to go on was the following:

Thanks for all the suggestions. Please forgive me for not responding to all the different comments right now. A lot of food for thought there but it will take some time to discuss with the team and digest.

[kdvolder]

Fair enough, and thanks for being so passionate about making sure we understand what you are saying :-)

[martinlippert]

The overall way we connect the IDE to running Spring Boot applications has changed already a while ago and adopted a more of an manual "attach to running process" way of doing things. Therefore I think the main issue that was discussed here has been resolved.

The "attach the IDE to a running process" got implemented for Eclipse as well as VSCode and is already documented in the user guide wiki section here: https://github.com/spring-projects/sts4/wiki/Live-Application-Information#managing-live-data-connections

If you still observe any strange behavior around this, please raise a new issue here and we can work from there.

Thanks again for raising this and for the detailed explanations and thoughts, much appreciated.