[core] Config loader: first draft by masci · Pull Request #4 · DataDog/datadog-agent

masci · 2016-06-13T16:41:13Z

First draft of the loader system for checks' configurations.
The agent main module was adapted to follow the new logic (first search for the configuration, then search for the corresponding checks) but only for the Python checks atm.

More details on the README file

truthbk · 2016-06-15T15:44:14Z

I like go-logging but we've been using seelog almost everywhere else...

will open a PR to switch the logging library

olivielpeau · 2016-06-20T09:17:01Z

👍 LGTM, don't have any additional comment to make

changed config loading logic cosmetics use config provider interface renamed modules docs

Add open, rename, rmdir and unlink events

Kernel approvers

Fix #4

* Revert "Revert gitlab-use-module #3 (#25024)" This reverts commit b98551c. * [gitlab-use-module] Fixed trigger child pipeline * [gitlab-use-module] Applied suggestion

…ere" (#4…" This reverts commit 7df7851.

# This is the 1st commit message: [build] Fork rules_multitool to our own extension. Import the core parts of rules_multitool but with appropriate modifications for our needs. - Re-root things so this is a local extension, rather than a distinct module. - remove the ability to use .netrc for authentication. - remove the :cwd and :workspace_root variations. - There are equivalent workarounds using `--run_under="cd <path> &&"` - Using `bazel run` is usually not the best practice. Rules should use `$(location <tool>)`. If a tool is so common that people need to call it anywhere, at any time, then it should be in their path. - The code is left commented out. Ready to be enabled if a good case is presented. - Remove the support for WORKSPACE. Next steps: - Change download structure so the exeuctable name is the tool, rather than `"executable"`. - Stop passing the attributes of each binary around as json blobs to be encoded and decoded. - Make `:path` feature that will print the full execution path. # This is the commit message #2: Update bazel/multitool/extension.bzl Yeah. That's a better name. Co-authored-by: Joseph Gette <[email protected]> # This is the commit message #3: Update bazel/multitool/extension.bzl Co-authored-by: Joseph Gette <[email protected]> # This is the commit message #4: Update bazel/multitool/private/templates.bzl Co-authored-by: Joseph Gette <[email protected]> # This is the commit message #5: render

# This is the 1st commit message: [build] Fork rules_multitool to our own extension. Import the core parts of rules_multitool but with appropriate modifications for our needs. - Re-root things so this is a local extension, rather than a distinct module. - remove the ability to use .netrc for authentication. - remove the :cwd and :workspace_root variations. - There are equivalent workarounds using `--run_under="cd <path> &&"` - Using `bazel run` is usually not the best practice. Rules should use `$(location <tool>)`. If a tool is so common that people need to call it anywhere, at any time, then it should be in their path. - The code is left commented out. Ready to be enabled if a good case is presented. - Remove the support for WORKSPACE. Next steps: - Change download structure so the exeuctable name is the tool, rather than `"executable"`. - Stop passing the attributes of each binary around as json blobs to be encoded and decoded. - Make `:path` feature that will print the full execution path. # This is the commit message #2: Update bazel/multitool/extension.bzl Yeah. That's a better name. Co-authored-by: Joseph Gette <[email protected]> # This is the commit message #3: Update bazel/multitool/extension.bzl Co-authored-by: Joseph Gette <[email protected]> # This is the commit message #4: Update bazel/multitool/private/templates.bzl Co-authored-by: Joseph Gette <[email protected]> # This is the commit message #5: render # This is the commit message #6: iff # This is the commit message #7: commit # This is the commit message #8: lintyfresth

#42435) …2229)" ### What does this PR do? This reverts commit 57eccdf. ### Motivation [#incident-44969](https://dd.enterprise.slack.com/archives/C09P8AQQBV0) ### Describe how you validated your changes ### Additional Notes Co-authored-by: stanley.liu <[email protected]>

Skip the SSH session patcher and add a test to illustrate the current issue. In addition, adds the possibility to check specific fields in the json returned for ssh_session events. ### Motivation The retry mechanism could cause the agent to send no more than one event per minute if an SSH session was not properly resolved. Previously, the event was not sent and the agent would wait one minute before sending it with the `unknown` type. However, this `authtype` would never be resolved because the session was initialized before the agent started processing events. As a result, every subsequent SSH event would wait one minute for nothing, causing a significant delay in agent events, potentially blocking all the other events. ### Describe how you validated your changes Added a test that illustrate the issue : `TestSSHUserSessionBlocking` With this change, the ssh_session event is now sent with `authtype` set to `unknown` and directly sent. Error without commenting the patcher : ``` Error: Received unexpected error: All attempts fail: #1: not found #2: not found #3: not found #4: not found #5: not found #6: not found #7: not found #8: not found #9: not found #10: not found #11: not found #12: not found #13: not found #14: not found #15: not found #16: not found #17: not found #18: not found #19: not found #20: not found #21: not found #22: not found #23: not found #24: not found #25: not found #26: not found #27: not found #28: not found #29: not found #30: not found Test: TestSSHUserSessionBlocking/second_ssh_no_auth ``` Co-authored-by: theo.putegnat <[email protected]> (cherry picked from commit 40d1f09) ___ Co-authored-by: Théo Putegnat <[email protected]>

Backport 40d1f09 from #45437. ___ ### What does this PR do? Skip the SSH session patcher and add a test to illustrate the current issue. In addition, adds the possibility to check specific fields in the json returned for ssh_session events. ### Motivation The retry mechanism could cause the agent to send no more than one event per minute if an SSH session was not properly resolved. Previously, the event was not sent and the agent would wait one minute before sending it with the `unknown` type. However, this `authtype` would never be resolved because the session was initialized before the agent started processing events. As a result, every subsequent SSH event would wait one minute for nothing, causing a significant delay in agent events, potentially blocking all the other events. ### Describe how you validated your changes Added a test that illustrate the issue : `TestSSHUserSessionBlocking` With this change, the ssh_session event is now sent with `authtype` set to `unknown` and directly sent. Error without commenting the patcher : ``` Error: Received unexpected error: All attempts fail: #1: not found #2: not found #3: not found #4: not found #5: not found #6: not found #7: not found #8: not found #9: not found #10: not found #11: not found #12: not found #13: not found #14: not found #15: not found #16: not found #17: not found #18: not found #19: not found #20: not found #21: not found #22: not found #23: not found #24: not found #25: not found #26: not found #27: not found #28: not found #29: not found #30: not found Test: TestSSHUserSessionBlocking/second_ssh_no_auth ``` Co-authored-by: axel.vonengel <[email protected]>

Backport 40d1f09 from #45437. ___ ### What does this PR do? Skip the SSH session patcher and add a test to illustrate the current issue. In addition, adds the possibility to check specific fields in the json returned for ssh_session events. ### Motivation The retry mechanism could cause the agent to send no more than one event per minute if an SSH session was not properly resolved. Previously, the event was not sent and the agent would wait one minute before sending it with the `unknown` type. However, this `authtype` would never be resolved because the session was initialized before the agent started processing events. As a result, every subsequent SSH event would wait one minute for nothing, causing a significant delay in agent events, potentially blocking all the other events. ### Describe how you validated your changes Added a test that illustrate the issue : `TestSSHUserSessionBlocking` With this change, the ssh_session event is now sent with `authtype` set to `unknown` and directly sent. Error without commenting the patcher : ``` Error: Received unexpected error: All attempts fail: #1: not found #2: not found #3: not found #4: not found #5: not found #6: not found #7: not found #8: not found #9: not found #10: not found #11: not found #12: not found #13: not found #14: not found #15: not found #16: not found #17: not found #18: not found #19: not found #20: not found #21: not found #22: not found #23: not found #24: not found #25: not found #26: not found #27: not found #28: not found #29: not found #30: not found Test: TestSSHUserSessionBlocking/second_ssh_no_auth ``` Co-authored-by: YoannGh <[email protected]> Co-authored-by: florent.clarret <[email protected]>

### What does this PR do? Skip the SSH session patcher and add a test to illustrate the current issue. In addition, adds the possibility to check specific fields in the json returned for ssh_session events. ### Motivation The retry mechanism could cause the agent to send no more than one event per minute if an SSH session was not properly resolved. Previously, the event was not sent and the agent would wait one minute before sending it with the `unknown` type. However, this `authtype` would never be resolved because the session was initialized before the agent started processing events. As a result, every subsequent SSH event would wait one minute for nothing, causing a significant delay in agent events, potentially blocking all the other events. ### Describe how you validated your changes Added a test that illustrate the issue : `TestSSHUserSessionBlocking` With this change, the ssh_session event is now sent with `authtype` set to `unknown` and directly sent. Error without commenting the patcher : ``` Error: Received unexpected error: All attempts fail: #1: not found #2: not found #3: not found #4: not found #5: not found #6: not found #7: not found #8: not found #9: not found #10: not found #11: not found #12: not found #13: not found #14: not found #15: not found #16: not found #17: not found #18: not found #19: not found #20: not found #21: not found #22: not found #23: not found #24: not found #25: not found #26: not found #27: not found #28: not found #29: not found #30: not found Test: TestSSHUserSessionBlocking/second_ssh_no_auth ``` Co-authored-by: theo.putegnat <[email protected]>

Summary of Changes HIGH Priority Issues Fixed: #1: Write lock held across network I/O (impl/delegatedauth.go:270) - Refactored refreshAndGetAPIKey to release the lock before making network calls (authenticate) - The lock is now only held briefly to check/update state, not during network I/O #2: Context not propagated to signer.SignHTTP (aws.go:195) - Updated generateAwsAuthData to accept a context parameter - Changed signer.SignHTTP(context.Background(), ...) to signer.SignHTTP(ctx, ...) #3: Context not propagated to getCredentials IMDS call (aws.go:119) - Updated getCredentials to accept a context parameter - Removed ctx := context.Background() and now uses the passed context for IMDS calls MEDIUM Priority Issues Fixed: #4: No response body size limit (api/delegated_auth.go:97) - Added maxResponseBodySize = 1 * 1024 * 1024 constant (1 MB) - Wrapped response body with io.LimitReader to prevent memory exhaustion #5: No overall HTTP client timeout (api/delegated_auth.go:82) - Added httpClientTimeout = 30 * time.Second constant - Added Timeout: httpClientTimeout to the HTTP client #6: config.Set called while holding write lock (impl/delegatedauth.go:341) - Moved updateConfigWithAPIKey call outside the lock in startBackgroundRefresh - Captured the API key while holding the lock, then released it before calling config.Set #7: Blocking IMDS calls while holding write lock (impl/delegatedauth.go:127) - Refactored initializeIfNeeded to perform cloud detection without holding locks - IMDS calls now happen outside any lock, then state is updated with a brief write lock #8: Regex fails silently for non-standard formats (api/delegated_auth.go:36) - Added debug log when endpoint doesn't match known Datadog domain pattern - Updated function documentation to clarify behavior #9: Uncached IMDS credential fetch (aws.go:104) - Added documentation explaining the trade-off (refresh interval is typically 60 minutes, so caching is not critical) #10: Auth proof format undocumented (aws.go:98) - Added detailed comment documenting the auth proof format: <base64-body>|<base64-headers>|<method>|<base64-url> LOW Priority Issues Fixed: #11: Unnecessarily exported types (aws.go) - Changed SigningData to signingData (unexported) - Changed AWSAuth.AwsRegion to AWSAuth.region (unexported) - Updated all references in aws.go and aws_test.go #12: Tests exercise copy of goroutine (impl/delegatedauth_test.go:19) - Added documentation explaining why tests use a simplified goroutine pattern - Clarified that integration tests cover the actual startBackgroundRefresh function #13: Subsequent Config param silently ignored (def/delegatedauth.go:24) - Updated documentation to clearly state that only the first Config is used - Added warning log when a different Config is passed on subsequent calls

- Run benchmarks across comp/dogstatsd/server, pkg/aggregator, and comp/forwarder/defaultforwarder; results saved to plans/bench-baseline-forwarder.txt - Generated pprof profiles (mem.out, cpu.out) from aggregator flush benchmarks - Created scripts/profile_pipeline.sh documenting exact reproduction commands - Documented top 10 allocation sites and CPU hotspots in plans/profiling-baseline.md - Added benchmark test files for aggregator (time_sampler, context_resolver), forwarder, and dogstatsd/server that will be used in subsequent stories - Typecheck passes (go build ./comp/... ./pkg/aggregator/... ./comp/forwarder/...) Key findings: - pkg/metrics.(*Gauge).flush is #1 allocator (26.97% of objects) — target US-004 - contextResolver.trackContext is #4 allocator and 26.58% cumulative CPU — target US-003 - GC overhead accounts for ~22% of CPU — directly reducible via alloc reduction - Forwarder: 15 allocs/op per transaction creation — target US-007 Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

…ata" (#4…" This reverts commit d14ef14.

### What does this PR do? Add `.claude/rules/bazel.md`, a path-scoped Claude Code _rule_ that instructs Claude to injest `bazel/AGENTS.md` before reading or editing any Bazel-related file. ### Motivation Claude does not systematically load `bazel/AGENTS.md` when touching Bazel-related files, leading to mistakes the document is meant to prevent: - gardening `BUILD.bazel` files by hand instead of running `gazelle`, - writing `genrules` with non-portable scripts instead of leveraging `native_binary`/`run_binary`, - etc. A path-scoped rule loads only when a matching file is in scope, so the directive is delivered at the moment it is needed without inflating context the rest of the time. Addresses item #4 of the AI Dev Week 1 project ideas: https://datadoghq.atlassian.net/wiki/spaces/ABLD/pages/6638142056/Week+1+-+May+25th+to+29th#%F0%9F%92%A1-Project-Ideas ### Describe how you validated your changes - in a fresh session, ran `/memory` with no Bazel file touched: `.claude/rules/bazel.md` is not listed, confirming **conditional loading**, - in a fresh session, asked what to do before reading any `BUILD.bazel` file: Claude mentioned to read `bazel/AGENTS.md` as instructed, - in another fresh session, asked to remove default package visibility from the root `BUILD.bazel`: Claude indeed consulted `bazel/AGENTS.md` and started applying its guidance. ### Additional Notes Frontmatter key is `paths:` per https://code.claude.com/docs/en/memory#path-specific-rules, not `globs:`. The sibling .claude/rules/allium.md uses `globs:` and is therefore loading unconditionally => left as-is for a separate change.

Extends three test files: - packet/packet_test.go: TestGetTagsWithCustomTags{,SNMPV1} verify the built-in tags stay first and user tags append in declared order; TestGetTagsWithEmptyCustomTags covers the empty/nil path (PRD success criterion #4 — byte-for-byte unchanged behaviour). - config/def/config_test.go: covers YAML unmarshal, default-empty, whitespace/empty normalization, and the OversizedTags helper. - formatter/impl/formatter_test.go: TestFormatPacketIncludesCustomTagsInDDTags asserts ddtags includes user tags (criterion #2); TestFormatPacketCustomTagsOnTelemetry asserts traps_not_enriched and incorrect_format both carry the custom tags (criterion #3). Split out TestFormatPacketToJSONBody to preserve the original variable-by-variable coverage now that TestFormatPacketToJSON only covers ddtags. Co-Authored-By: Claude <[email protected]>

Replaces the brittle 'HorizontalLastActions > 0' gate with an explicit LastScaledTarget tracker on PodAutoscalerInternal. The tracker records (namespace, name, GVK) every time the horizontal controller successfully writes `.spec.replicas`, and is cleared after a successful release. The release path now fires on three triggers, not one: - horizontal scaling disabled (existing) - apply mode switched to Preview (#4 — previously not covered) - the DPA's TargetRef was retargeted to a different workload, so the OLD target still holds the stale managedFields entry while the new target has none (#7 — previously released against the wrong target) Release-failure handling also overhauled: - On failure, the helper now constructs a ConditionError, emits a Warning Event on the DPA, calls UpdateFromHorizontalAction(nil, err), increments HorizontalActionErrorInc, and returns the error so the workqueue's maxRetry guard caps the loop instead of hot-looping invisibly with (Requeue, nil) (#3 — was previously silent). - The three delete branches (remote-owned, profile-managed, local-owned) now retry release up to maxRetry attempts via c.Workqueue.NumRequeues; on exhaustion they log Errorf and proceed with the delete so a permanently broken release (RBAC never granted) cannot indefinitely block a user from deleting a DPA (#1). Tests: - Existing TestHorizontalControllerReleaseOwnershipOnDisable updated to seed LastScaledTarget instead of HorizontalLastActions. - New TestHorizontalControllerReleaseOwnershipOnPreviewTransition. - New TestHorizontalControllerReleaseOwnershipOnTargetRefChange asserts release fires against the OLD target, not the spec's current target. - TestHorizontalControllerReleaseOwnershipOnDisable_FailureRetainsState rewritten to assert (Requeue, err) and LastScaledTarget retention. - TestLeaderCreateDeleteLocal / TestLeaderCreateDeleteRemote updated to seed LastScaledTarget so the delete path actually exercises the release call. - testScalingDecision now mirrors SetLastScaledTarget after every successful scale, matching production semantics. LastScaledTarget is in-memory only — on cluster-agent restart it resets and the next successful scale re-populates it. The trade-off is a narrow window where a DPA disabled across a controller restart with no subsequent scale would not release; acceptable in exchange for not needing CRD status schema changes. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

#4) Turn-1 token usage of a tool workflow was cached per connection, so two workflows multiplexed on one connection overwrote each other's entry and a follow-up could report the wrong turn-1 cost on its first llm span. Key the cache by the tool_call id instead: turn-1's response produces the id(s), and the follow-up request references the same id — so caching usage under each produced tool_call id and looking it up by the id the follow-up responds to attributes turn-1 cost to the correct workflow, even under concurrency. TestGenUsagePairsByToolCallID drives two interleaved workflows (A usage 10, B usage 20) on one connection and asserts each follow-up recovers its own turn-1 usage. All existing tests still pass (tool-call-gen assertion updated to key by id). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

masci force-pushed the massi/loader_first_draft branch from 65dc49a to 52f2d52 Compare June 15, 2016 10:18

truthbk reviewed Jun 15, 2016
View reviewed changes

masci added the component/collector label Jun 16, 2016

first pass at loader logic and structure

f32d3a5

changed config loading logic cosmetics use config provider interface renamed modules docs

masci force-pushed the massi/loader_first_draft branch from 85d7386 to f32d3a5 Compare June 21, 2016 15:22

masci merged commit d4636e9 into master Jun 21, 2016

masci deleted the massi/loader_first_draft branch June 21, 2016 15:26

safchain added a commit to safchain/datadog-agent that referenced this pull request May 11, 2020

Merge pull request DataDog#4 from lebauce/more-hooks

ed107a6

Add open, rename, rmdir and unlink events

safchain pushed a commit to safchain/datadog-agent that referenced this pull request Jun 4, 2020

Merge pull request DataDog#4 from DataDog/poc-in-kernel-approvers

c6c9366

Kernel approvers

pgimalac pushed a commit that referenced this pull request May 12, 2023

Capacity metrics are in bytes not in kb so it needs to be converted

34093d3

Fix #4

akarpz mentioned this pull request Oct 26, 2023

do DNS lookups in require.eventually #20444

Closed

10 tasks

CelianR added a commit that referenced this pull request Aug 20, 2025

Revert "Revert "[ACIX-616] Stack cleaner migration: enable it everywh…

2172190

…ere" (#4…" This reverts commit 7df7851.

BaptisteFoy mentioned this pull request Feb 23, 2026

fix(fleet/installer): fix ~90s daemon stop delay during installer setup #46757

Closed

matt-dz mentioned this pull request Feb 26, 2026

Implement Agent Safe Shell — POSIX commands as safe builtins #46945

Closed

4 tasks

chouetz added a commit that referenced this pull request Apr 16, 2026

Revert "Revert "[CONTP-1583]Collect cluster check status in DCA metad…

88b2861

…ata" (#4…" This reverts commit d14ef14.

ellataira mentioned this pull request May 2, 2026

coord run-log (full) — 2026-05-01 20:23 #50249

Draft

gh123man mentioned this pull request May 18, 2026

[AGNTLOG-369] feat(logs): add ignore_older config for file scanner #50956

Closed

4 tasks

coignetp mentioned this pull request May 26, 2026

[FA] test(fleet/e2e): close blind spots in integration-preservation suite #51286

Merged

3 tasks

rdesgroppes mentioned this pull request May 27, 2026

Add path-scoped Claude _rule_ for Bazel matters #51353

Merged

celenechang mentioned this pull request Jun 10, 2026

fix(autoscaling): release .spec.replicas ownership when DPA stops scaling #52040

Open

5 tasks

JSGette added a commit that referenced this pull request Jun 11, 2026

Enable even more flavored tests #4

fb6c6a7

BaptisteFoy mentioned this pull request Jul 7, 2026

fix(fleet/installer): correctly select and build the FIPS package variant #52777

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[core] Config loader: first draft#4

[core] Config loader: first draft#4
masci merged 1 commit into
masterfrom
massi/loader_first_draft

masci commented Jun 13, 2016 •

edited

Loading

Uh oh!

truthbk Jun 15, 2016

Uh oh!

masci Jun 21, 2016

Uh oh!

olivielpeau commented Jun 20, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

masci commented Jun 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

truthbk Jun 15, 2016

Choose a reason for hiding this comment

Uh oh!

masci Jun 21, 2016

Choose a reason for hiding this comment

Uh oh!

olivielpeau commented Jun 20, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

masci commented Jun 13, 2016 •

edited

Loading