Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement number of HTTP frames metric #3044

Closed
wants to merge 3 commits into from

Conversation

nerodono
Copy link

Implements metric described in linkerd/linkerd2#12319

@nerodono nerodono requested a review from a team as a code owner June 28, 2024 13:20
@nerodono nerodono changed the title Feat/number of frames metric Implement number of HTTP frames metric Jun 28, 2024
@loyd
Copy link

loyd commented Jul 1, 2024

@nerodono, it seems some test can be added to linkerd/app/integration/src/tests/telemetry.rs

@nerodono nerodono force-pushed the feat/number-of-frames-metric branch from 3467567 to 5863a73 Compare July 1, 2024 13:08
@loyd
Copy link

loyd commented Jul 8, 2024

@olix0r, could you look at this PR?

@olix0r
Copy link
Member

olix0r commented Jul 18, 2024

Thanks for submitting this. There is work in flight (planned to merge soon) that will add route metrics. My preference would be to add this functionality here so that we can include parent/route/backend labels on these metrics. I'll update this PR when that is available.

@loyd
Copy link

loyd commented Aug 30, 2024

There is work in flight (planned to merge soon) that will add route metrics

Is there any news regarding this? I really don't want to live on a fork =(

@olix0r
Copy link
Member

olix0r commented Aug 30, 2024

The first version of it has merged but it's not in a place where the frame metrics are really setup. We, coincidentally, had some reports of issues related to applications sending many very small frames, and so I'm planning to introduce a per-route data_size histogram so we can understand both the count of data frames transiting the proxy as well as a rough distribution of their sizes. This work was slightly delayed by some travel, etc. (so please excuse the silence), but it is actively being worked on as of this week. I'll update here when there is something to show.

@loyd
Copy link

loyd commented Aug 31, 2024

per-route data_size histogram

It sounds very useful, actually! Which GH issue is related to this work?

@jazvit
Copy link

jazvit commented Nov 5, 2024

@olix0r, Hi there! Any updates? I'm eagerly looking forward to the chance to start using the metrics!

@jazvit
Copy link

jazvit commented Nov 18, 2024

Hi @cratelyn and @olix0r,

I noticed this pull request has been open for a few months without any updates. Given that active development is ongoing in the repository, could you please take a look and provide feedback or consider merging it? It seems like it could benefit the project.

Thank you for your attention!

@cratelyn
Copy link
Collaborator

Hi @cratelyn and @olix0r,

I noticed this pull request has been open for a few months without any
updates. Given that active development is ongoing in the repository, could
you please take a look and provide feedback or consider merging it? It seems
like it could benefit the project.

Thank you for your attention!

hi there @jazvit!

the metrics that @olix0r should be landing shortly, and included in the next
edge release. #3308 has landed and introduces a family of histograms labeled
by backend to track response body frames. this can be used to measure the
number of frames and the total number of bytes yielded, as well as a coarse
distribution of the size of response body frames.

#3334 should be up for review shortly, which introduces an equivalent
route-level histogram for request bodies.

thank you for patience while we've gotten these implemented. 🙂

@olix0r
Copy link
Member

olix0r commented Nov 27, 2024

This feature is implemented in recent edge releases (edge-24.11.6+)! Thanks for your patience.

@olix0r olix0r closed this Nov 27, 2024
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.

5 participants