URI Is Being Generated With URI Variables Before RestTemplate Does It

[john-slater]

spring-integration/spring-integration-http/src/main/java/org/springframework/integration/http/outbound/AbstractHttpRequestExecutingMessageHandler.java

Line 301 in cf01b7c

private URI generateUri(Message<?> requestMessage) {

Spring Integration is generating the entire URI with the declared uri variables, and then passing the generated URI to the RestTemplate exchange method defined in HttpRequestExecutingMessageHandler.exchange()

Here is a truncated declaration of the outbound gateway with the LANGUAGE query parameter templated

<int-http:outbound-gateway
                url="/properties/availability?language={LANGUAGE}"                
                rest-template="restTemplate"
        >            
            <int-http:uri-variable name="LANGUAGE" expression="headers['LANGUAGE']"/>            
        </int-http:outbound-gateway>

This is causing issues in Micrometer which auto instruments the RestTemplate, and is creating a new Timer for every request as the path/query parameters differ. See micrometer-metrics/micrometer#98 for the issue.

From the above we would expect metrics to be recorded like

httpClientRequests{clientName="test",method="GET",status="200",uri="/properties/availability&language={LANGUAGE}",quantile="0.5"}

However what we are seeing is

httpClientRequests{clientName="test",method="GET",status="200",uri="/properties/availability&language=en-US",quantile="0.5"} httpClientRequests{clientName="test",method="GET",status="200",uri="/properties/availability&language=de-DE",quantile="0.5"} httpClientRequests{clientName="test",method="GET",status="200",uri="/properties/availability&language=ko-KR",quantile="0.5"} ........

I believe the correct behavior should be that the URI and the URI variables should be passed to the HttpRequestExecutingMessageHandler.exchange() method, so that RestTemplate can map the variables to the URI. This would correctly create the Micrometer timers for the RestTemplate

The call currently is

`httpResponse = this.restTemplate.exchange(uri, httpMethod, httpRequest,
(ParameterizedTypeReference<?>) expectedResponseType);

`

but it should be

httpResponse = this.restTemplate.exchange(uri, httpMethod, httpRequest, (ParameterizedTypeReference<?>) expectedResponseType, uriVariables);

[artembilan]

I believe there was the reason to populate variables into URI before calling a RestTemplate and that has been done long ago before Micrometer.
Plus even if we fix it here you can't make a guarantee that everybody else who's using RestTemplate is going to follow your requirements and they won't send enricher URL directly instead of relying on RestTemplate logic.

So, I would say that flaw is right now is exactly in the Micrometer and it is just not ready for all the use-cases in the field.

Fixing this issue for your blindly may lead to breaks in other projects.
Plus it doesn't look like we do something wrong, that's just a Micrometer omission that not everybody is going to use URI templating feature of the RestTemplate.

[john-slater]

Hi thanks for responding.

You have marked this as waiting for reporter but I'm not sure what feedback you are looking for.

This is currently causing an issue in our production system because of the amount of timers that are being generated by Micrometer. We have got around this by by setting

management.metrics.web.client.request.autotime.enabled=false

but this is not satisfactory as we have basically turned off this feature.

Micrometer is the defacto instrumentation library for Spring Boot. It auto instruments RestTemplate and this is it's default behavior. Spring Integration also utilizes RestTemplate for the http outbound gateway, but does so in away that is incompatible with the inbuilt Micrometer library. I've read in the Spring Batch release notes that Micrometer has also been integrated with that project too so is heavily linked with the Spring Eco system. I find it strange that core pieces of Spring (even if they are 3rd party libraries) don't work properly together.

We've only recently moved from writing pure Spring boot applications to using Spring Integration. We didn't have any problems with Micrometer instrumentation as we could call RestTemplate.exchange and let it handle the uri variables.

With Spring Integration we don't have this choice. It seems a strange decision to let Spring Integration perform this task, when RestTemplate already does it. Perhaps RestTemplate didn't always inject in uri variables, and when it did Spring Integration wasn't upgraded to take advantage of this.

You are correct when you say

Plus even if we fix it here you can't make a guarantee that everybody else who's using RestTemplate is going to follow your requirements and they won't send enricher URL directly instead of relying on RestTemplate logic.

this is the case today for RestTemplate if somebody decides to build the entire URL themselves, rather than use the URI Template. This is the same for Spring Integration, there is nothing stopping anybody from building up the entire URL instead of the uri-variables. But right now we are building up our URIs in the correct way using a URI Template, but we still can't get it to work.

I just looked into the source code for RestTemplate and I can't see the difference between what is happening in Spring Integration and why this functionality can't be left to the RestTemplate to do. They both expand a map of URI variables into the URL template.

https://github.com/spring-projects/spring-framework/blob/master/spring-web/src/main/java/org/springframework/web/client/RestTemplate.java

The only thing I can see that Spring Integration must do is check if uri variables have been defined in the integration flow and call

Map<String, ?> uriVariables = determineUriVariables(requestMessage);

As it has to generate the correct map of uriVariables from what was defined in the XML. Then it should be able to pass this map to the RestTemplate.

[artembilan]

Right. This is that kind of feedback I'd like to here from reporter.

It is really unfortunate that such a Micrometer functionality has not been tested against Spring Integration, but who would guess that we needed it.

As I said: there was a reason to supply variables into an URI before RestTemplate and no one expected a Micrometer to worry about that.
Plus I said that no one can guarantee how RestTemplate is going to be used, so that's not fair to always rely on the UIR templating from Micrometer.

I'm afraid that disabling that feature and writing your own Micrometer timer is going to be a right way to overcome the flaw in Micrometer.
You definitely can't claim that Spring Integration is wrong in here: we do nothing what causes bugs in HTTP protocol interaction, that's just matter of Micrometer that it doesn't count with slightly different RestTemplate use-case.

Of course, you can try to fix the issue here in Spring Integration, if you wish, but I'm afraid it is not going to be so easy. Because again: there was a reason to do that outside of RestTemplate

[john-slater]

Hi,

You keep stating that there was a reason why it was done outside of RestTemplate but you haven't said what that is. The only thing that Spring Integration should have to do with the uri variables is identify them and pass this map to RestTemplate.

The URLTemplate also works for path parameters e.g.

/bookings/{bookingid}

In the Spring Integration implementation it would be impossible for any metrics library to be able to deal with this by instrumenting the RestTemplate as by the time it reaches Spring Integration has already manipulated the URL to be

/bookings/12457 /bookings/12235

We need to be able to instrument different paths as /bookings/{bookingId} is not the same as /bookings/{bookingId}/rooms They might have different implementations with different levels of performance that we'd like to record.

How is anybody supposed to instrument this? Is there a plan for Spring Integration to add instrumentation for http outbound gateways?

If I was using Spring Boot itself I'd be able to implement this, by calling the appropriate restTemplate.exchange method. It's even documented in the official Spring Boot documentation https://docs.spring.io/spring-boot/docs/current/reference/html/production-ready-features.html#production-ready-metrics-http-clients.

Request’s URI template prior to variable substitution, if possible (for example, /api/person/{id})

In Spring Integration this isn't possible.

I've found somebody else's implementation that does as I've suggested. It doesn't look that difficult to implement, nor does it break what people would expect to happen.

https://github.com/iwein/spring-integration/blob/c4d31af848b6611debef455c5a67d0c7fbe23773/spring-integration-http/src/main/java/org/springframework/integration/http/HttpRequestExecutingMessageHandler.java#L210

protected Object handleRequestMessage(Message<?> requestMessage) {
  try {
			Map<String, Object> uriVariables = new HashMap<String, Object>();
			for (Map.Entry<String, Expression> entry : this.uriVariableExpressions.entrySet()) {
				Object value = entry.getValue().getValue(this.evaluationContext, requestMessage, String.class);
				uriVariables.put(entry.getKey(), value);
			}
			HttpEntity<?> httpRequest = this.generateHttpRequest(requestMessage);
			ResponseEntity<?> httpResponse = this.restTemplate.exchange(this.uri, this.httpMethod, httpRequest, this.expectedResponseType, uriVariables);

[artembilan]

@john-slater ,

thank you very much for your feedback!

The change I'm talking about was this: #755.

Indeed, previously, before encodeUri we had propagated uriVariables directly into a RestTemplate.

Now I see there is a DefaultUriBuilderFactory.EncodingMode to be supported for the RestTemplate:

/**
	 * Configure a strategy for expanding URI templates.
	 * <p>By default, {@link DefaultUriBuilderFactory} is used and for
	 * backwards compatibility, the encoding mode is set to
	 * {@link EncodingMode#URI_COMPONENT URI_COMPONENT}. As of 5.0.8, prefer
	 * using {@link EncodingMode#TEMPLATE_AND_VALUES TEMPLATE_AND_VALUES}.
	 * <p><strong>Note:</strong> in 5.0 the switch from
	 * {@link org.springframework.web.util.DefaultUriTemplateHandler
	 * DefaultUriTemplateHandler} (deprecated in 4.3), as the default to use, to
	 * {@link DefaultUriBuilderFactory} brings in a different default for the
	 * {@code parsePath} property (switching from false to true).
	 * @param handler the URI template handler to use
	 */
	public void setUriTemplateHandler(UriTemplateHandler handler) {

So, we really can remove a workaround in Spring Integration and fully rely on the RestTempalte configuration plus propagate uriVariables expansion into that RestTemplate.

I'm not sure that we can still treat this as a bug and try to fix it in the version 5.2.x, because it is going to be slightly breaking change, but we definitely can fix it a proper way for the current 5.3 generation.

Does it make sense?

Thank you!