Skip to content

Add param to echo captured client output in logs#22564

Merged
ericgribkoff merged 1 commit intogrpc:masterfrom
ericgribkoff:inline_logs
Apr 3, 2020
Merged

Add param to echo captured client output in logs#22564
ericgribkoff merged 1 commit intogrpc:masterfrom
ericgribkoff:inline_logs

Conversation

@ericgribkoff
Copy link
Copy Markdown
Contributor

Our internal staging tests cannot (easily) pick up the nicely outputted files produced by report_utils.render_junit_xml_report. This is a temporary-ish workaround for that to allow us to at least see the client output in any staging failures.

Also considered an argument to just disable capturing the output from the subprocess.Popen call and including them inline in the script's output, but went with this approach as it seems helpful to still have the separation (albeit still in stdout, but at least not jumbled together) between run_xds_test.py logs and the client logs. If that seems preferable let me know and I'll switch to that approach.

@ericgribkoff ericgribkoff added lang/other release notes: no Indicates if PR should not be in release notes labels Apr 3, 2020
@ericgribkoff ericgribkoff requested a review from gnossen April 3, 2020 00:32
Copy link
Copy Markdown
Contributor

@gnossen gnossen left a comment

Choose a reason for hiding this comment

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

My only concern would be if there weren't timestamps in the client logs. But I believe all clients currently do.

@ericgribkoff ericgribkoff merged commit b53a4b4 into grpc:master Apr 3, 2020
@ericgribkoff ericgribkoff deleted the inline_logs branch April 3, 2020 19:54
ericgribkoff added a commit to ericgribkoff/grpc that referenced this pull request Apr 3, 2020
Add param to echo captured client output in logs
ericgribkoff added a commit that referenced this pull request Apr 3, 2020
Merge pull request #22564 from ericgribkoff/inline_logs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lang/other release notes: no Indicates if PR should not be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants