Skip to content

Enhancement log stream reading and writing and handle new lines and max-size#5683

Merged
anbraten merged 11 commits into
woodpecker-ci:mainfrom
LUKIEYF:feature/5646
Oct 29, 2025
Merged

Enhancement log stream reading and writing and handle new lines and max-size#5683
anbraten merged 11 commits into
woodpecker-ci:mainfrom
LUKIEYF:feature/5646

Conversation

@LUKIEYF
Copy link
Copy Markdown
Contributor

@LUKIEYF LUKIEYF commented Oct 24, 2025

close: #5646
feature: 5646
enhancement of the log stream reading and writing.
two criteria to write to dist:

  1. reach the '\n'
  2. reach the max_size

how:
add a new buf to cache the data unwrite and read the fixed size of array from the kernel interface each time.
the maximum size of the buf would be 2 * maxSize - 1

@LUKIEYF
feature: read the log stream from cache while it has '\n' in the stre…
4e4c995 
…am byte.

For the most of log byte less than 1024 in a row, directly write into, otherwise chunk writting. (slightly enhancement in reducing the function calling)
@LUKIEYF
feature: 5646
9f53af1 
enhancement of the log stream reading and writing.
two criteria to write to dist:
1. reach the '\n'
2. reach the max_size

how:
add a new buf to cache the data unwrite and read the fixed size of array from the kernel interface each time.
the maximum size of the buf would be 2 * maxSize - 1
@LUKIEYF
Copy link
Copy Markdown
Contributor Author

LUKIEYF commented Oct 24, 2025

please kindly give comment :) thanks all. @6543

@LUKIEYF LUKIEYF changed the title Feature/5646 Feature/5646 enhancement of the log stream reading and writing. Oct 24, 2025
@qwerty287 qwerty287 added the refactor delete or replace old code label Oct 24, 2025
@qwerty287
Copy link
Copy Markdown
Contributor

qwerty287 commented Oct 24, 2025

@LUKIEYF thanks! Can you checkout the linter and tests?

@LUKIEYF
Copy link
Copy Markdown
Contributor Author

LUKIEYF commented Oct 24, 2025

@LUKIEYF thanks! Can you checkout the linter and tests?

No problem :)

LUKIEYF and others added 2 commits October 27, 2025 14:43
@LUKIEYF
feature: 5646
39df0e9 
changes:
1. change the if-else statement to switch to comply with the golint requirement.
2. the test case TestCopyLineByLineSizeLimit() the first testing scenario should be expected 1 write once the max size is 4 and the input bytes length is 5.
3. add three testing case about verify the scenario related to newline character appear into the bytes, the scenario of long line and the scenario of writeChunks() function would be used.
@LUKIEYF
Merge branch 'main' into feature/5646
34a9bb9
@LUKIEYF
Copy link
Copy Markdown
Contributor Author

LUKIEYF commented Oct 27, 2025

Hi all,

thanks for your comments.

  1. the code that did not pass golint was been revised.

  2. add three more testing cases for newline character matched, long line, and writeChunk() function.

  3. modified one assert requirement since the max size is length in 4 but input is length in 5 therefore 1 writes will happen.

qwerty287
qwerty287 reviewed Oct 27, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@qwerty287 qwerty287 left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me so far. Just 2 small points.

Maybe someone else from @woodpecker-ci/maintainers would like to take a look?

Comment thread pipeline/log/utils.go
Comment thread pipeline/log/utils_test.go
@qwerty287
Copy link
Copy Markdown
Contributor

qwerty287 commented Oct 27, 2025

And the linter is still not happy ;)

LUKIEYF and others added 2 commits October 27, 2025 15:25
@LUKIEYF @qwerty287
Update pipeline/log/utils.go
79c2e33 
Co-authored-by: qwerty287 <80460567+qwerty287@users.noreply.github.com>
@LUKIEYF
1. fixed the golangci-lint warning.
b1dd18d
@LUKIEYF
Copy link
Copy Markdown
Contributor Author

LUKIEYF commented Oct 27, 2025

Hi all,

all golangci-lint warnings are fixed

thank you :)

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 27, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 69.04762% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 21.22%. Comparing base (393a598) to head (adea1f5).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pipeline/log/utils.go 69.04% 8 Missing and 5 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #5683      +/-   ##
==========================================
+ Coverage   21.20%   21.22%   +0.02%     
==========================================
  Files         425      425              
  Lines       38367    38402      +35     
==========================================
+ Hits         8135     8152      +17     
- Misses      29477    29491      +14     
- Partials      755      759       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@LUKIEYF
Copy link
Copy Markdown
Contributor Author

LUKIEYF commented Oct 27, 2025

I saw the codecov said "Patch coverage is 72.72727% with 12 lines in your changes missing coverage."
May I know is that need to cover to 100% without any missing lines in the testing code?
thank you :)

@qwerty287
Copy link
Copy Markdown
Contributor

qwerty287 commented Oct 27, 2025

No, it's fine how it's now

@anbraten anbraten changed the title Feature/5646 enhancement of the log stream reading and writing. Enhancement of the log stream reading and writing Oct 27, 2025
anbraten
anbraten reviewed Oct 27, 2025
View reviewed changes
Comment thread pipeline/log/utils.go Outdated
anbraten
anbraten reviewed Oct 27, 2025
View reviewed changes
Comment thread pipeline/log/utils.go Outdated
LUKIEYF and others added 3 commits October 28, 2025 10:10
@LUKIEYF @anbraten
Update pipeline/log/utils.go
9903cd7 
Co-authored-by: Anbraten <6918444+anbraten@users.noreply.github.com>
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks [CI SKIP]
f9512c5 
for more information, see https://pre-commit.ci
@LUKIEYF
1. remove the unnecessary comment
edbe800 
2. add the break out statement after handling the EOF and writting the remaining buffer, otherwise the EOF would be treated as an error and the function cannot exit cleanly.
@LUKIEYF
Copy link
Copy Markdown
Contributor Author

LUKIEYF commented Oct 28, 2025

  1. remove the unnecessary comments.
  2. add a break out statement after handling EOF and writing the remaining buffer otherwise the EOF would be treated as an error and the function cannot exit cleanly.

@xoxys xoxys added the server label Oct 28, 2025
@anbraten anbraten changed the title Enhancement of the log stream reading and writing Enhancement log stream reading and writing and handle new lines and max-size Oct 29, 2025
@anbraten anbraten merged commit 7ce6536 into woodpecker-ci:main Oct 29, 2025
9 checks passed
@woodpecker-bot woodpecker-bot mentioned this pull request Oct 29, 2025
Merged
1 task
@BrewTestBot BrewTestBot mentioned this pull request Nov 18, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qwerty287 qwerty287 qwerty287 left review comments

@anbraten anbraten anbraten approved these changes

Assignees

No one assigned

Labels

refactor delete or replace old code server

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Imporove log streaming

4 participants

@LUKIEYF @qwerty287 @anbraten @xoxys