feat(ddgr): surface Datadog Monitor state in CR status#3047
Conversation
🛑 Gate Violations
|
1c4acd5 to
2c606b1
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3047 +/- ##
=======================================
Coverage 42.99% 43.00%
=======================================
Files 339 339
Lines 29193 29226 +33
=======================================
+ Hits 12551 12568 +17
- Misses 15820 15836 +16
Partials 822 822
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:
|
2c606b1 to
8d0e93a
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds live Datadog-side “state” visibility to DatadogGenericResource (DDGR) status for resource types that can expose it (currently Monitors), similar to the existing UX for DatadogMonitor. It introduces three new status fields (state, stateLastUpdateTime, stateLastTransitionTime) and a StateSynced condition that reports refresh success/failure while preserving last-known state on refresh errors.
Changes:
- Extend the DDGR controller/handler interface with
refreshState()and implement Monitor state refresh (other handlers are no-ops). - Add new DDGR status fields + printer columns + generated CRD/OpenAPI/deepcopy updates.
- Add unit tests for state application logic and reconcile-path state refresh behavior; update docs to describe the new status surface.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/controller/utils/condition/condition_generic.go | Adds StateSynced condition type constant used by DDGR refresh logic. |
| internal/controller/datadoggenericresource/resource_handler.go | Extends ResourceHandler with refreshState() and defines ResourceState. |
| internal/controller/datadoggenericresource/controller.go | Adds idle-tick branch to refresh Datadog-side state and merges it into status fields + condition updates. |
| internal/controller/datadoggenericresource/monitors.go | Implements Monitor-specific refreshState() via Datadog API overall state. |
| internal/controller/datadoggenericresource/dashboards.go | Adds no-op refreshState() implementation. |
| internal/controller/datadoggenericresource/notebooks.go | Adds no-op refreshState() implementation. |
| internal/controller/datadoggenericresource/downtimes.go | Adds no-op refreshState() implementation. |
| internal/controller/datadoggenericresource/synthetics.go | Adds no-op refreshState() implementations for synthetics handlers. |
| internal/controller/datadoggenericresource/monitors_test.go | New unit tests for applyResourceState() timestamp/transition semantics. |
| internal/controller/datadoggenericresource/mock_handler_test.go | Extends mock handler to support refreshState() and tracks calls/results. |
| internal/controller/datadoggenericresource/controller_test.go | Adds reconcile tests covering state refresh success and failure behavior. |
| api/datadoghq/v1alpha1/datadoggenericresource_types.go | Adds new DDGR status fields + kubebuilder printer columns. |
| api/datadoghq/v1alpha1/zz_generated.deepcopy.go | Adds deepcopy for the new status time pointers. |
| api/datadoghq/v1alpha1/zz_generated.openapi.go | Updates OpenAPI schema for the new status fields. |
| config/crd/bases/v1/datadoghq.com_datadoggenericresources.yaml | Adds printer columns + schema for the new status fields. |
| config/crd/bases/v1/datadoghq.com_datadoggenericresources_v1alpha1.json | Adds schema for the new status fields in JSON CRD base. |
| docs/datadog_generic_resource.md | Documents the new Datadog-side status fields and StateSynced condition. |
| docs/datadog_generic_resource_dev.md | Updates contributor guide for the new handler method and post-dev steps. |
Files not reviewed (2)
- api/datadoghq/v1alpha1/zz_generated.deepcopy.go: Language not supported
- api/datadoghq/v1alpha1/zz_generated.openapi.go: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
OliviaShoup
left a comment
There was a problem hiding this comment.
thanks for the PR! 🚀 left one suggestion to chop up a long sentence into two
tbavelier
left a comment
There was a problem hiding this comment.
👍 , except ResourceState struct IMHO, see comment
a36238e to
740f7aa
Compare
tbavelier
left a comment
There was a problem hiding this comment.
LGTM, unblocking, but the dev documentation should be updated accordingly
7a40632 to
bde5952
Compare
Signed-off-by: Wassim DHIF <wassim.dhif@datadoghq.com>
bde5952 to
be5f03b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated no new comments.
Files not reviewed (2)
- api/datadoghq/v1alpha1/zz_generated.deepcopy.go: Language not supported
- api/datadoghq/v1alpha1/zz_generated.openapi.go: Language not supported
8b39974
into
main
What does this PR do?
Surfaces the live Datadog-side state of Monitor resources managed by
DatadogGenericResource(DDGR), matching the UXDatadogMonitoralready provides. Three new fields appear on the CR status —state,stateLastUpdateTime,stateLastTransitionTime— populated on a ~60s cadence from the Datadog API. A newStateSyncedcondition signals refresh success/failure (reason=GetErrorwhen the API call fails; last-known state is preserved).The design is intentionally type-agnostic at every layer (schema, condition, dispatch, reconcile branch). Only
MonitorHandler.refreshStatusFuncdoes Monitor-specific work; the other five handlers are one-line no-ops. When SLO support lands in DDGR, its handler implements the samerefreshStatusFunc, populates the same three generic fields, and uses the sameStateSyncedcondition — zero schema changes required.Motivation
This PR closes that gap for Monitor and leaves the door open for SLO without committing to its schema.
Additional Notes
DatadogMonitor:triggeredState(per-group breakdown — which hosts/services are firing inside a grouped monitor) is intentionally omitted from DDGR. It's Monitor-specific and can't be expressed type-agnostically without polluting the generic schema. Users who need per-group visibility should use the dedicatedDatadogMonitorCRD. If a real need emerges later, a Monitor-onlytriggeredStatefield can be added at the top level without breaking anything.setMonitorStateOnStatusinmonitors.gomirrorsconvertStateToStatusininternal/controller/datadogmonitor/controller.goby design — a one-time minor DRY violation in exchange for a smaller blast radius (no shared-package refactor that would touchDatadogMonitor). Both copies carry a cross-reference comment so future divergence is easy to spot.Minimum Agent Versions
No agent-side change. No minimum version bump.
Describe your test plan
1. Install Operator and DDGR
2. Apply a Monitor
DatadogGenericResource3. Verify
Expected — new
STATEandLAST STATE SYNCcolumns populated:Expected —
StateSyncedcondition + newState*status fields:Checklist
bug,enhancement,refactoring,documentation,tooling, and/ordependencies— please addenhancement(primary) and optionallydocumentationqa/skip-qalabel