Add endpoint support for web-specific stack

[spencergibb]

In spring cloud there are numerous MvcEndpoints that took advantage of the ability of SpringMVC to add arbitrarily complex behaviors to the actuator such as:

• complex POST parameters (maps & objects)
• wrapping a servlet (/hystrix.stream), basically access to raw HttpServletRequest & HttpServletResponse

The benefit of having them in actuator is that they aren't part of the main application and can be protected by actuator security.

If we move them to normal endpoints we would lose all of the benefits of actuator (global disabled, security, management port, etc...) that are users are used to.

Thoughts?

[snicoll]

The end goal of the endpoint infrastructure in 2.0 is not to replicate what Spring MVC can do (imagine the work to abstract that to webflux and Jersey). If you have those needs you need to keep your MVC controllers as they are today.

We still need to work on letting users define whatever they like using a traditional controller (and with that limitation in mind). Several things:

• If you register your controller in a @ManagementContextConfiguration it will be part of the actuator application context (if one). This isn't new and documented
• We have @ConditionalOnEnabledEndpoint that currently only works when set on an endpoint or an extension. We probably need to relax that a bit for that scenario

I am happy to discuss alternative routes but your endpoint needs to be an endpoint in the first place. I would argue that the second example is not an endpoint: where is the JMX feature with a method that takes servlet and response? How do you pass those complex map and objects with JMX for the first case?

On our side, we did move Jolokia because we realized it wasn't an endpoint. I believe you should do the same.

[wilkinsona]

For the security aspect of this, we now have the same problem with our Jolokia support. Whatever is figured out for that will, hopefully, be relevant to Spring Cloud as well.

[spencergibb]

Regarding @ManagementContextConfiguration it puts it in the management application context, but not the web (/application) context. Currently, that has to be done by hand.

[wilkinsona]

This doesn't appear to be moving much. @spencergibb Have you reviewed your current endpoints to determine which of them aren't really endpoints? What, if anything, do you still need to help you to make the transition?

[spencergibb]

There are a few. In some cases I changed ids from /bus/refresh to /bus-refresh. Other cases we've simplified (removed complex input). Some cases I moved request parameters to a path parameter. The hystrix servlet, I copied the jolokia management auto-configuration. I think we're ok writing mvc controllers, etc... where it doesn't fit. The one thing missing is disabling the non @Endpoints when all endpoints are disabled.

[wilkinsona]

The one thing missing is disabling the non @endpoints when all endpoints are disabled.

I don’t think we’ll do that. IMO, if something isn’t an endpoint it would be counter-intuitive for disabling all endpoints to disable it.

[snicoll]

At the last team meeting, we discussed the possibility to add a new kind of Endpoint. Recent work that we're going to ship in M7 brings the concept of @JmxEndpoint and @WebEndpoint that are a specialization of an @Endpoint: they are meta-annotated with it and define a filter that determines if that particular endpoint has to be exposed on an environment.

We discussed the opportunity to introduce a @MvcEndpoint that would be a regular @Controller (as it is the case in 1.x) for those corner cases where the underlying technology is required. Similar @WebFluxEndpoint or @JerseyEndpoint could be envisioned for those who want to offer the full support without using the API.

The required features would be:

1. Reserve an id for the feature so that a regular Endpoint can't "steal it"
2. Offer a way to enable/disable the endpoint using the same infrastructure (management.endpoint.<xyz>.enabled)
3. Automatically find and deploy the controller on the chosen management.endpoints.web.base-path
4. Make sure the security infrastructure is compatible (i.e. because it is an endpoint, we can secure it like any other endpoint)

From a user perspective, this feature makes sense: a user doesn't care if the implementation has to inject HttpServletRequet or whatever fine-grained feature the underlying tech provides. The proof of that is our Jolokia support that is deployed in the same area but is a raw Servlet. Anybody can create a @Endpoint(id = "jolokia") and potentially shadowing it (or vice versa).

Having said that, from our perspective, we would allow to define something as an Endpoint that isn't one:

1. Most probably the endpoint will have zero Operations
2. Because of 1., the caching feature would feel awkward: we provide a cache property automatically but it has no effect

That last point could be fixed by moving the cache configuration at management.endpoints.cache like we did for web path customization. The annotation processor would only generate an enabled flag and we should not be able to add any extra property to that namespace then. We need to figure out if we are ready to pay that price.

[dsyer]

Probably @MvcEndpoint needs to be augmented (or merged) with @WebfluxEndpoint? Anyone who adds an MVC endpoint (and wants it to be managed with enable flag, prefix, security etc. along with the other endpoints) will probably do the same for WebFlux. It might even be the same bean, given the overlap of the annotation programming model. Certainly we would want to do this with the endpoints in Spring Cloud.

[snicoll]

@philwebb the remark in my previous comment has not been fully addressed, specifically:

The caching feature would feel awkward: we provide a cache property automatically but it has no effect. That could be fixed by moving the cache configuration at management.endpoints.cache like we did for web path customization. The annotation processor would only generate an enabled flag and we should not be able to add any extra property to that namespace then. We need to figure out if we are ready to pay that price.

I've just tried with latest master and that property is generated. Is that deliberate or is it an oversight?

[philwebb]

@snicoll I missed that comment, rather than re-open this one I've raised #11703. We can certainly live with the extra meta-data in RC1.