-
Notifications
You must be signed in to change notification settings - Fork 40.8k
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 |
It looks to me like your |
@wilkinsona I'm using a different port so I guess I should try the |
Unfortunately I only found this ticket after the 3.3 release. Our reason for using |
@rainerfrey-inxmail Are you able to share a sample application that shows one of your more complicated inputs? It doesn't need to be your production code, just something that gives us an idea of how you're currently using |
@johanhaleby We are currently having the same issue did you get it to work using the |
@IfvJonas I haven't tried yet unfortunately. |
@philwebb I second @rainerfrey-inxmail use case. We're synchronizing data between our environments and use actuator endpoints for that. Right now we have an endpoint like @PostMapping
fun synchronize(@RequestBody model: SynchronizationRequestModel): Mono<ResponseEntity<Void>> =
… which basically accepts a list of entities that should be synced. It seems that this is no longer possible 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 |
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: