Skip to content

Conversation

Raheela1024
Copy link
Contributor

No description provided.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 4, 2019
@wilkinsona wilkinsona added this to the 2.2.x milestone Feb 7, 2019
@wilkinsona wilkinsona added type: enhancement A general enhancement theme: performance Issues related to general performance and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 7, 2019
@wilkinsona wilkinsona changed the title Consider making RestTemplateMetricsConfiguration conditional on a RestTemplateBuilder bean Make RestTemplateMetricsConfiguration conditional on a RestTemplateBuilder bean Feb 7, 2019
@wilkinsona
Copy link
Member

wilkinsona commented Feb 11, 2019

Thanks very much for the PR, @Raheela1024. The first commit (7beca9e) is great. I'm not 100% sure that we need the changes in the other two commits (other than adding you as an @author) but I could easily be missing something. Can you please fill in the blanks for me and explain why they're needed? If they're not strictly needed then I think it would be better to do without them (for now at least) as we try to keep changes to the minimum that's needed to fix and test a particular issue.

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Feb 11, 2019
@Raheela1024
Copy link
Contributor Author

Raheela1024 commented Feb 11, 2019

Thanks very much for the PR, @Raheela1024. The first commit (7beca9e) is great. I'm not 100% sure that we need the changes in the other two commits (other than adding you as an @author) but I could easily be missing something. Can you please fill in the blanks for me and explain why they're needed? If they're not strictly needed then I think it would be better to do without them (for now at least) as we try to keep changes to the minimum that's needed to fix and test a particular issue.

Actually, In first commit i just added test case for added annotation but while running whole test case file getting below exception in all test cases instead of my added test cases.

io.micrometer.core.instrument.search.MeterNotFoundException: Unable to find a meter that matches all the requirements at once. Here's what was found:
   FAIL: No meter with name 'http.client.requests' was found.
   FAIL: No meters have the required tag 'uri'.

While debugging the issue i got to know in InterceptingHttpAccessor ClientHttpRequestInterceptor is not initiated but when i remove @ConditionalOnBean(RestTemplateBuilder.class) from RestTemplateMetricsConfiguration ClientHttpRequestInterceptor is initiated and test cases are working fine.

In test cases we have below code that is update the http.client.requests meter size into MeterRegistry.

		for (int i = 0; i < 3; i++) {
			restTemplate.getForObject("/test/" + i, String.class);
		}

May b i fixed in wrong way please let me know if there is any other way?

@wilkinsona
Copy link
Member

Thanks very much for the explanation, @Raheela1024.

I think the problem is that HttpClientMetricsAutoConfiguration is being processed before RestTemplateAutoConfiguration. As a result, when RestTemplateMetricsConfiguration is conditional on a RestTemplateBuilder bean, it backs off as there isn't one at that time. If I'm right, the problem can be fixed by adding RestTemplateAutoConfiguration.class to the existing @AutoConfigureAfter on HttpClientMetricsAutoConfiguration.

@Raheela1024
Copy link
Contributor Author

Thanks very much @wilkinsona you solved my Problem. Thanks again kindly review and proceed.

wilkinsona added a commit that referenced this pull request Feb 12, 2019
* gh-15842:
  Polish "Make auto-config of RestTemplate metrics back off with no builder bean"
  Make auto-config of RestTemplate metrics back off with no builder bean

Closes gh-15842
@wilkinsona
Copy link
Member

Thanks very much, @Raheela1024. The proposed changes have now been merged into master. If you are interested, I polished things a little bit in 22192c2. The biggest thing, and it was still really minor, was to move the new test method up next to the others rather than it being at the end of the class after the helper methods.

@wilkinsona wilkinsona modified the milestones: 2.2.x, 2.2.0.M1 Feb 12, 2019
@wilkinsona wilkinsona removed the status: waiting-for-feedback We need additional information before we can continue label Mar 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: performance Issues related to general performance type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants