Skip to content
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

[CECO-2][DatadogMonitor] Configure force sync period #1404

Merged
merged 2 commits into from
Oct 8, 2024

Conversation

tbavelier
Copy link
Member

What does this PR do?

  • Allows the user to configure the force sync period for API monitors (version stored in Datadog back-end) using DD_MONITOR_FORCE_SYNC_PERIOD env var

Motivation

Describe your test plan

  1. Deploy this version of the operator with the env var DD_MONITOR_FORCE_SYNC_PERIOD set to 5
  2. Deploy an example monitor, e.g. kubectl apply -f examples/datadogmonitor/metric-monitor-imagepullbackoff.yaml
  3. After a minute or so, in the Datadog UI, edit the monitor query, e.g. changing the threshold to 2 and the 10 minutes to 5 minutes instead (creating a drift from the DatadogMonitor)
  4. Ensure the monitor is reverted back to the DatadogMonitor query within 5 minutes at most from your modification : you can identify this by looking at the monitor audit trail in the UI or in kubernetes by reviewing the Monitor Last Force Sync Time in the DatadogMonitor (within Status) or the associated events
    image
  5. Modify DD_MONITOR_FORCE_SYNC_PERIOD to a non integer value, e.g. "0,5" and assess the presence of the operator log complaining the value is invalid:
{"level":"ERROR","ts":"2024-09-11T12:44:03.864Z","logger":"controllers.DatadogMonitor","msg":"Invalid value for monitor force sync period. Defaulting to 60 minutes.","datadogmonitor":{"name":"pods-imagepullbackoff","namespace":"system"},"error":"strconv.Atoi: parsing \"0,5\": invalid syntax"}

Checklist

  • PR has at least one valid label: bug, enhancement, refactoring, documentation, tooling, and/or dependencies
  • PR has a milestone or the qa/skip-qa label

@tbavelier tbavelier added this to the v1.10.0 milestone Sep 11, 2024
@tbavelier tbavelier requested review from a team as code owners September 11, 2024 12:45
if err != nil {
logger.Error(err, "Invalid value for monitor force sync period. Defaulting to 60 minutes.")
} else {
logger.V(1).Info("Setting monitor force sync period", "minutes", forceSyncPeriodInt)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid spamming the used sync period, this log is set to appear only in debug with logger.V(1)

@@ -71,8 +71,11 @@ To deploy a `DatadogMonitor` with the Datadog Operator, use the [`datadog-operat
```

This automatically creates a new monitor in Datadog. You can find it on the [Manage Monitors][7] page of your Datadog account.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this break line so the note stands out properly instead of being on the same line:
image

@codecov-commenter
Copy link

codecov-commenter commented Sep 11, 2024

Codecov Report

Attention: Patch coverage is 54.54545% with 5 lines in your changes missing coverage. Please review.

Project coverage is 49.01%. Comparing base (7fe7979) to head (6a54ec4).

Files with missing lines Patch % Lines
internal/controller/datadogmonitor/controller.go 54.54% 4 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1404      +/-   ##
==========================================
- Coverage   49.01%   49.01%   -0.01%     
==========================================
  Files         223      223              
  Lines       19501    19508       +7     
==========================================
+ Hits         9559     9562       +3     
- Misses       9453     9456       +3     
- Partials      489      490       +1     
Flag Coverage Δ
unittests 49.01% <54.54%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
internal/controller/datadogmonitor/controller.go 48.04% <54.54%> (-0.15%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7fe7979...6a54ec4. Read the comment docs.

@tbavelier
Copy link
Member Author

/merge

@dd-devflow
Copy link

dd-devflow bot commented Oct 8, 2024

🚂 MergeQueue: pull request added to the queue

The median merge time in main is 14m.

Use /merge -c to cancel this operation!

@dd-devflow
Copy link

dd-devflow bot commented Oct 8, 2024

🚨 MergeQueue: This merge request is in error

Gitlab pipeline didn't start on its own and we were unable to create it... Please retry.

If you need support, contact us on Slack #devflow with those details!

@tbavelier
Copy link
Member Author

/merge

@dd-devflow
Copy link

dd-devflow bot commented Oct 8, 2024

🚂 MergeQueue: pull request added to the queue

The median merge time in main is 14m.

Use /merge -c to cancel this operation!

@dd-devflow
Copy link

dd-devflow bot commented Oct 8, 2024

🚨 MergeQueue: This merge request is in error

Gitlab pipeline didn't start on its own and we were unable to create it... Please retry.

If you need support, contact us on Slack #devflow with those details!

@tbavelier tbavelier merged commit 1d9b69b into main Oct 8, 2024
41 of 42 checks passed
@tbavelier tbavelier deleted the tbavelier/parameterize_force_sync_monitor branch October 8, 2024 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants