Skip to content

hubble-cli: prevent empty strings in colorer escape sequences to fix excessive cpu usage#44119

Merged
tklauser merged 1 commit intocilium:mainfrom
tporeba:bug42564_checkForEmptyStrings
Feb 18, 2026
Merged

hubble-cli: prevent empty strings in colorer escape sequences to fix excessive cpu usage#44119
tklauser merged 1 commit intocilium:mainfrom
tporeba:bug42564_checkForEmptyStrings

Conversation

@tporeba
Copy link
Copy Markdown

@tporeba tporeba commented Feb 2, 2026

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
    // no, thank you :)
  • Thanks for contributing!

This PR adds additional empty string checks to the colorer sequences() func responsible for generating color escape sequences.
A bug in this func caused non-empty list of sequenced being generated for cases when coloring was expected to be disabled - there was an empty string returned in the array in such cases. Those empty strings were misinterpreted and caused a lot of unnecessary string replace calls in terminalEscaperWriter, increasing CPU usage of hubble observe command significantly under high load.

Fixes: #42564

Fixes increased CPU usage in `hubble observe` caused by log coloring feature, even when coloring was disabled

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 2, 2026
@github-actions github-actions bot added hubble-cli PRs or GitHub issues related with hubble-cli hubble-cli-exclusive Only changes hubble-cli files. kind/community-contribution This was a contribution made by a community member. labels Feb 2, 2026
@tporeba tporeba force-pushed the bug42564_checkForEmptyStrings branch from 7f4fa7b to ce6d0bb Compare February 2, 2026 14:00
@tporeba tporeba marked this pull request as ready for review February 2, 2026 14:07
@tporeba tporeba requested a review from a team as a code owner February 2, 2026 14:07
@tporeba tporeba requested a review from chancez February 2, 2026 14:07
@tporeba tporeba changed the title Bug42564 check for empty strings hubble-cli: prevent empty strings in colorer escape sequences to fix excessive cpu usage Feb 2, 2026
@ldelossa ldelossa added area/hubble Impacts hubble server or relay release-note/bug This PR fixes an issue in a previous release of Cilium. labels Feb 5, 2026
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 5, 2026
@tporeba tporeba force-pushed the bug42564_checkForEmptyStrings branch from ce6d0bb to 92b3fbb Compare February 6, 2026 15:56
@pchaigno
Copy link
Copy Markdown
Member

/test

@pchaigno
Copy link
Copy Markdown
Member

@chancez This small pull request is waiting for a review from you.

@pchaigno
Copy link
Copy Markdown
Member

@tporeba You'll need to address the CI failures before we can merge :)

@tklauser tklauser force-pushed the bug42564_checkForEmptyStrings branch 2 times, most recently from e2035ea to 05824d6 Compare February 18, 2026 16:21
Add additional empty string checks to the code generating colorer escape sequences.
Empty strings in sequences were misinterpreted and caused a lot of unnecessary string replace calls in terminalEscaperWriter
increasing CPU usage significantly under high load.

Fixes cilium#42564

Signed-off-by: Tomasz Poręba <tporeba@gmail.com>
@tklauser tklauser force-pushed the bug42564_checkForEmptyStrings branch from 05824d6 to e95c0f3 Compare February 18, 2026 16:24
@tklauser
Copy link
Copy Markdown
Member

/test

@tklauser tklauser enabled auto-merge February 18, 2026 16:32
@tklauser tklauser added this pull request to the merge queue Feb 18, 2026
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Feb 18, 2026
Merged via the queue into cilium:main with commit 78b46f6 Feb 18, 2026
77 of 79 checks passed
@tporeba
Copy link
Copy Markdown
Author

tporeba commented Feb 19, 2026

I see that tklauser did that before I noticed. Thank you :)

@tporeba
Copy link
Copy Markdown
Author

tporeba commented Feb 20, 2026

@pchaigno @tklauser Could you please add necessary labels to this PR, so this gets backported to versions 1.17, 1.18, 1.19?
I found this article https://docs.cilium.io/en/stable/contributing/release/backports/ so I guess it needs
needs-backport/1.17, needs-backport/1.18, needs-backport/1.19, right?

But I don't have necessary permissions to do that myself.

@rolinh rolinh added the needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch label Mar 10, 2026
@rolinh rolinh added needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch needs-backport/1.19 This PR / issue needs backporting to the v1.19 branch labels Mar 10, 2026
@rolinh
Copy link
Copy Markdown
Member

rolinh commented Mar 10, 2026

@tporeba I've added the labels for backporting to all supported versions. The regression is severe enough and the patch simple enough that I think we can argue for backporting to v1.18 and v1.17.

@smagnani96 smagnani96 mentioned this pull request Mar 16, 2026
4 tasks
@smagnani96 smagnani96 added backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. and removed needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch labels Mar 16, 2026
@smagnani96 smagnani96 mentioned this pull request Mar 16, 2026
4 tasks
@smagnani96 smagnani96 added backport-pending/1.18 The backport for Cilium 1.18.x for this PR is in progress. and removed needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch labels Mar 16, 2026
@smagnani96 smagnani96 mentioned this pull request Mar 16, 2026
10 tasks
@smagnani96 smagnani96 added backport-pending/1.19 The backport for Cilium 1.19.x for this PR is in progress. and removed needs-backport/1.19 This PR / issue needs backporting to the v1.19 branch labels Mar 16, 2026
@github-actions github-actions bot added backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. backport-done/1.19 The backport for Cilium 1.19.x for this PR is done. backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. and removed backport-pending/1.18 The backport for Cilium 1.18.x for this PR is in progress. backport-pending/1.19 The backport for Cilium 1.19.x for this PR is in progress. backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. labels Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/hubble Impacts hubble server or relay backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. backport-done/1.19 The backport for Cilium 1.19.x for this PR is done. hubble-cli PRs or GitHub issues related with hubble-cli hubble-cli-exclusive Only changes hubble-cli files. kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Increased "hubble observe" CPU usage after upgrade from 1.15.15 to 1.15.16 or newer

7 participants