Skip to content

grid: adding logging#15778

Merged
alyssawilk merged 2 commits intoenvoyproxy:mainfrom
alyssawilk:http3_logging
Apr 1, 2021
Merged

grid: adding logging#15778
alyssawilk merged 2 commits intoenvoyproxy:mainfrom
alyssawilk:http3_logging

Conversation

@alyssawilk
Copy link
Copy Markdown
Contributor

Adding trace logs to the connectivity grid
Risk Level: n/a (adding an accessor to connection pools)
Testing: new unit tests
Part of #15649

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link
Copy Markdown
Contributor

@DavidSchinazi DavidSchinazi left a comment

Choose a reason for hiding this comment

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

I ❤️ logs

Callbacks& callbacks) PURE;

/**
* Returns a user-friendly protocol descipton for logging.
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.

Is protocol description the right thing here? if longer term we want to add cell vs Wi-Fi, should that be reflected? If that the case, should this be named description?

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.

My thought is we'd have separate strings for each, and combine in the pool, but we can rename if we end up going the other way.

random_generator, state, protocol),
codec_fn_(codec_fn), client_fn_(client_fn) {}
codec_fn_(codec_fn), client_fn_(client_fn), protocol_(protocol[0]) {
ASSERT(protocol.size() == 1);
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.

nit: I'm realizing that this predates this change, but should this parameter be called protocols since it's a vector?

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk alyssawilk merged commit a48bfb6 into envoyproxy:main Apr 1, 2021
@alyssawilk alyssawilk deleted the http3_logging branch August 18, 2021 13:59
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.

3 participants