-
Notifications
You must be signed in to change notification settings - Fork 600
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
feat(metrics): Add Fragment Level Exchange Metrics #3696
Changes from all commits
7444c9c
8cfe606
8241831
7683aa1
b74968f
2549902
f2166b2
0630c76
8abd4b2
ecd4aa1
993f047
ea1624d
18cdda8
6ed19fc
de05386
8b3953e
582a574
6338a88
eaa5653
98ade72
4e162f6
8af4137
8432355
8395848
ade896f
527c6bd
28f54eb
1b3feb5
ace70a0
0e51b8d
3dc3e16
5d82776
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ use prometheus::{register_int_counter_vec_with_registry, Registry}; | |
pub struct ExchangeServiceMetrics { | ||
pub registry: Registry, | ||
pub stream_exchange_bytes: GenericCounterVec<AtomicU64>, | ||
pub stream_fragment_exchange_bytes: GenericCounterVec<AtomicU64>, | ||
pub actor_sampled_serialize_duration_ns: GenericCounterVec<AtomicU64>, | ||
} | ||
|
||
|
@@ -31,6 +32,14 @@ impl ExchangeServiceMetrics { | |
) | ||
.unwrap(); | ||
|
||
let stream_fragment_exchange_bytes = register_int_counter_vec_with_registry!( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why send and recv metrics are defined in two places? This would make things a little bit confusing... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I follow the PR about the actor exchange, the exchange and receive are defined in two places. |
||
"stream_exchange_frag_send_size", | ||
"Total size of messages that have been send to downstream Fragment", | ||
&["up_fragment_id", "down_fragment_id"], | ||
registry | ||
) | ||
.unwrap(); | ||
|
||
let actor_sampled_serialize_duration_ns = register_int_counter_vec_with_registry!( | ||
"actor_sampled_serialize_duration_ns", | ||
"Duration (ns) of sampled chunk serialization", | ||
|
@@ -42,6 +51,7 @@ impl ExchangeServiceMetrics { | |
Self { | ||
registry, | ||
stream_exchange_bytes, | ||
stream_fragment_exchange_bytes, | ||
actor_sampled_serialize_duration_ns, | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So previously we are getting things wrong here? We should use actor_id instead of fragment_id for
GetStream
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes correct, I did ask Martin before and He say the fragment refer to actor.