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.

Sign up for GitHub

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

Jump to bottom

KNOX-2953 - Uniform name of http method of client metric to lowerCase #789

Merged
merged 1 commit into from
Aug 31, 2023
Merged

KNOX-2953 - Uniform name of http method of client metric to lowerCase #789

merged 1 commit into from
Aug 31, 2023

Conversation

carrey-feng
Copy link
Contributor

@carrey-feng carrey-feng commented Aug 30, 2023 •
edited
Loading

Fix KNOX-2953

What changes were proposed in this pull request?

Uniform name of http method of client metric to lowerCase, when build metric name, just like the service metric name at InstrHttpClientBuilderProvider::methodNameString

How was this patch tested?

  1. Apply this PR and recompile
  2. Deploy new jar files to ${KNOX_HOME}/lib direcotry and restart gateway
  3. Use curl -X Get and curl -X GET to query from hbase
  4. Use visualVM or other tool to check whether the metric name is consistent

@carrey-feng carrey-feng changed the title Uniform name of http method of client metric to lowerCase KNOX-2953 - Uniform name of http method of client metric to lowerCase Aug 30, 2023
@moresandeep
Copy link
Contributor

moresandeep commented Aug 30, 2023 •
edited
Loading

Thank you for the patch @a516072575
I'll wait for the tests to pass before committing, if I forget please ping me here and I'll submit it.

moresandeep
moresandeep approved these changes Aug 30, 2023
View reviewed changes
Copy link
Contributor

@moresandeep moresandeep left a comment

Choose a reason for hiding this comment

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

looks good

@moresandeep moresandeep merged commit 69a1a71 into apache:master Aug 31, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@moresandeep moresandeep moresandeep approved these changes

@smolnar82 smolnar82 Awaiting requested review from smolnar82

@zeroflag zeroflag Awaiting requested review from zeroflag

@pzampino pzampino Awaiting requested review from pzampino

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@carrey-feng @moresandeep