Add experiment signals to fleet remote config#2872
Conversation
github-actions
bot
added
team/container-platform
team/container-autoscaling
team/fleet
labels
![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: 2e153f00f4
ℹ️ 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".
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2872 +/- ##
==========================================
+ Coverage 40.03% 40.54% +0.51%
==========================================
Files 319 321 +2
Lines 28066 28514 +448
==========================================
+ Hits 11235 11560 +325
- Misses 16008 16107 +99
- Partials 823 847 +24
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
khewonc
force-pushed
the
khewonc/experiment-signals
branch
from
2e153f0 to
e686774
Compare
khewonc
force-pushed
the
khewonc/experiment-signals
branch
from
e686774 to
d0ff10a
Compare
* Add cluster_name as tag * Add updater_type
🛑 Gate Violations
ℹ️ Info🎯 Code Coverage (details) 🔗 Commit SHA: a816a2a | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback! |
|
|
||
| // Apply the spec patch. | ||
| if err := retryWithBackoff(ctx, func() error { | ||
| return d.client.Patch(ctx, dda, client.RawPatch(types.MergePatchType, op.Config)) |
There was a problem hiding this comment.
❓ question: Is this a blocking call? Else, any UPDATER_TASK instruction will block any further UPDATER_TASK until the first one is done.
This means we can't stop a change from the backend
There was a problem hiding this comment.
This is indeed a blocking call. Both spec patch and status update are needed to start the experiment before stoping it though. If we patch the spec, then cancel the start signal to stop, stopExperiment would see there's no active experiment (since the status wasn't updated yet) and fail. The patched spec would still be applied, but with no experiment to track it. If we cancel the start signal before patching, then there wouldn't be an experiment to stop either
| continue | ||
| } | ||
|
|
||
| seen[req.ID] = struct{}{} |
There was a problem hiding this comment.
❓ question: Should it be before the h(req)?
There was a problem hiding this comment.
I'm not entirely certain on this, but I think either would be fine
3 participants
What does this PR do?
Adds experiment signals: start, stop, and promote. This depends on #2838. The latest commit contains the relevant changes
Motivation
https://datadoghq.atlassian.net/browse/CONTP-1424
https://datadoghq.atlassian.net/browse/CONTP-1425
https://datadoghq.atlassian.net/browse/CONTP-1426
Additional Notes
Merge after #2838
Minimum Agent Versions
Are there minimum versions of the Datadog Agent and/or Cluster Agent required?
Describe your test plan
TBD
Checklist
bug,enhancement,refactoring,documentation,tooling, and/ordependenciesqa/skip-qalabel