Skip to content
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

Exception Metrics cause IllegalArgumentException when using custom ErrorDecoders #1466

Closed
DarkAtra opened this issue Jul 22, 2021 · 5 comments · Fixed by #1468
Closed

Exception Metrics cause IllegalArgumentException when using custom ErrorDecoders #1466

DarkAtra opened this issue Jul 22, 2021 · 5 comments · Fixed by #1468

Comments

@DarkAtra
Copy link
Contributor

Seems like the MeteredInvocationHandleFactory in feign-micrometer 11.5 tries to create both a timer and counter with the same name when using a custom error decoder that does not return a subclass of FeignException. This results in the following exception:

Caused by: java.lang.IllegalArgumentException: There is already a registered meter of a different type with the same name
	at io.micrometer.core.instrument.MeterRegistry.registerMeterIfNecessary(MeterRegistry.java:571)
	at io.micrometer.core.instrument.MeterRegistry.registerMeterIfNecessary(MeterRegistry.java:561)
	at io.micrometer.core.instrument.MeterRegistry.counter(MeterRegistry.java:284)
	at io.micrometer.core.instrument.Counter$Builder.register(Counter.java:128)
	at io.micrometer.core.instrument.MeterRegistry.counter(MeterRegistry.java:391)
	at feign.micrometer.MeteredInvocationHandleFactory.createExceptionCounter(MeteredInvocationHandleFactory.java:108)
	at feign.micrometer.MeteredInvocationHandleFactory.lambda$create$0(MeteredInvocationHandleFactory.java:85)
	at com.sun.proxy.$Proxy128.getById(Unknown Source)

I have created this repo to demonstrate the issue (refer to the tests).

@velo
Copy link
Member

velo commented Jul 22, 2021

Can you make that repo into a PR with a test? Thanks

@DarkAtra
Copy link
Contributor Author

do you have any preferences for the metric name? right now both are named feign.exception if i recall correctly

@velo
Copy link
Member

velo commented Jul 22, 2021

do you have any preferences for the metric name? right now both are named feign.exception if i recall correctly

If both express the same information, probably is safe to just eliminate one

@OlgaMaciaszek
Copy link

We've come across this issue while trying to upgrade Spring Cloud OpenFeign to Feign 11.5 so we're waiting for it to get solved for the upgrade.

velo added a commit that referenced this issue Jul 26, 2021
…1468)

* #1466: fixed metered feign client throwing IllegalArgumentException when using custom ErrorDecoder

* #1466: add missing license header

* Fold MeteredFeignClientTest into AbstractMetricsTestBase

Co-authored-by: Marvin Froeder <marvin.froeder@dovetailstudios.com>
@velo
Copy link
Member

velo commented Jul 26, 2021

Fixed on 11.6

velo added a commit that referenced this issue Oct 7, 2024
…1468)

* #1466: fixed metered feign client throwing IllegalArgumentException when using custom ErrorDecoder

* #1466: add missing license header

* Fold MeteredFeignClientTest into AbstractMetricsTestBase

Co-authored-by: Marvin Froeder <marvin.froeder@dovetailstudios.com>
velo added a commit that referenced this issue Oct 8, 2024
…1468)

* #1466: fixed metered feign client throwing IllegalArgumentException when using custom ErrorDecoder

* #1466: add missing license header

* Fold MeteredFeignClientTest into AbstractMetricsTestBase

Co-authored-by: Marvin Froeder <marvin.froeder@dovetailstudios.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants