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

Disable JMX by default #16090

Closed
bclozel opened this issue Mar 4, 2019 · 9 comments

Comments

Projects
None yet
4 participants
@bclozel
Copy link
Member

commented Mar 4, 2019

This feature doesn't seem to be widely used and takes a significant amount of resources so we're reconsidering this default for the 2.2 line.

This option is not only about switching spring.jmx.enabled, but also about checking that the other features relying on that option don't fail as a result.

@bclozel bclozel added this to the 2.2.x milestone Mar 4, 2019

@snicoll

This comment has been minimized.

Copy link
Member

commented Mar 4, 2019

IDEs support comes to mind but if the user decides to go with the default and not have that info, I think that's fine.

@bclozel bclozel self-assigned this Mar 4, 2019

@bclozel bclozel changed the title Disable JMX-related options by default in Actuator Disable JMX by default Mar 4, 2019

@snicoll

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

IntelliJ IDEA gracefully handles the fact beans are not there but the error message could be improved a bit. They also have a "Enable JMX agent" option that could be used to set the property. I'll reach out to the team.

@martinlippert

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

The Spring Tools 4 feature to show live hovers for boot apps heavily relies on this JMX actuator endpoints being available and enabled - and having them enabled by default makes those live hovers appear "magically", which is a quite nice user experience. So to be honest, I am not a big fan of flipping the default to disabled.

Of course devs could still enable that, but it would turn our live hover IDE feature much harder to discover.

@wilkinsona

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

Unfortunately, slowing down startup for everyone isn't a nice user experience either. Given that IDE tooling has the means to enable JMX (Spring Tools also has an Enable JMX option), I believe that disabling JMX by default and allowing those that want to use it to enable it is the right choice. It means that only those that want to use JMX pay the cost of using it.

@martinlippert

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

I see your point. But looking from the tooling perspective, it looks different. I need to mention that enabling this in the IDEs might not be as easy as it sounds, just to get back to the point where we are at the moment where this feature of the tooling just works out-of-the-box as soon as you have the actuators on your project. Of course users could click the checkbox to enable it (in Spring Tools 4 for Eclipse), add that option to the launch config (VSCode, Theia, etc.), or put that in the environment variables (for their apps on CF), but it all causes these different extra steps - whereas at the moment all those settings just work out-of-the-box. In the end, just two different perspectives... :-)

Looks like it boils down to the question: save startup time by default for X people/apps (who don't need JMX access) vs. save developer time for extra config work for Y people/apps (who need JMX access), right?

Do you have any numbers how many people (or apps) using the actuator endpoints need / don't need them via JMX?

@wilkinsona

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

It's not just about Actuator. Enabling JMX by default has a cost when Actuator isn't in the picture. We don't have any numbers specifically for JMX, but we can make some educated inferences from the data that we do have.

Judging by download numbers, we know that only ~33% of Boot's users use the Actuator. IMO, that alone is reason enough not to enable JMX by default just for the Actuator's benefit. When we factor in that not everyone will be using an IDE that relies on JMX, or won't be using the functionality in their IDE that relies on JMX, that 33% drops further. Anecdotally, when users are accessing Actuator themselves (rather than via some tooling), I see them almost exclusively using HTTP. I think that's to be expected given the relative ease of using curl, http, or your browser vs JConsole or the like. If more people were using JMX, I'd expect to see more questions about the JSON response format as it's rather atypical in that context. To my knowledge, there haven't been any such questions.

Spring Boot's model of enabling functionality when it's on the classpath has served us well and it continues to do so. However, it falls down a bit when something that should trigger some functionality being enabled is always on the classpath. JMX falls into that category. We have the spring.jmx.enabled property but we've now realised that we got its default value wrong. The cost of enabling JMX (startup time and memory usage) is too high for it to be paid by everyone by default, especially given that it's almost certainly a minority of users that use JMX.

Those that need JMX can easily enable it by setting spring.jmx.enabled=true and those that don't will get faster startup and lower memory usage. From a user's perspective, that feels like a win-win situation. I appreciate that this may create some work for tooling support. While that's not ideal, I don't think it's a reason not to flip the default. That work will be a one-off cost isolated to a few people, whereas the benefits will be enjoyed by a majority of all of Boot's users.

@martinlippert

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

I agree that JMX should not be enabled by default in the framework. I would argue that having JMX enabled if you have actuators on the classpath would make a lot of sense to me - if possible - and would serve our ease-of-use needs.

@wilkinsona

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

It's technically possible, but, other than DevTools, we don't have any other situation where adding a dependency changes the default value of a property. It would be confusing to make Actuator and spring.jmx.enabled an exception to that.

As I've tried to explain above, I don't think changing a default for everyone or everyone who uses Actuator can be justified purely to serve the ease-of-use needs of tooling. It's particularly hard to justify as tooling is a development-only concern yet the change in default would affect an app in production too.

We could consider enabling JMX when DevTools and Actuator are on the classpath, but I don't think that will remove the need for tooling to adapt to the change in default. Not everyone will be using DevTools after all.

@martinlippert

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

I will file an issue in our tracker to somehow deal with this upcoming change in Boot 2.2 then.

@bclozel bclozel modified the milestones: 2.2.x, 2.2.0.M1 Mar 5, 2019

@bclozel bclozel closed this in ce9626d Mar 5, 2019

bclozel added a commit to bclozel/spring-boot that referenced this issue Mar 6, 2019

Add @ConditionalOnExposedEndpoint condition
Prior to this commit, Actuator `Endpoint` instantiations would be
guarded by `@ConditionalOnEnabledEnpoint` condition annotations. This
feature saves resources as disabled endpoints aren't unnecessarily
instantiated.

By default, only `"health"` and "`info`" endpoints are exposed over the
web and all endpoints are exposed over JMX.

As of spring-projectsgh-16090, JMX is now disabled by default. This is an opportunity
to avoid instantiating endpoints if they won't be exposed at all, which
is more likely due to the exposure defaults.

This commit adds a new `@ConditionalOnExposedEndpoint` conditional
annotation that checks the `Environment` for configuration properties
under `"management.endpoints.web.exposure.*"` and
`"management.endpoints.jmx.exposure.*"`. In the case of JMX, an
additional check is perfomed, checking that JMX is enabled first.
The rules implemented in the condition itself are following the ones
described in `ExposeExcludePropertyEndpointFilter`.

See spring-projectsgh-16093

bclozel added a commit to bclozel/spring-boot that referenced this issue Mar 6, 2019

Add @ConditionalOnExposedEndpoint condition
Prior to this commit, Actuator `Endpoint` instantiations would be
guarded by `@ConditionalOnEnabledEnpoint` condition annotations. This
feature saves resources as disabled endpoints aren't unnecessarily
instantiated.

By default, only `"health"` and `"info"` endpoints are exposed over the
web and all endpoints are exposed over JMX.

As of spring-projectsgh-16090, JMX is now disabled by default. This is an opportunity
to avoid instantiating endpoints if they won't be exposed at all, which
is more likely due to the exposure defaults.

This commit adds a new `@ConditionalOnExposedEndpoint` conditional
annotation that checks the `Environment` for configuration properties
under `"management.endpoints.web.exposure.*"` and
`"management.endpoints.jmx.exposure.*"`. In the case of JMX, an
additional check is perfomed, checking that JMX is enabled first.
The rules implemented in the condition itself are following the ones
described in `ExposeExcludePropertyEndpointFilter`.

See spring-projectsgh-16093

bclozel added a commit that referenced this issue Mar 7, 2019

Add @ConditionalOnExposedEndpoint condition
Prior to this commit, Actuator `Endpoint` instantiations would be
guarded by `@ConditionalOnEnabledEnpoint` condition annotations. This
feature saves resources as disabled endpoints aren't unnecessarily
instantiated.

By default, only `"health"` and `"info"` endpoints are exposed over the
web and all endpoints are exposed over JMX.

As of gh-16090, JMX is now disabled by default. This is an opportunity
to avoid instantiating endpoints if they won't be exposed at all, which
is more likely due to the exposure defaults.

This commit adds a new `@ConditionalOnExposedEndpoint` conditional
annotation that checks the `Environment` for configuration properties
under `"management.endpoints.web.exposure.*"` and
`"management.endpoints.jmx.exposure.*"`. In the case of JMX, an
additional check is perfomed, checking that JMX is enabled first.
The rules implemented in the condition itself are following the ones
described in `ExposeExcludePropertyEndpointFilter`.

See gh-16093
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.