Skip to content

[Windows] Improve SCM error logging#15638

Merged
mattklein123 merged 10 commits intoenvoyproxy:mainfrom
davinci26:scmEventLogErrors
Apr 6, 2021
Merged

[Windows] Improve SCM error logging#15638
mattklein123 merged 10 commits intoenvoyproxy:mainfrom
davinci26:scmEventLogErrors

Conversation

@davinci26
Copy link
Copy Markdown
Member

Signed-off-by: Sotiris Nanopoulos sonanopo@microsoft.com

Commit Message:

Envoy registers with SCM before it parses the command line and as a result we don't have access to loggers. Additionally SCM by default captures the stdout/stderr so we can't really write to the standard streams. As a result debugging startup failures in SCM is a bit tricky.

To solve this I used the spdlog::sinks::win_eventlog_sink_mt which writes to the Windows Event Viewer. We use this logger directly for failures during SCM initialization.

Additional Description: N/A
Risk Level: Low
Testing: Manual
Docs Changes: Done
Release Notes: N/A
Platform Specific Features: Windows Only

Signed-off-by: davinci26 <sotirisnan@gmail.com>
Signed-off-by: davinci26 <sotirisnan@gmail.com>
@davinci26
Copy link
Copy Markdown
Member Author

@wrowe PTAL

Signed-off-by: davinci26 <sotirisnan@gmail.com>
Signed-off-by: davinci26 <sotirisnan@gmail.com>
Signed-off-by: davinci26 <sotirisnan@gmail.com>
Signed-off-by: davinci26 <sotirisnan@gmail.com>
Signed-off-by: davinci26 <sotirisnan@gmail.com>
Signed-off-by: davinci26 <sotirisnan@gmail.com>
@davinci26
Copy link
Copy Markdown
Member Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #15638 (comment) was created by @davinci26.

see: more, trace.

@davinci26
Copy link
Copy Markdown
Member Author

@wrowe lets get it merged before 1.18 since it is an already experimental feature. I look for your comments

wrowe
wrowe previously approved these changes Apr 1, 2021
Copy link
Copy Markdown
Contributor

@wrowe wrowe left a comment

Choose a reason for hiding this comment

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

This looks good to me, ready for a pass by @envoyproxy/senior-maintainers specific to logging architecture. Since this gets the error text of well-formed exceptions into the event log for troubleshooting, it's a good step forward.

@wrowe wrowe self-requested a review April 1, 2021 21:54
@mattklein123 mattklein123 self-assigned this Apr 2, 2021
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks LGTM with small comments.

/wait

@davinci26
Copy link
Copy Markdown
Member Author

@mattklein123 sorry for the delayed response, I was on PTO. Ack the comments and I will address them today

Signed-off-by: davinci26 <sotirisnan@gmail.com>
@davinci26
Copy link
Copy Markdown
Member Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #15638 (comment) was created by @davinci26.

see: more, trace.

@davinci26
Copy link
Copy Markdown
Member Author

Unrelated flake in coverage

Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks!

@mattklein123 mattklein123 merged commit 641d115 into envoyproxy:main Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants