Skip to content

feat: [metrics] Create metrics framework and add connection metrics#407

Open
charlesdong1991 wants to merge 8 commits intoapache:mainfrom
charlesdong1991:metrics-framework
Open

feat: [metrics] Create metrics framework and add connection metrics#407
charlesdong1991 wants to merge 8 commits intoapache:mainfrom
charlesdong1991:metrics-framework

Conversation

@charlesdong1991
Copy link
Contributor

Purpose

Linked issue: close #390

Brief change log

This PR adds a metrics framework and connection level metrics.

NOTE:
There is an parity from Java, which is the scopeguard for in-flight on cancellation, because Java callback model doesn't have equivalent to tokio future cancellation. And if a tokio future is dropped mid-execution, it would skip decrement and cause gauge drift. So i think it's important to add "scopeguard" to avoid that.

And I can have writer and scanner metrics as follow-up PRs.

Tests

All tests are passed locally.

Copy link
Contributor

@fresh-borzoni fresh-borzoni left a comment

Choose a reason for hiding this comment

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

@charlesdong1991 Ty for the PR, left some comments
PTAL

@charlesdong1991
Copy link
Contributor Author

Thanks for reviews, i made some changes and leave a question. PTAL @fresh-borzoni

Copy link
Contributor

@fresh-borzoni fresh-borzoni left a comment

Choose a reason for hiding this comment

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

@charlesdong1991 LGTM overall, small comment about misleading doc, and we can do simple math to match Java here, at least semantically. PTAL

@charlesdong1991
Copy link
Contributor Author

and we can do simple math to match Java here, at least semantically. PTAL

btw, just to clarify before making changes, do you mean to update code to fix header overhead? or to update the doc to reflect this? @fresh-borzoni

@fresh-borzoni
Copy link
Contributor

fresh-borzoni commented Mar 5, 2026

and we can do simple math to match Java here, at least semantically. PTAL

btw, just to clarify before making changes, do you mean to update code to fix header overhead? or to update the doc to reflect this? @fresh-borzoni

Sorry, I should have been more specific - I meant code @charlesdong1991

@charlesdong1991
Copy link
Contributor Author

Gotcha, thanks @fresh-borzoni will do a quick change, should be straightforward 👍

@charlesdong1991
Copy link
Contributor Author

i updated code to calculate total size "manually", PTAL @fresh-borzoni

thanks again!

@fresh-borzoni
Copy link
Contributor

@charlesdong1991 TY, LGTM

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.

Client metrics framework

2 participants