[BUG][Java][spring][webclient] webclient metrics contains actual query parameters, resulting in OOM

[cdmatta]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• Have you tested with the latest master to confirm the issue still exists?
• Have you searched for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

Generate the spring webclient implementation for an open api endpoint which is using query parameters.
Use the generated ApiClient to make calls to the endpoint with different query parameters in a spring webflux application (with micrometer metrics).

Check the uri tag for the metric http_client_requests.
This uri tag contains the query parameter. This will continue to grow as different values are used.
Eventually, leading to an OutOfMemory error.

openapi-generator version

openapi-generator version: 4.3.1

This is not a regression.

OpenAPI declaration file content or url

This can be reproduced with the sample petstore fake endpoints yaml file in the project

/fake/body-with-query-params

Steps to reproduce

1. Generate the spring webclient implementation for the yaml
2. Use it in a Spring boot project with actuator and metrics
3. Call the endpoint with query parameters

Actual

• Query parameter values are present in the http_client_requests metric
  

Expected

• Query parameters values must not be present in the metrics.

Suggest a fix

Solution for this is available in spring framework 5.2 webclient

I could provide a PR, but that means the minimum required version of spring framework will be 5.2.
Or, we need to make rules in the template to add the following code on spring 5.2+ only, hence leaving it for you to decide.

Here is the fix to the ApiClient.mustache template required to fix this. webclient-ApiClient.patch.txt

diff --git a/modules/openapi-generator/src/main/resources/Java/libraries/webclient/ApiClient.mustache b/modules/openapi-generator/src/main/resources/Java/libraries/webclient/ApiClient.mustache
index 98f4bfb2..080b6fbd 100644
--- a/modules/openapi-generator/src/main/resources/Java/libraries/webclient/ApiClient.mustache
+++ b/modules/openapi-generator/src/main/resources/Java/libraries/webclient/ApiClient.mustache
@@ -564,11 +564,13 @@ public class ApiClient{{#java8}} extends JavaTimeFormatter{{/java8}} {
         updateParamsForAuth(authNames, queryParams, headerParams, cookieParams);
 
         final UriComponentsBuilder builder = UriComponentsBuilder.fromHttpUrl(basePath).path(path);
-        if (queryParams != null) {
-            builder.queryParams(queryParams);
-        }
 
-        final WebClient.RequestBodySpec requestBuilder = webClient.method(method).uri(builder.build(false).toUriString(), pathParams);
+        final WebClient.RequestBodySpec requestBuilder = webClient.method(method).uri(builder.build(false).toUriString(), uriBuilder -> {
+            if (queryParams == null) {
+                return uriBuilder.build(pathParams);
+            }
+            return uriBuilder.queryParams(queryParams).build(pathParams);
+        });
         if(accept != null) {
             requestBuilder.accept(accept.toArray(new MediaType[accept.size()]));
         }

[auto-labeler]

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

[admoca60]

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

Maybe the right label should be "Client: Spring" because this affect to the client. The client can be used in a Server (with Spring obviously) or in Batch process.

[admoca60]

Dear @cdmatta
You have suggested that the query parameters should be removed from uriTag, but in my opinion, that should't be so.
Maybe, the best approach could be create a uriTag with queryParams parametrized as path params.

For example, the path "/cars". That resource can be queried by one or two queryParams, the "color" or the "model" and the operation is different in each case.
Taking previous example, this is a relationshipt between the request and the uriTag proposed.

URI REQUEST -----------------------> URI TAG
/cars?color=black -----------------------> /cars?color={colorParamValue}
/cars?color=white -----------------------> /cars?color={colorParamValue}
/cars?color=blue -----------------------> /cars?color={colorParamValue}
/cars?model=ford ----------------------->/cars?model={modelParamValue}
/cars?model=seat&color=yellow ----------------------->/cars?model={modelParamValue}&color={colorParamValue}

The first three request go to the same "uriTag".

What do you think about it?

[cdmatta]

Definitely, having the query parameter templatized for metrics would be a good thing.

In my experience of using spring webclient + metrics, I only had 2 options

1. Drop query parameters from metrics, like the example patch above.
2. Follow the below mechanism to get the templatized url.

The only way to get the url template with query parameters in metrics was, to use this api call, setting up everything by hand.

webclient.method(method).uri("/cars/{id}?feature={featureValue}", "toyota", "automatic")

As you can see above, this requires custom parameters for the api call. Given that the api can have more than a few parameters + can also contain array parameters (which I have not used, thus cannot comment on what is needed for its support), one can see how it quickly becomes unwieldy in the generator

The generator template had a single entry point for the webclient call.

This means the generator need to generate code for each such combination of query parameters accounting for these challenges.

[admoca60]

This means the generator need to generate code for each such combination of query parameters accounting for these challenges.

Yes, I'm aware of that, but it could be easy to do. In runtime, the ApiClient can generate the queryParams templatized string and append it to base url. The generator has the parameter "MultiValueMap<String, String> queryParams" which can be used to build this "queryParams uri templatized string". In fact, is the same behaviour that Spring does when we use "builder.queryParams(queryParams);"

What do you think?

[cdmatta]

Sure, generating the templatized url is not the challenge IMHO.

Passing the query parameters is. Last I tried the builder did not lend itself to supporting query parameters with templates.

The varargs mechanism in my previous comment was the only way I got the metrics to work correctly for query parameters with uri templates. Either standalone or inside uriBuilder

webclient.method(method).uri("/cars/{id}?feature={featureValue}", "toyota", "automatic")
// OR
webclient.method(method).uri("/cars/{id}?feature={featureValue}", builder -> builder.build("toyota", "automatic"))

[admoca60]

Did you test with this?

webclient.method(method).uri("/cars/{id}?feature={featureValue}", uriMapParameters)

We can include the pathParams and queryParams in the same map Map<String, Object> pathParams, so in that case the code could do that:

   final UriComponentsBuilder builder = UriComponentsBuilder.fromHttpUrl(basePath).path(path);
      String finalUri = builder.build(false).toUriString();
      Map<String,Object> uriParams = new HashMap();
      uriParams.putAll(pathParams);
      
       if (queryParams != null) {
           //Include queryParams in uriParams taking into account the paramName
          //Append to finalUri the templatized query string like "?param1={param1Value}&.......
        }

        final WebClient.RequestBodySpec requestBuilder = webClient.method(method).uri(finalUri, uriParams );

[cdmatta]

Oh forgot that one. Yes, that works. Indeed then it is fairly easy to get that working in the generator.

[admoca60]

I'm realised that for RestTemplate happends the same: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/Java/libraries/resttemplate/ApiClient.mustache#L606

But in that case the approach to fix it is sligtly different. In the custom api client this line is executed before Invoke method in ApiClient, so the right solution here should be generate the templatized path (with queryParams) before that line. In addition of that, the invokeMethod parameters should also change to exclude "queryParams" because they would be included in the global "uriParams" map.

[admoca60]

Maybe, the case of RestTemplate should be in another different issue. What do you think @wing328, @bkabrda @cbornet ?

Can anybody generate a PR to solve this (webclient) fix?

[jorgerod]

Hi @admoca60 @cdmatta
I can contribute for fix this issue (I have it fixed but my colleague forgot to contribute)

[admoca60]

Go ahead!! Maybe your solution could be very familiar for me!!! xD