Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

container logs not sent line by line is broken #10925

Merged
merged 2 commits into from
Aug 23, 2023
Merged

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Aug 23, 2023

we can't assume we receive container logs line by line. Some framework won't buffer output and will send char by char, and we also can receive looong lines which get buffered to 32kb and then cut into multiple logs.

This assumes we will catch container streams being closed before we receive a die event for container, which could be subject to race condition, but at least the impact here is minimal and the fix works for reproduction examples provided in linked issues.

Related issue
fixes #10917
fixes #10658
https://docker.atlassian.net/browse/ENV-246

(not mandatory) A picture of a cute animal, if possible in relation to what you did

@codecov
Copy link

codecov bot commented Aug 23, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.02% ⚠️

Comparison is base (c79f67f) 58.22% compared to head (746bab6) 58.20%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10925      +/-   ##
==========================================
- Coverage   58.22%   58.20%   -0.02%     
==========================================
  Files         121      121              
  Lines       10693    10689       -4     
==========================================
- Hits         6226     6222       -4     
+ Misses       3856     3854       -2     
- Partials      611      613       +2     
Files Changed Coverage Δ
pkg/compose/attach.go 61.15% <100.00%> (+0.65%) ⬆️
pkg/utils/writer.go 80.00% <100.00%> (+12.25%) ⬆️

... and 4 files with indirect coverage changes

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

@ndeloof ndeloof requested review from a team, nicksieger, StefanScherer, ulyssessouza, glours, milas and laurazard and removed request for a team August 23, 2023 12:45
@milas milas merged commit 6204fb1 into docker:main Aug 23, 2023
25 checks passed
@ndeloof ndeloof deleted the line_logger branch August 23, 2023 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Compose up logger breaks lines at 32768 characters [BUG] drops last line of container log on exit
2 participants