Comment From: wilkinsona
Thanks for the report. It's an interesting problem. I'm not sure that there's much we can do about it in Spring Boot alone as it would require detailed understanding of Spring Data REST's URI structure and knowing the likely cardinality of each parameter in the path. For example, you would want separate stats for /api/person and api/house, but you would not want separate stats for /api/house/1, /api/house/2, …, /api/house/12345.
We currently get the matching path pattern (/api/{repository}/ in this case) from a request attribute that's set by Spring MVC. Perhaps Spring Data REST could set an attribute of its own that's more detailed. Things like /api/house/, /api/person, api/house/{id}, and api/person/{id}.
What do you think @olivergierke and @mp911de?
Comment From: odrotbohm
The usage of $basePath/{repository} stems from the fact that this so far has been the most convenient way to register a controller that will eventually end up "multiple" mappings registered. Effectively it's just one and we determine the repository to be used at invocation time when resolving the handler mapping etc.
We also use the {repository} placeholder to resolve the repository in HandlerMethodArgumentResolver implementations based on the @RequestMapping annotation on the invoked method. I'd assume this get's more complicated if we switched to avoiding these annotations and rather add explicit mappings separately.
Another reason that moving off that model might not be as easy is that users can currently use the {repository} placeholder to override resources globally. I assume we'd break that functionality moving to explicit mappings.
I'll consult @rstoyanchev but I'm afraid this – if possible at all – will not be a quick fix. I wonder if in the meantime there's a hook in the metrics APIs that some component registered by the auto-configuration for Spring Data REST could use to create different buckets. I.e. something similar to what @high-stakes has in place but that can be more focussed using Spring Data REST internal metadata.
Comment From: wilkinsona
Thanks for taking a look, @olivergierke.
I wasn't proposing a changing to any of Spring Data REST's mappings. I was wondering if it would be possible if Spring Data REST could, during request handling, set a request attribute with a path pattern in it after it's resolved the repository. That pattern could contains a little more information than Spring MVC's as it could replace the {repository} placeholder with the concrete value for the repository that's been resolved. It could also, perhaps, add more components to the pattern for the various resources that each repository provides, just leaving in {id} and the like for high cardinality parts of the pattern.
Comment From: wilkinsona
To put that another way, I am proposing that this:
I wonder if in the meantime there's a hook in the metrics APIs that some component registered by the auto-configuration for Spring Data REST could use to create different buckets. I.e. something similar to what @high-stakes has in place but that can be more focussed using Spring Data REST internal metadata.
Could be done by Spring Data REST and the result placed in a request attribute.
Comment From: rstoyanchev
The usage of $basePath/{repository} stems from the fact that this so far has been the most convenient way to register a controller that will eventually end up "multiple" mappings registered.
Not sure if this helps or not but as of 5.1, it possible to assign a base path to a controller, through external configuration (via WebMvcConfigurer), see SPR-16336 and docs.
Also since 4.2 there has been a public API to register and unregister controller methods programmatically, see SPR-11541 and docs.
Comment From: odrotbohm
I've filed, fixed and back-ported DATAREST-1294 into Lovelace and Kay maintenance branches. We now expose a EFFECTIVE_REPOSITORY_LOOKUP_PATH request attribute to contain a partially resolved PathPattern that has the repository base resolved, i.e. one that will effectively produce different patterns per exposed repository.
As per @rstoyanchev's suggestion I also filed DATAREST-1295 to investigate if we can drop our own base path handling in favor of Spring MVCs.
Comment From: wilkinsona
Great stuff. Thank you, @olivergierke. We'll need to wait a little while to implement this as there's no release of the Spring Data release train planned before Boot 2.1.0.RELEASE.
Comment From: odrotbohm
If you'd be willing to upgrade to a bugfix release even after RC1 we can easily ship one in time.
Comment From: wilkinsona
Yeah, we'd be happy to do that. Bug fixes are fine post-RC. Would you be able to ship a Kay release train too? That would allow us to do this in 2.0.x at the same time. I think a change in 2.0.x is warranted as it's low-risk and it's a bit arguable whether the current behaviour could actually be considered as a bug due to the metrics being rather useless.
Comment From: odrotbohm
I'll schedule a Kay and Lovelace service release for Friday next week then. /cc @mp911de
Comment From: wilkinsona
I'll defer to @olivergierke on that one. Any change to support it would be in Spring Data REST so DATAREST-1294 is probably the best place to continue the discussion.