Skip to content

Guard TCP input debug logging with IsDebug() to halve per-event allocations#49737

Open
Copilot wants to merge 3 commits intomainfrom
copilot/reduce-per-event-allocations
Open

Guard TCP input debug logging with IsDebug() to halve per-event allocations#49737
Copilot wants to merge 3 commits intomainfrom
copilot/reduce-per-event-allocations

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 27, 2026

Debugw in the TCP input hot path unconditionally constructs structured log args (including RemoteAddr.String()) for every received packet, even when debug is disabled. This adds ~500 allocations/op on the ingest path.

  • Wrap the Debugw call with ctx.Logger.IsDebug(), consistent with the existing pattern used throughout the codebase (kafka client, autodiscover, CEL input, etc.)
if ctx.Logger.IsDebug() {
    ctx.Logger.Debugw(
        "Data received",
        "bytes", len(data),
        "remote_address", metadata.RemoteAddr.String(),
        "truncated", metadata.Truncated)
}

Benchmark results: 1014 → 518 allocs/op (−49%), no throughput regression.


💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Mar 27, 2026
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 27, 2026

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @copilot? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit
  • backport-active-all is the label that automatically backports to all active branches.
  • backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
  • backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

Agent-Logs-Url: https://github.com/elastic/beats/sessions/a6f371a4-cf63-4b2e-8596-5b63bc4f773b

Co-authored-by: strawgate <6384545+strawgate@users.noreply.github.com>
Copilot AI changed the title [WIP] Reduce per-event allocations in filebeat TCP input debug logging Guard TCP input debug logging with IsDebug() to halve per-event allocations Mar 27, 2026
Copilot AI requested a review from strawgate March 27, 2026 15:27
@strawgate strawgate marked this pull request as ready for review March 27, 2026 15:57
@strawgate strawgate requested a review from a team as a code owner March 27, 2026 15:57
@strawgate
Copy link
Copy Markdown
Contributor

/ai are there other TCP benchmarks which show improvement with this change?

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1221604c-4546-4e40-8619-556bf85b557e

📥 Commits

Reviewing files that changed from the base of the PR and between 61c2e9e and f677eb0.

📒 Files selected for processing (1)
  • filebeat/input/net/tcp/input.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • filebeat/input/net/tcp/input.go

📝 Walkthrough

Walkthrough

The TCP input Test method now creates the listener with net.ListenConfig{}.Listen(context.Background(), ...) instead of net.Listen(...), adding an explicit context. The "Data received" debug log is wrapped in if ctx.Logger.IsDebug() to avoid argument evaluation when debug logging is disabled. The TCP startup error message text was changed from "Failed to start TCP server: %w" to "failed to start TCP server: %w". Metrics recording, event timestamping, metadata handling, and event emission remain unchanged. No exported/public declarations were modified.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch copilot/reduce-per-event-allocations
  • 🛠️ Update Documentation: Commit on current branch
  • 🛠️ Update Documentation: Create PR

Comment @coderabbitai help to get the list of available commands and usage tips.

@strawgate strawgate added the backport-active-9 Automated backport with mergify to all the active 9.[0-9]+ branches label Mar 27, 2026
@pierrehilbert pierrehilbert added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Mar 28, 2026
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Mar 28, 2026
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

- Use (*net.ListenConfig).Listen instead of net.Listen (noctx)
- Fix typo: initalises → initialises
- Lowercase error string (ST1005)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-active-9 Automated backport with mergify to all the active 9.[0-9]+ branches skip-changelog Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[performance-profiler] Reduce per-event allocations in filebeat TCP input debug logging path

4 participants