-
Notifications
You must be signed in to change notification settings - Fork 141
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
Abstract metrics to reduce dependency to legacy #2747
Abstract metrics to reduce dependency to legacy #2747
Conversation
94fd724
to
1f2b1e8
Compare
Signed-off-by: Tomoyuki Morita <[email protected]>
Signed-off-by: Tomoyuki Morita <[email protected]>
Signed-off-by: Tomoyuki Morita <[email protected]>
f9fdde6
to
9ec44c4
Compare
public enum EmrMetrics { | ||
EMR_CANCEL_JOB_REQUEST_FAILURE_COUNT, | ||
EMR_GET_JOB_RESULT_FAILURE_COUNT, | ||
EMR_START_JOB_REQUEST_FAILURE_COUNT, | ||
EMR_INTERACTIVE_QUERY_JOBS_CREATION_COUNT, | ||
EMR_STREAMING_QUERY_JOBS_CREATION_COUNT, | ||
EMR_BATCH_QUERY_JOBS_CREATION_COUNT; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The last three metrics relate specifically to the dispatcher, not directly to EMR APIs. It may be beneficial to group these separately from the EMR API failure metrics and consider removing the 'EMR_' prefix to avoid confusion and maintain clarity in naming conventions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think those metrics are already used ones. Do you know if it is OK to change the metrics name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets only do refactoring..changing names we can handle in a different PR.
Look into build errors if those are related. |
Failures are bwc tests and looks related to version bump. |
* Abstract metrics to reduce dependency to legacy Signed-off-by: Tomoyuki Morita <[email protected]> * Add comment Signed-off-by: Tomoyuki Morita <[email protected]> * Fix style Signed-off-by: Tomoyuki Morita <[email protected]> --------- Signed-off-by: Tomoyuki Morita <[email protected]> (cherry picked from commit ef2cef3) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Abstract metrics to reduce dependency to legacy * Add comment * Fix style --------- (cherry picked from commit ef2cef3) Signed-off-by: Tomoyuki Morita <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
) * Abstract metrics to reduce dependency to legacy Signed-off-by: Tomoyuki Morita <[email protected]> * Add comment Signed-off-by: Tomoyuki Morita <[email protected]> * Fix style Signed-off-by: Tomoyuki Morita <[email protected]> --------- Signed-off-by: Tomoyuki Morita <[email protected]>
Description
Issues Resolved
n/a
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.