[metrics] Add client metrics and prometheus example#617
Conversation
|
cc @fresh-borzoni @leekeiabstraction PTAL! 🙏 |
leekeiabstraction
left a comment
leekeiabstraction
left a comment
There was a problem hiding this comment.
TY for the PR. Left a question
leekeiabstraction
left a comment
leekeiabstraction
left a comment
There was a problem hiding this comment.
TY for clarifying. Left a couple more qs.
|
|
||
| let scan_records = log_scanner.poll(Duration::from_secs(1)).await?; | ||
| let mut count = 0; | ||
| for record in scan_records { |
There was a problem hiding this comment.
We are only verifying metrics for sent records, do we need the loop for polling records? If not, it might be worth removing this loop for a more concise example.
There was a problem hiding this comment.
we need poll still because all scanner metrics are recorded inside poll(), but you are right that the per-field decode loop drives no metrics, so i replaced it!
| The client caches metric handles the first time a writer or scanner is created, | ||
| binding them to whichever recorder is installed at that moment. Install your global | ||
| recorder **before** calling `FlussConnection::new` (ideally as the very first thing | ||
| in `main`). If you install it after creating a writer or scanner, those metrics will |
There was a problem hiding this comment.
This behaviour might catch user off guard. Is it worth making it so that an Error is returned instead?
There was a problem hiding this comment.
i think returning an error isn't workable because "no recorder installed" is a supported mode most users run in, so writer/scanner creation can't fail just because no recorder is present
Also metrics facade has no hook to detect "installed too late". The only alternative is to stop caching handles and re-resolve per record, and that will have hot-path cost so we should try to avoid.
wdyt? i can also some more explanation on that in this doc 🙏
leekeiabstraction
left a comment
leekeiabstraction
left a comment
There was a problem hiding this comment.
Ty for the PR and addressing comments/answering questions. LGTM!
2 participants
Purpose
Document the implemented connection/writer/scanner client metrics and show how to collect them with a Prometheus exporter.
Linked issue: close #618