Skip to content

[8.19](backport #51737) x-pack/filebeat/input/streaming: self-heal CrowdStrike transient discover failures#51763

Merged
kcreddy merged 3 commits into
8.19from
mergify/bp/8.19/pr-51737
Jul 7, 2026
Merged

[8.19](backport #51737) x-pack/filebeat/input/streaming: self-heal CrowdStrike transient discover failures#51763
kcreddy merged 3 commits into
8.19from
mergify/bp/8.19/pr-51737

Conversation

@mergify

@mergify mergify Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Proposed commit message

x-pack/filebeat/input/streaming: self-heal CrowdStrike transient discover failures

Transient connection-level failures from the CrowdStrike discover
endpoint previously counted toward the retry attempt limit and, once it
was exceeded, terminated the input and required an agent restart to
recover. An empty 200 body (io.EOF), a network error, or a timeout while
reaching discover or fetching the OAuth token are now classified as a
transientError and retried with capped back-off indefinitely, so the
input self-heals once the upstream recovers.

Termination is reserved for genuine hardErrors (origin mismatch, publish
failure). Other soft errors -- non-200 discover responses, OAuth auth
failures from bad credentials, malformed discover bodies, and firehose
failures -- still honour the configured (or default) attempt cap, so a
real misconfiguration continues to surface rather than retrying forever.
A net.Error check distinguishes network failures from HTTP-level ones so
an auth error is not mistaken for a transient failure.

The retry loop keeps two counters: attempt, which only counts
non-transient failures and drives the termination cap, and failures,
which counts every consecutive failure and drives the back-off and
DEGRADED reporting. This keeps transient failures from eroding the
attempt budget of a later non-transient failure, while a persistent
transient outage still backs off and is surfaced as DEGRADED without
terminating the input.

TestCrowdstrikeOAuthHTTPClientRespectsConfiguredTimeout is updated for
the new behaviour: a token-fetch timeout now retries rather than
terminating, so the test verifies the configured timeout is still
respected on each attempt (the discover endpoint is never reached)
instead of expecting the input to fail. Add tests for the transient
retry-past-cap path, the transient discover GET failure, and that
transient failures do not consume the non-transient attempt cap.

Assisted-By: Cursor (Claude Opus 4.8)

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works. Where relevant, I have used the stresstest.sh script to run them under stress conditions and race detector to verify their stability.
  • I have added an entry in ./changelog/fragments using the changelog tool.

Disruptive User Impact

How to test this PR locally

Related issues

Use cases

Screenshots

Logs


This is an automatic backport of pull request #51737 done by [Mergify](https://mergify.com).

…over failures (#51737)

Transient connection-level failures from the CrowdStrike discover
endpoint previously counted toward the retry attempt limit and, once it
was exceeded, terminated the input and required an agent restart to
recover. An empty 200 body (io.EOF), a network error, or a timeout while
reaching discover or fetching the OAuth token are now classified as a
transientError and retried with capped back-off indefinitely, so the
input self-heals once the upstream recovers.

Termination is reserved for genuine hardErrors (origin mismatch, publish
failure). Other soft errors -- non-200 discover responses, OAuth auth
failures from bad credentials, malformed discover bodies, and firehose
failures -- still honour the configured (or default) attempt cap, so a
real misconfiguration continues to surface rather than retrying forever.
A net.Error check distinguishes network failures from HTTP-level ones so
an auth error is not mistaken for a transient failure.

The retry loop keeps two counters: attempt, which only counts
non-transient failures and drives the termination cap, and failures,
which counts every consecutive failure and drives the back-off and
DEGRADED reporting. This keeps transient failures from eroding the
attempt budget of a later non-transient failure, while a persistent
transient outage still backs off and is surfaced as DEGRADED without
terminating the input.

TestCrowdstrikeOAuthHTTPClientRespectsConfiguredTimeout is updated for
the new behaviour: a token-fetch timeout now retries rather than
terminating, so the test verifies the configured timeout is still
respected on each attempt (the discover endpoint is never reached)
instead of expecting the input to fail. Add tests for the transient
retry-past-cap path, the transient discover GET failure, and that
transient failures do not consume the non-transient attempt cap.

Assisted-By: Cursor (Claude Opus 4.8)
(cherry picked from commit ff9660b)

# Conflicts:
#	x-pack/filebeat/input/streaming/crowdstrike.go
@mergify mergify Bot added backport conflicts There is a conflict in the backported pull request labels Jul 7, 2026
@mergify
mergify Bot requested review from a team as code owners July 7, 2026 07:21
@mergify
mergify Bot requested review from AndersonQ and andrzej-stencel and removed request for a team July 7, 2026 07:21
@mergify mergify Bot assigned kcreddy Jul 7, 2026
@mergify

mergify Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Cherry-pick of ff9660b has failed:

On branch mergify/bp/8.19/pr-51737
Your branch is up to date with 'origin/8.19'.

You are currently cherry-picking commit ff9660ba6.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   changelog/fragments/1783326685-crowdstrike-streaming-self-heal-transient.yaml
	modified:   x-pack/filebeat/input/streaming/crowdstrike_test.go
	modified:   x-pack/filebeat/input/streaming/crowdstrike_unit_test.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   x-pack/filebeat/input/streaming/crowdstrike.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@botelastic botelastic Bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🤖 GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)
  • /test : Run the Buildkite pipeline.

@github-actions github-actions Bot added bugfix Team:Security-Service Integrations Security Service Integrations Team labels Jul 7, 2026
@botelastic botelastic Bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jul 7, 2026
@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

TL;DR

agentbeat: Run pre-commit failed because the backport branch still contains unresolved merge conflict markers in x-pack/filebeat/input/streaming/crowdstrike.go:299-303. Resolve that hunk and rerun pre-commit.

Remediation

  • In x-pack/filebeat/input/streaming/crowdstrike.go, remove the <<<<<<<, =======, and >>>>>>> lines at 299-303 and keep the resolved backoff call that matches the new retry accounting: waitTime = calculateWaitTime(s.cfg.Retry.WaitMin, s.cfg.Retry.WaitMax, failures, s.cfg.Retry.MaxAttempts).
  • Run pre-commit run --all-files after resolving the conflict. For a narrower code check, also run the affected streaming package tests.
Investigation details

Root Cause

This is a configuration/code hygiene failure from an incomplete conflict resolution in the automatic backport. The Buildkite checkout merged PR #51763 into 8.19, then check-merge-conflict found conflict markers in x-pack/filebeat/input/streaming/crowdstrike.go.

The PR head currently contains this unresolved hunk:

// x-pack/filebeat/input/streaming/crowdstrike.go:297-303
var waitTime time.Duration
if s.cfg.Retry != nil {
<<<<<<< HEAD
	waitTime = calculateWaitTime(s.cfg.Retry.WaitMin, s.cfg.Retry.WaitMax, attempt)
=======
	waitTime = calculateWaitTime(s.cfg.Retry.WaitMin, s.cfg.Retry.WaitMax, failures, s.cfg.Retry.MaxAttempts)
>>>>>>> ff9660ba6 (x-pack/filebeat/input/streaming: self-heal CrowdStrike transient discover failures (#51737))

The failures branch is the intended one for this PR because the surrounding logic now tracks attempt for non-transient retry caps and failures for consecutive-failure backoff/DEGRADED reporting.

Evidence

check for merge conflicts................................................Failed
- hook id: check-merge-conflict
- exit code: 1

x-pack/filebeat/input/streaming/crowdstrike.go:299: Merge conflict string '<<<<<<<' found
x-pack/filebeat/input/streaming/crowdstrike.go:301: Merge conflict string '=======' found
x-pack/filebeat/input/streaming/crowdstrike.go:303: Merge conflict string '>>>>>>>' found

Verification

Not run locally; the failure is from a static pre-commit hook detecting literal conflict markers in the PR content.


What is this? | From workflow: PR Buildkite Detective

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

TL;DR

golangci-lint failed on all three OS jobs because the CrowdStrike streaming tests still use deprecated/global logger setup. Replace the new logp.TestingSetup() / logp.L() usage with localized logptest.NewTestingLogger patterns already used in the same files.

Remediation

  • In x-pack/filebeat/input/streaming/crowdstrike_test.go:46-47, remove logp.TestingSetup() and set logger := logptest.NewTestingLogger(t, t.Name()).
  • In x-pack/filebeat/input/streaming/crowdstrike_unit_test.go:27 and x-pack/filebeat/input/streaming/crowdstrike_unit_test.go:121, remove the logp.TestingSetup() calls; the test helper already creates localized loggers with logptest.NewTestingLogger(t, t.Name()).
  • If crowdstrike_unit_test.go no longer references logp, remove its github.com/elastic/elastic-agent-libs/logp import.
  • Rerun the scoped lint/test check for the affected files or package, for example golangci-lint run --whole-files x-pack/filebeat/input/streaming/... and the CrowdStrike streaming tests.
Investigation details

Root Cause

The PR head still contains test setup that is forbidden by the repo lint rules. The current GitHub Actions failure is materially different from the earlier Buildkite detective report about unresolved conflict markers: this run reaches golangci-lint and fails on forbidigo / staticcheck diagnostics in CrowdStrike test files.

Evidence

x-pack/filebeat/input/streaming/crowdstrike_test.go:47:12: use of `logp.L` forbidden because "accept a *logp.Logger as a parameter instead of using the global logp.L()" (forbidigo)
x-pack/filebeat/input/streaming/crowdstrike_test.go:46:2: SA1019: logp.TestingSetup is deprecated: Prefer using localized loggers. Use logptest.NewTestingLogger. (staticcheck)
x-pack/filebeat/input/streaming/crowdstrike_unit_test.go:27:2: SA1019: logp.TestingSetup is deprecated: Prefer using localized loggers. Use logptest.NewTestingLogger. (staticcheck)
x-pack/filebeat/input/streaming/crowdstrike_unit_test.go:121:2: SA1019: logp.TestingSetup is deprecated: Prefer using localized loggers. Use logptest.NewTestingLogger. (staticcheck)

Affected PR source locations:

// x-pack/filebeat/input/streaming/crowdstrike_test.go:45-47
func TestCrowdstrikeFalconHose(t *testing.T) {
	logp.TestingSetup()
	logger := logp.L()
// x-pack/filebeat/input/streaming/crowdstrike_unit_test.go:26-28
func TestFollowSession_FirehoseHTTPError(t *testing.T) {
	logp.TestingSetup()
// x-pack/filebeat/input/streaming/crowdstrike_unit_test.go:120-122
func TestFollowSession_NonObjectMessage(t *testing.T) {
	logp.TestingSetup()

Validation

Not run locally; this read-only workflow inspected the failed Actions logs and PR source only.

Follow-up

After the logger cleanup, rerun the golangci-lint workflow. The earlier Buildkite comment about conflict markers should also be considered resolved only if the PR branch no longer contains <<<<<<<, =======, or >>>>>>> markers.


What is this? | From workflow: PR Actions Detective

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

@kcreddy
kcreddy enabled auto-merge (squash) July 7, 2026 11:10
@kcreddy
kcreddy merged commit 3b42486 into 8.19 Jul 7, 2026
30 of 33 checks passed
@kcreddy
kcreddy deleted the mergify/bp/8.19/pr-51737 branch July 7, 2026 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport bugfix conflicts There is a conflict in the backported pull request Team:Security-Service Integrations Security Service Integrations Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant