http: dumping session state on the decode path #7390
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| #pragma once | ||
|
|
||
| #include <ostream> | ||
|
|
||
| #include "envoy/common/pure.h" | ||
|
|
||
| namespace Envoy { | ||
|
|
||
| /* | ||
| * A class for tracking the scope of work. | ||
| * Currently this is only used for best-effort tracking the any L7 stream doing | ||
| * work if a crash occurs. | ||
| */ | ||
| class ScopeTrackedObject { | ||
| public: | ||
| virtual ~ScopeTrackedObject() {} | ||
|
|
||
| /** | ||
| * Dump debug state of the object in question to the provided ostream | ||
| * | ||
| * @param os the ostream to output to. | ||
| * @param indent_level how far to indent, for pretty-printed classes and subclasses. | ||
| */ | ||
| virtual void logState(std::ostream& os, int indent_level = 0) const PURE; | ||
htuch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }; | ||
|
|
||
| } // namespace Envoy | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| #pragma once | ||
|
|
||
| #include <sstream> | ||
|
|
||
| namespace Envoy { | ||
|
|
||
| #define LOG_MEMBER(member) ", " #member ": " << (member) | ||
|
|
||
| #define LOG_OPTIONAL_MEMBER(member) \ | ||
| ", " #member ": " << (member.has_value() ? absl::StrCat(member.value()) : "null") | ||
|
|
||
| // Macro assumes local member variables | ||
| // os (ostream) | ||
| // indent_level (int) | ||
| #define LOG_DETAILS(member) \ | ||
| do { \ | ||
| os << spaces << #member ": "; \ | ||
| if (member != nullptr) { \ | ||
| os << "\n"; \ | ||
| (member)->logState(os, indent_level + 1); \ | ||
|
There was a problem hiding this comment. I'm not a huge fan of this style of printing and indenting TBH. I get that you want to make this efficient and have no allocations, but it's leaking through above into a duck typing of a pure interface (the header map), which seems like a pattern best avoided across if it's going to appear in more places. There was a problem hiding this comment. Yeah, to your point I think there is value in having a structured data print type, and I can think of several use cases where it'd be valuable internally. From (possibly outdated?) priors I believe there is also value in a minimal allocation state drop, which is what I planned on implementing first. I think for out-process communication and machine-consumption we'll eventually want to tackle the latter, but I don't think we'd want to use that version in the fatal error handler and I'd like the "core dumps are hard" session state to be human readable where possible. There was a problem hiding this comment. This is a fair point. I think it would be useful to document (i.e. provide the citations) on how allocations during fatal handling are a problem. The main one I can imagine is that we're in a soft OOM situation, and allocation would lead to a hard OOM, would be good to have some more intuition on this one. |
||
| } else { \ | ||
| os << spaces << "null\n"; \ | ||
| } \ | ||
| } while (false) | ||
|
|
||
| // Return the const char* equivalent of string(level*2, ' '), without dealing | ||
| // with string creation overhead. Cap arbitrarily at 6 as we're (hopefully) | ||
| // not going to have nested objects deeper than that. | ||
| inline const char* spacesForLevel(int level) { | ||
htuch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| switch (level) { | ||
| case 0: | ||
| return ""; | ||
| case 1: | ||
| return " "; | ||
| case 2: | ||
| return " "; | ||
| case 3: | ||
| return " "; | ||
| case 4: | ||
| return " "; | ||
| case 5: | ||
| return " "; | ||
| default: | ||
| return " "; | ||
| } | ||
| return ""; | ||
| } | ||
|
|
||
| } // namespace Envoy | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| #pragma once | ||
|
|
||
| #include "envoy/common/scope_tracker.h" | ||
| #include "envoy/event/dispatcher.h" | ||
|
|
||
| namespace Envoy { | ||
|
|
||
| class ScopeTrackerImpl { | ||
|
There was a problem hiding this comment. nit: I might call this There was a problem hiding this comment. I think this name is better future-proofed for when we use the scope object for customer memory/cpu accounting (the htuch@ feature) - WDYT? There was a problem hiding this comment. I don't feel strongly about it, other than to say that There was a problem hiding this comment. (I guess a small class comment on what it is for would be nice either way) |
||
| public: | ||
| ScopeTrackerImpl(const ScopeTrackedObject* object, Event::Dispatcher& dispatcher) | ||
| : dispatcher_(dispatcher) { | ||
| latched_object_ = dispatcher_.setTrackedObject(object); | ||
| } | ||
|
|
||
| ~ScopeTrackerImpl() { dispatcher_.setTrackedObject(latched_object_); } | ||
|
|
||
| private: | ||
| const ScopeTrackedObject* latched_object_; | ||
| Event::Dispatcher& dispatcher_; | ||
| }; | ||
|
|
||
| } // namespace Envoy | ||
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.
s/the any/any ?