-
Notifications
You must be signed in to change notification settings - Fork 41.1k
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
Deprecate @ServletEndpoint, @ControllerEndpoint and @RestControllerEndpoint #31768
Comments
We have some endpoints where we wanted to be able to include app and/or actuator links in the response. The low friction way we found was to move to Or if the idea of those links is an anathema to the endpoint abstraction entirely... they are strange endpoints I won't defend in general so no objections to deprecation, just a data point... |
We have quite a few Main use cases that I think would be useful and helpful to support going forward would be:
I think most of these could likely be converted and written to conform to the new way of doing this.
It might be doable to do this today, but I wasn't able to find an alternative. |
Thank you, @mmoayyed.
In Actuator endpoint terms, Can you expand a bit on the need for
We don't support this at the moment. Any body is read up front as a source of arguments for the endpoint operation's method parameters. There's an implicit assumption here that the body can be converted into key-value pairs. |
Thank you Andy. The main example that I found for
Likewise, the reverse of this operation is handled as an |
You can return a Lines 53 to 54 in 35c49af
I guess a missing piece is the ability to set a
On first impressions, this doesn't sound like something that you'd do as part of operating an app that's running in production so it isn't necessarily something that we'd expect to support in Actuator. Can you expand a bit on the use case please? |
Sure. I'd have to go back a few steps, so please bear with me. This project ships a number auto-starters that control the implementation of a particular repository type. The ultimate build script (put together by an adopter in form of a overlay), allows an adopter to include the appropriate starter in their build and let that repository manage their data. I suppose there are scenarios where one would want to take data from one repository in one version, and move it over to another repository in the next version. Let's say JSON to Cassandra, Mongo to DynamoDb, etc. So the import/export operations provide a facility to do that as a point of convenience, behind an actuator endpoint that of course needs to be enabled, protected, used likely once or twice in dev, and then turned off in production as you note. Certainly, it's not something to actively use in production. I suppose one could always use native tooling, if and when available, to handle the data migration task, or fall back onto dedicated libraries and frameworks that take care of such things. While all should be better options in theory, (and surely there is nothing stopping the advanced user to play around with such things) they add additional points of complexity and learning curve that in this particular context, a very select group of users would want to deal with. The overwhelming preference generally is to deal with an endpoint that handles such things (IIRC, there is a command-line utility available that does this as well in form a spring shell command, but I digress). |
We currently use the management endpoints as a means of providing an internal API that is served on a different port. These annotations allow us to actually provide dedicated endpoints on the management port as well as handling security on these differently. This way we can provide an Kubernetes ingress for the public API on one port while providing management actions an internal API without ingress on another. |
Thanks, @frederic-kneier. Would it be fair to say that you're using an Actuator endpoint as a convenient way of exposing the API on a different port rather than because it's used as part of operating the application in production? |
Not entirely, but regarding |
We have a few in spring cloud. I'll get details in a bit |
One other thing that might be useful here: |
We have discussed this again earlier today and decided that we should deprecate this for 3.3.0-RC1. We don't plan on removing this feature soon - we will honor at least our 2 minor releases policy. In the meantime, we will also gather feedback from the community. |
We had a look before scheduling this, and it seems this endpoint would work well with |
We have the same issue.
I can also help providing a minimal app for that but there's nothing fancy there, just: @RestControllerEndpoint (id = "customer")
@Component
public class CustomerEndpoint {
@PostMapping ("/{id}")
public Customer createCustomer(final @PathVariable (value = "id") Integer id, final @Valid @RequestBody Customer customer)
{
return customer;
}
public static record Customer(String name, Address address){}
public static record Address(String street){}
} Now you can POST to /actuator/customer/1 with { "name": "cust", "address": {"street": "1"}} And placing a get to /actuator, it returns: "customer": {
"href": "http://localhost:8081/actuator/customer",
"templated": false
} There's a limitation where the |
@philwebb Any news on this. This is a blocker on our side as well :-( |
Sorry I was otherwise engaged for a couple of weeks. Do you need a runnable application or is code of endpoint and request body sufficient? |
@kschlesselmann We've not had a chance to discuss it again yet, but the issue is flagged so we won't forget. @rainerfrey-inxmail Ideally something that we can run so we can investigate alternative API ideas. |
@philwebb Maybe this issue should be re-opened to show that there is an active discussion (an a pending one as well) here? |
Thanks for the suggestion. This issue was tracking the deprecation which has been done and shipped in 3.3.0-RC1. It doesn't make sense to have an open issue for work that has been completed so it won't be re-opened. Please rest assured that the feedback that people have provided is still very much on our radar and it will all be considered before anything is actually removed. |
Thanks @wilkinsona Using RestControllerEndpoint solved the problem of creating a custom endpoint with context type as event-stream., Since we are deprecating these annotationsit from 3.3.0 and removing these annotations ( |
We may consider it as a future enhancement to the |
We were actually in the midst of constructing a custom actuator endpoint since we have many circuit breakers and we wanted to send our circuit breaker events to the developer management portal managing bby us. Although we can accomplish the same thing with a controller or rest controller, this isn't what I presume it's meant for. Given that this is management configuration data, it made sense to expose it on the actuator endpoint. |
We had the same use case as @talasila-sairam before when using Hystrix and Turbine. |
Hi we use Rest Actuator for some tech operation at work. We do not use jmx exposure, so we use all the flexibility a REST controller could provide such as this kind of actuator:
Now, RestControllerEndpoint is deprecated, but I didn't find any equivalent migration path in spring documentation. Of course, we can convert this actuator to a RestController, but we'll lose actuator built-in configuration, such as exposure properties: |
At present, we still need to pass the body parameter, but @WriteOperation does not support it. I don't know how to migrate either.It seems that abandoning @RestControlEndpoint did not have sufficient consideration now |
@zxuanhong can you please be more specific? A concrete example of what you're doing with |
Can we do complex objects for The object itself if here https://github.com/spring-cloud/spring-cloud-gateway/blob/main/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/route/RouteDefinition.java#L41 |
I suspect we may have overlooked the inherited post mappings. Apologies. |
@wilkinsona For example, how to migrate @RestControlEndpoint to @WebEndpoint (we need to pass the body parameter).There are over 20 fields in ChangeInfo. I don't want to use query parameters
|
We also use |
Chaos Monkey for Spring Boot also uses complex objects in actuator endpoints via Details about our current actuator endpoints and what they are used for can be found in our documentation or our code: |
We got consistent feedback here about binding complex types on custom actuator web endpoints. We also understand that some developers use the full potential and flexibility of MVC web endpoints. I think that for the second point, we're not likely to walk back the deprecation notice we declared in 3.3.0. The flexibility and possibilities are just too broad and this currently prevents us from improving things in this part of Spring Boot. Maybe there is a way to solve the problem from the other end: offering a way to easily bind web endpoints on the actuator path, completely separate of the Actuator infrastructure? To be clear, I didn't explore this possibility myself yet. Now the main point of my comment is the following: I have a draft change that could help with the binding of complex types on actuator web endpoints. I wrote a short section as reference documentation explaining how this would work: If you are interested in this particular aspect, would this feature help with your use case and moving off of Thanks! |
As indicated by linked issues from Jolokia and Hawtio, we were benefiting from powerful methods of registering additional actuator endpoints (Jolokia is using ServletEndpoint and Hawtio is using ControllerEndpoint). Technically, both Jolokia and Hawtio are full web applications (servlet based) and maybe it wasn't a good idea to expose these ( Spring Boot itself dropped Jolokia actuator endpoint with #28704 due to lack of Servlet API 5 support (which was added by Jolokia 2.0) and justifiably not brought it back in #37568. But with this issue we can't simply provide servlet-based actuator endpoint... |
@bclozel This would solve the issue regarding complex objects, thank you. However, it will not be possible to expose multiple nested paths via one actuator endpoint like we do now. We'd have to configure multiple actuator endpoints (with their own ID respectively) instead. This would also mean that users who want to use these endpoints (or not use them) would have to change their configuration accordingly and enable/disable multiple actuator endpoints instead of just one. Definitely an improvement to how it is not but still not ideal for our use case. |
@denniseffing I received additional feedback about this and yeah, complex nested objects with full deserialization support is the expectation here from developers. Unortunately deserialization already happened at that point in the actuator support. I'll try and explore other solutions but so far they're all pointing to the fact that "full" MVC endpoints support is asking too much and prevents much needed improvements in this area. |
For the record, In Jolokia (jolokia/jolokia@6902490) I moved from deprecated |
We should decide if we want to deprecate and later remove
@ControllerEndpoint
and@RestControllerEndpoint
from the actuator. Using them ties the user to WebMVC or WebFlux and they were meant to ease the upgrade path to the weblayer-abstracting@Endpoint
with@ReadOperation
, etc.Getting rid of those would pave the way for #20290.
If you're seeing this ticket and object to this idea, please comment, your feedback is very valuable. Please also explain your use case and why this use case can't be solved with
@Endpoint
or@WebEndpoint
.The text was updated successfully, but these errors were encountered: