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

Skip metric increment during existence check #12763

Merged
merged 4 commits into from
May 5, 2022

Conversation

vinayak03
Copy link
Contributor

@vinayak03 vinayak03 commented Oct 7, 2021

The metric are counted twice on single API call because we are calling the same function during existence check as well as update/write phase. This PR is simply adding check as to when we should record the metric.

I tried adding a unit test however since we are directly using the metrics in router, I was not able to add it.

Closes #12060

Signed-off-by: Vinayak Kadam kadamvinayak03@gmail.com

Signed-off-by: Vinayak Kadam <kadamvinayak03@gmail.com>
@vercel vercel bot temporarily deployed to Preview – vault October 7, 2021 07:23 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 7, 2021 07:23 Inactive
Signed-off-by: Vinayak Kadam <kadamvinayak03@gmail.com>
@vercel vercel bot temporarily deployed to Preview – vault October 7, 2021 07:29 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 7, 2021 07:29 Inactive
@@ -0,0 +1,3 @@
```release-note: bug
COMPONENT: Telemetry data getting counted twice
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
COMPONENT: Telemetry data getting counted twice
core: Fix double counting for "route" metrics

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated changelog text

Signed-off-by: Vinayak Kadam <kadamvinayak03@gmail.com>
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 7, 2021 12:16 Inactive
@vercel vercel bot temporarily deployed to Preview – vault October 7, 2021 12:16 Inactive
Copy link
Contributor

@HridoyRoy HridoyRoy left a comment

Choose a reason for hiding this comment

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

Hi @vinayak03 , thank you for the contribution!

Lgtm!

@heatherezell
Copy link
Contributor

Hi @vinayak03 - CircleCI has been a bit squirrelly as of late, and isn't wanting to report the automated test results to GitHub. Would you be willing to push an empty commit in order to get the tests nudged forward, so we can get this merged? Thanks in advance for your patience!

@vinayak03
Copy link
Contributor Author

Hi
I have rebased the changes to main branch, Please let me know if something is required from my side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vault telemetry double counting route metrics
4 participants