Skip to content

http3: stats and stats docs.#16600

Merged
alyssawilk merged 2 commits intoenvoyproxy:mainfrom
alyssawilk:test_todo
May 25, 2021
Merged

http3: stats and stats docs.#16600
alyssawilk merged 2 commits intoenvoyproxy:mainfrom
alyssawilk:test_todo

Conversation

@alyssawilk
Copy link
Copy Markdown
Contributor

Adding HTTP/3 stream reset stats
Removing a bunch of unused HTTP/3 stats.
Doccing up the stats which exist and adding tests.

While I'm in there, adding scope strings to the test client and test server, so it's easier to differentiate which stats are Envoy's vs test code.
Also changing wait for counter calls to not do infinite waits.

Risk Level: Low (http/3 stats addition)
Testing: new integration tests
Docs Changes: documenting HTT/3 stats
Release Notes: n/a

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

/wait


/**
* All stats for the HTTP/3 codec. @see stats_macros.h
* TODO(danzh) populate all of them in codec.
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.

Do we have to remove these? I planned to populate them gradually.

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.

I don't think leaving them in the code base while they're not being used makes sense, but I think it'd be fine to add them back (and doc them up) as they're implemented.

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

@antoniovicente antoniovicente left a comment

Choose a reason for hiding this comment

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

Thanks for the improvements to stats and docs.

`downstream_rq_active` gauge due to differences in stream accounting between the codec and the
HTTP connection manager.

Http3 codec statistics
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.

cc @yanavlasov @danzh2010

I think that the difference in HTTP2 and HTTP3 stats point at missing functionality in the HTTP3 codec. For example, no stats for header_overflow, inbound_window_update_frames_flood, keepalive_timeout and streams_active hint at lower level of hardening options or lack of stats to observe the behavior of those protections.

Worth thinking which of these are relevant and how to track their absense.

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.

@alyssawilk alyssawilk merged commit e9a149e into envoyproxy:main May 25, 2021
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
Adding HTTP/3 stream reset stats
Removing a bunch of unused HTTP/3 stats.
Doccing up the stats which exist and adding tests.

While I'm in there, adding scope strings to the test client and test server, so it's easier to differentiate which stats are Envoy's vs test code.
Also changing wait for counter calls to not do infinite waits.

Risk Level: Low (http/3 stats addition)
Testing: new integration tests
Docs Changes: documenting HTT/3 stats
Release Notes: n/a
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk alyssawilk deleted the test_todo branch August 4, 2022 00:56
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