-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Extend Profiler to allow for non-timing info #18631
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
Conversation
Signed-off-by: TJ Neuenfeldt <tjneu@amazon.com>
Signed-off-by: TJ Neuenfeldt <tjneu@amazon.com>
❌ Gradle check result for 0086e78: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: TJ Neuenfeldt <tjneu@amazon.com>
❌ Gradle check result for a57cbaf: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: TJ Neuenfeldt <tjneu@amazon.com>
❌ Gradle check result for 6cce78a: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: TJ Neuenfeldt <tjneu@amazon.com>
❌ Gradle check result for 179a190: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
server/src/main/java/org/opensearch/search/profile/query/QueryProfileBreakdown.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/search/profile/query/QueryProfileBreakdown.java
Outdated
Show resolved
Hide resolved
Thanks for this work, @neuenfeldttj the change is really look great (at least to me), we should be close to finish it, thank you |
Signed-off-by: TJ Neuenfeldt <tjneu@amazon.com>
❌ Gradle check result for 0f9e007: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: TJ Neuenfeldt <tjneu@amazon.com>
0f9e007
to
60e9e77
Compare
❌ Gradle check result for 60e9e77: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: TJ Neuenfeldt <tjneu@amazon.com>
❌ Gradle check result for d22ebd6: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for d22ebd6: null Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for d22ebd6: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfileBreakdown.java
Outdated
Show resolved
Hide resolved
Signed-off-by: TJ Neuenfeldt <tjneu@amazon.com>
server/src/main/java/org/opensearch/search/profile/AbstractProfileBreakdown.java
Outdated
Show resolved
Hide resolved
One last thing (sorry, I missed it before), but LGTM otherwise, thank you @neuenfeldttj ! |
❌ Gradle check result for 4be314f: null Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: TJ Neuenfeldt <tjneu@amazon.com>
❕ Gradle check result for 8565746: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
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.
Thanks @neuenfeldttj, this looks like a lot of really useful plumbing work related to profiling. I'll defer to @reta for final approval and merging, but just had one small comment.
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.
Thanks @neuenfeldttj , great work! Thanks @jed326 for review!
Description
This is a smaller PR that helps solve #18460. Timer now extends a new class called ProfileMetric. The new code works how the current profiler code works but allows a breakdown to output all information (timers and non-timers). This will be useful for plugins once plugin profiling is supported. Concurrency is also achieved by determining the timers/non-timers and aggregating accordingly. Also, enums were eliminated from the generics so that the profiler could be more extensible and support this.
Related Issues
Resolves part of #18486
Resolves part of #18460
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.