fix(metricprovider): implement Terminate for Kayenta provider. Fixes #4815#4825
Open
Goutham-Annem wants to merge 1 commit into
Open
fix(metricprovider): implement Terminate for Kayenta provider. Fixes #4815#4825Goutham-Annem wants to merge 1 commit into
Goutham-Annem wants to merge 1 commit into
Conversation
…rgoproj#4815 The Kayenta provider can return AnalysisPhaseRunning from Run(), making it an asynchronous provider. When an AnalysisRun is terminated while a Kayenta measurement is in-flight, the controller calls Terminate() on the provider. Previously, Terminate() returned the measurement unchanged (still with phase AnalysisPhaseRunning), so the AnalysisRun could never fully terminate — leaking the in-flight measurement indefinitely. Fix: set FinishedAt and transition the phase to AnalysisPhaseSuccessful so the AnalysisRun termination path can proceed normally. Also correct the misleading doc comment on Resume() which implied it would never be called. Signed-off-by: Goutham Annem <gouthemannem@gmail.com>
|
Contributor
Published E2E Test Results 4 files 4 suites 3h 45m 4s ⏱️ Results for commit e2fc199. |
Contributor
Published Unit Test Results2 517 tests 2 517 ✅ 3m 23s ⏱️ Results for commit e2fc199. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4825 +/- ##
==========================================
- Coverage 85.09% 84.97% -0.12%
==========================================
Files 166 166
Lines 19136 19139 +3
==========================================
- Hits 16283 16264 -19
- Misses 2005 2016 +11
- Partials 848 859 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #4815
Problem
The Kayenta metric provider is asynchronous —
Run()POSTs a canary analysis job and returnsAnalysisPhaseRunning, thenResume()polls until the job completes. When anAnalysisRunis terminated while a Kayenta measurement is in-flight, the controller callsTerminate()on the provider.Previously,
Terminate()only logged a warning and returned the measurement unchanged (still withAnalysisPhaseRunning). Because the analysis framework only considers theAnalysisRunterminated once all measurements reach a completed phase, the stuckAnalysisPhaseRunningmeasurement caused a resource leak — theAnalysisRuncould never fully terminate.Fix
Set
FinishedAtand transition the measurement phase toAnalysisPhaseSuccessfulso the normal termination path inanalysis.gocan proceed. Since Kayenta does not expose a job cancellation API the in-flight analysis job is left to expire naturally on Kayenta's side; only the controller-side measurement state needs to be resolved.Also corrects the misleading doc comment on
Resume(), which incorrectly stated the method would never be called — in factResume()is the polling path for every in-flight Kayenta measurement.Testing
TestRunSuccessfullytest to assert thatTerminatesetsAnalysisPhaseSuccessfuland a non-nilFinishedAt.TestTerminatetest covering the standaloneTerminatepath.Checklist