Skip to content

Add local_end_stream_ to crash dump for H1#17367

Merged
alyssawilk merged 2 commits intoenvoyproxy:mainfrom
pradeepcrao:h1_dump
Jul 19, 2021
Merged

Add local_end_stream_ to crash dump for H1#17367
alyssawilk merged 2 commits intoenvoyproxy:mainfrom
pradeepcrao:h1_dump

Conversation

@pradeepcrao
Copy link
Copy Markdown
Contributor

Signed-off-by: Pradeep Rao pcrao@google.com

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: Pradeep Rao <pcrao@google.com>
!active_request_.value().request_url_.getStringView().empty()
? active_request_.value().request_url_.getStringView()
: "null");
os << DUMP_MEMBER_AS(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks so much for adding this extra state to the dump.

Instead of adding a second bunch of messy code, what do you think of having a dumpState function for the active request, and then just calling DUMP_DETAILS(active_request_)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion Alyssa, I will implement this.

@alyssawilk alyssawilk self-assigned this Jul 15, 2021
Signed-off-by: Pradeep Rao <pcrao@google.com>
Copy link
Copy Markdown
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

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

Yeah, looks great!
the only problem is coverage flaked. Let me see if I can get a clean run to merge
/retest

@repokitteh-read-only
Copy link
Copy Markdown

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

🐱

Caused by: a #17367 (review) was submitted by @alyssawilk.

see: more, trace.

@pradeepcrao
Copy link
Copy Markdown
Contributor Author

Thanks so much Alyssa, hope it passes on rerun.

@alyssawilk alyssawilk merged commit a1705b4 into envoyproxy:main Jul 19, 2021
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
Risk Level: low
Testing: unit test improvements
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Pradeep Rao <pcrao@google.com>
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.

2 participants