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
distinguish stdout and stderr in up
logs
#10070
Conversation
fa026a3
to
dffa53b
Compare
Codecov ReportBase: 75.74% // Head: 77.02% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## v2 #10070 +/- ##
==========================================
+ Coverage 75.74% 77.02% +1.27%
==========================================
Files 2 2
Lines 235 235
==========================================
+ Hits 178 181 +3
+ Misses 50 48 -2
+ Partials 7 6 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
afb91f8
to
285b0ec
Compare
pkg/api/api.go
Outdated
ContainerEventLog = iota | ||
// ContainerEventErr is a ContainerEvent of type log on stderr. Line is set | ||
ContainerEventErr = iota |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ContainerEventErr = iota | |
ContainerEventErr |
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
285b0ec
to
a0eccef
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arff I'm too late, wondering if removing the service name in the log won't break potential user tests? 😬
@@ -116,7 +115,7 @@ func (s *composeService) logContainers(ctx context.Context, consumer api.LogCons | |||
|
|||
name := getContainerNameWithoutProject(c) | |||
w := utils.GetWriter(func(line string) { | |||
consumer.Log(name, service, line) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really want to remove the reference to the service name in the logs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's actually unused (this is legacy from early ECS-CLI-plugin 😅
Does this really fix the issue? I'm using standalone version 2.14.2 and Ansible shows me that the logs of my
EDIT: Oh this was about the logs of the attached container. I may be wrong here with the compose output itself. |
@felixoi this is compose status message you see here. |
@ndeloof Yeah, saw this too late. Do you know if there is an existing issue for status messages, or is this wanted behaviour? |
That's intended behavior -- since there are only two streams (stdout/stderr), the CLI writes its own output to stderr; this ensures you can capture container output (from stdout) without having Compose output mixed in |
What I did
As we attach to containers to collect log, distinguish Stdout and Stderr, and print logs on os.Stdout/Stderr accordingly
Related issue
#fixes #8098
(not mandatory) A picture of a cute animal, if possible in relation to what you did