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-1238] Configure checks tag cardinality within spec.global.checksTagCardinality #1309

Merged
merged 5 commits into from
Nov 19, 2024

Conversation

tbavelier
Copy link
Member

@tbavelier tbavelier commented Jul 24, 2024

What does this PR do?

  • Similar to 3efc80a, provides a way to directly configure the checks tag cardinality from the Agent CR without having to define an env var override
  • As this is not a specific feature, it's configured within global

Motivation

Minimum Agent Versions

Are there minimum versions of the Datadog Agent and/or Cluster Agent required?

Describe your test plan

  1. make IMG=<replace me> deploy to deploy the image and CRDs in system namespace
  2. Test the following scenarios and ensure the results are adequate:
    1. apiVersion: datadoghq.com/v2alpha1
      kind: DatadogAgent
      metadata:
        name: datadog
      spec:
        global:
          checksTagCardinality: orchestrator

      Scenario : tag cardinality set to orchestrator

      • Ensure the Agent env var DD_CHECKS_TAG_CARDINALITY is set to orchestrator:
      kubectl describe pod <NODE AGENT POD> | grep "DD_CHECKS_TAG_CARDINALITY"
      • Ensure the Agent cardinality is set to orchestrator :
      k exec -it <NODE AGENT NAME> -- agent config | grep checks_tag_cardinality
      image
    2. apiVersion: datadoghq.com/v2alpha1
      kind: DatadogAgent
      metadata:
        name: datadog
      spec:
        global:
          checksTagCardinality: illegal_value

      Scenario : tag cardinality set to illegal_value

      • Ensure the Agent env var DD_CHECKS_TAG_CARDINALITY is set to illegal_value:
      kubectl describe pod <NODE AGENT POD> | grep "DD_CHECKS_TAG_CARDINALITY"
      image
      • Ensure the Agent cardinality defaults to low by asserting the presence of the log mentioning the value is illegal :
      k logs <NODE AGENT POD> | grep "check tag cardinality"
      image
    3. Remove global.checksTagCardinality from your Agent CR
      Scenario : no checks tag cardinality

      • Assert the absence of DD_CHECKS_TAG_CARDINALITY env var on the node Agent
      image

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 the enhancement New feature or request label Jul 24, 2024
@tbavelier tbavelier added this to the v1.9.0 milestone Jul 24, 2024
@tbavelier tbavelier requested review from a team as code owners July 24, 2024 12:26
@codecov-commenter
Copy link

codecov-commenter commented Jul 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.88%. Comparing base (583f3ec) to head (61a6a27).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1309      +/-   ##
==========================================
+ Coverage   48.85%   48.88%   +0.03%     
==========================================
  Files         226      226              
  Lines       20132    20144      +12     
==========================================
+ Hits         9835     9847      +12     
  Misses       9782     9782              
  Partials      515      515              
Flag Coverage Δ
unittests 48.88% <100.00%> (+0.03%) ⬆️

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

Files with missing lines Coverage Δ
api/datadoghq/v2alpha1/datadogagent_types.go 100.00% <ø> (ø)
api/datadoghq/v2alpha1/test/builder.go 92.04% <100.00%> (+0.04%) ⬆️
...nternal/controller/datadogagent/override/global.go 47.81% <100.00%> (+1.40%) ⬆️

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 583f3ec...61a6a27. Read the comment docs.

---- 🚨 Try these New Features:

Copy link
Contributor

@urseberry urseberry left a comment

Choose a reason for hiding this comment

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

Not sure how the links will show up in the docs, but I see other bare links in the file, so presumably it's okay.

@tbavelier tbavelier changed the title [CECO-1238] Configure checks tag cardinality within spec.global.checksCardinality [CECO-1238] Configure checks tag cardinality within spec.global.checksTagCardinality Aug 5, 2024
@levan-m levan-m modified the milestones: v1.9.0, v1.10.0 Aug 26, 2024
@tbavelier tbavelier force-pushed the tbavelier/checks_tag_cardinality branch from f8171cf to 4de1ec5 Compare September 10, 2024 08:18
@@ -770,14 +770,21 @@ func (builder *DatadogAgentBuilder) WithOriginDetectionUnified(enabled bool) *Da
return builder
}

// Global OriginDetectionUnified
// Global Registry
Copy link
Member Author

@tbavelier tbavelier Sep 10, 2024

Choose a reason for hiding this comment

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

This single modification is un-related to the PR, is a minor comment fix from a copy-paste in 021bd60#diff-79f1664318b07c82e329953388426262e2bf32b1a831bc4413e12c77c6416a2aR695-R702.
The relevant updated part of the builder is below

@fanny-jiang fanny-jiang modified the milestones: v1.10.0, v1.11.0 Oct 10, 2024
@tbavelier
Copy link
Member Author

/merge

@dd-devflow
Copy link

dd-devflow bot commented Nov 18, 2024

Devflow running: /merge

View all feedbacks in Devflow UI.


2024-11-18 16:44:31 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.


2024-11-18 16:47:54 UTC ⚠️ MergeQueue: This merge request was unqueued

This merge request was unqueued

@tbavelier
Copy link
Member Author

/remove

@dd-devflow
Copy link

dd-devflow bot commented Nov 18, 2024

Devflow running: /remove

View all feedbacks in Devflow UI.


2024-11-18 16:47:51 UTC ℹ️ Devflow: /remove

@tbavelier
Copy link
Member Author

/merge

@dd-devflow
Copy link

dd-devflow bot commented Nov 19, 2024

Devflow running: /merge

View all feedbacks in Devflow UI.


2024-11-19 09:23:51 UTC ℹ️ MergeQueue: pull request added to the queue

The median merge time in main is 15m.

@dd-mergequeue dd-mergequeue bot merged commit f15528b into main Nov 19, 2024
29 checks passed
@dd-mergequeue dd-mergequeue bot deleted the tbavelier/checks_tag_cardinality branch November 19, 2024 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants