Better USDT probes for ClickHouse protocol #8622
Conversation
This adds more observability into the network protocol used to talk to ClickHouse, in an attempt to help resolve #8595
| .await; | ||
| let elapsed = now.elapsed(); | ||
| probes::sql__query__done!(|| (&id)); | ||
| probes::sql__query__done!(|| id.to_string()); |
There was a problem hiding this comment.
that's interesting; maybe we should figure out how to to_string() things that implement it rather than doing serde (which is--I assume--why you're doing this).
There was a problem hiding this comment.
Yeah, that'd be sweet, especially for types like this where there's a canonical form or just one piece of data. The {"ok": "xxxx-yyyy-zzzz"} serialization form is annoying in that case.
| break Err(Error::UnexpectedPacket(kind)); | ||
| } | ||
| ServerPacket::Data(block) => { | ||
| probes::data__packet__received!(|| { |
There was a problem hiding this comment.
I'm confused; were we getting this twice or something?
There was a problem hiding this comment.
We were not. I removed this probe, subsuming it in the new packet-received probe that has the entire packet structure in it. This one is duplicative now.
| )); | ||
| Err(Error::UnexpectedPacket(packet.kind())) | ||
| } | ||
| Some(Err(e)) => Err(e), |
There was a problem hiding this comment.
are we interested in probes for this condition?
There was a problem hiding this comment.
The error condition? I think we get the invalid-packet probe firing in those cases, such as here. Does that seem like it covers it?
This adds more observability into the network protocol used to talk to ClickHouse, in an attempt to help resolve #8595