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

The counter indicator increases by 2 each time (on happy path) #1548

Merged
merged 6 commits into from Dec 13, 2021

Conversation

vitalijr2
Copy link
Collaborator

@vitalijr2 vitalijr2 commented Dec 11, 2021

Fixes #1480

We call sample.stop(timer) in the finally block and therefore the call in the try-catch block is unnecessary.

@vitalijr2 vitalijr2 changed the title The counter indicator increases by 2 each time (on happy path). The counter indicator increases by 2 each time (on happy path) Dec 11, 2021
@vitalijr2
Copy link
Collaborator Author

/cc @Kuvaldis @tommyk-gears

import static org.mockito.Mockito.*;

@RunWith(MockitoJUnitRunner.class)
public class MeteredClientTest {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know the existing tests are a bit painfull to read, but, could we move this to https://github.com/OpenFeign/feign/blob/master/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java ?

That would also test any problems on dropwizard metric implementation

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sure. I will do

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have moved test to old abstract class. It works for mictometer: old client breaks the test and new one does not.

Dropwizard clients have another meter implementation and I cannot find how to repeat this case (two calls of meter/counter) there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the bug was limited to micrometer implementation, that is great

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 this pull request may close these issues.

MeteredClient.execute(), The counter indicator increases by 2 each time
2 participants