Emit Kubernetes events for FA experiment transitions#3037
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: 785b9bb | Docs | Datadog PR Page | Give us feedback! |
1518df1 to
9c50323
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3037 +/- ##
==========================================
+ Coverage 42.43% 42.59% +0.15%
==========================================
Files 337 339 +2
Lines 29020 29120 +100
==========================================
+ Hits 12315 12404 +89
- Misses 15895 15906 +11
Partials 810 810
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
levan-m
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3b8301f66
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return | ||
| } | ||
| switch { | ||
| case oldPhase == "" && newStatus.Phase == v2alpha1.ExperimentPhaseRunning: |
There was a problem hiding this comment.
emitExperimentTransitionEvent only emits ExperimentStartProcessed for oldPhase == "", but processStartSignal can legitimately start a new experiment from non-running terminal phases (for example Promoted or Terminated). In those common follow-up runs, status still transitions to Running but no start event is produced, so users miss controller-side start signals for every experiment after the first one.
Useful? React with 👍 / 👎.
| d.emitInstallerStateRehydratedEvent(ctx, | ||
| types.NamespacedName{Namespace: dda.Namespace, Name: dda.Name}, | ||
| exp.ID, exp.Phase) |
There was a problem hiding this comment.
This startup path reads DatadogAgents with apiReader specifically because cache data may not be populated yet, but the new emitInstallerStateRehydratedEvent call resolves the same DDA again through emitDDAEventf (d.client.Get) and silently drops the event on cache miss. Because rehydration is one-time during daemon start, InstallerStateRehydrated can be permanently lost on cold starts even when rehydration succeeds.
Useful? React with 👍 / 👎.
… for idempotent tasks
| if oldStatus != nil { | ||
| oldPhase = oldStatus.Phase | ||
| } | ||
| if newStatus == nil || newStatus.Phase == oldPhase { |
There was a problem hiding this comment.
Question: in which scenario would the newStatus be nil?
There was a problem hiding this comment.
experimentStatus set only if experiment has run so all existing DDAs and fresh clusters will start with nil.
There was a problem hiding this comment.
..however in practice, if oldStatus isn't nil on line 56, new status can't be nil based, we don't clean up experiment status after it's promoted. But I'd still keep this to be more defensive.
adel121
left a comment
adel121
left a comment
There was a problem hiding this comment.
LGTM
Left minor questions
* fix(experiment): anchor timeout on Status.Experiment.StartedAt * fix(fleet): rehydrate installer state from DatadogAgent on daemon startup * fix(experiment): report TaskState_ERROR for the original start task on timeout * refactor(experiment): single experiment-state annotation on revisions * fix(fleet): publish abort to Fleet Automation the same way as timeout * feat(experiment): emit Kubernetes events for state transitions and Fleet tasks * fix linter * fix(experiment): cover all phase transitions and emit completed event for idempotent tasks * fix(fleet): bypass cache when emitting rehydrate event * fix linter (cherry picked from commit 1e1aeff) Co-authored-by: levan-m <[email protected]>
4 participants
What does this PR do?
Produce k8s events on experiment signals and transition. Option enabled by
and collected in DD by adding
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Minimum Agent Versions
Are there minimum versions of the Datadog Agent and/or Cluster Agent required?
Describe your test plan
Enabled events, run experiment and describe DDA to see events
Checklist
bug,enhancement,refactoring,documentation,tooling, and/ordependenciesqa/skip-qalabel