Adds more fields to connection stats #368
Conversation
| /** | ||
| * Start time of the connection. | ||
| */ | ||
| int64_t conn_start_at; |
There was a problem hiding this comment.
"conn_" seems excessive to me, as this field is part of quicly_conn_t. Maybe rename to created_at or something?
| if (epoch == QUICLY_EPOCH_INITIAL || epoch == QUICLY_EPOCH_HANDSHAKE) | ||
| ++conn->super.stats.num_packets.handshake_received; | ||
| if (epoch == QUICLY_EPOCH_0RTT) | ||
| ++conn->super.stats.num_packets.zerortt_received; |
There was a problem hiding this comment.
I'm not sure if this is the correct location for updating the metrics.
We update num_packets.received in quicly_accept and quicly_receive. Maybe we should change that to per-packet-type metric (i.e. uint64_t [QUICLY_NUM_EPOCHS]) and increment the counter of the corresponding epoch.
| @@ -1253,6 +1269,13 @@ static int record_receipt(quicly_conn_t *conn, struct st_quicly_pn_space_t *spac | |||
| conn->egress.send_ack_at = conn->stash.now + QUICLY_DELAYED_ACK_TIMEOUT; | |||
| } | |||
|
|
|||
| if (is_ack_only) | |||
| ++conn->super.stats.num_packets.ack_only_received; | |||
| if (get_epoch(*s->target.first_byte_at) == QUICLY_EPOCH_INITIAL || get_epoch(*s->target.first_byte_at) == QUICLY_EPOCH_HANDSHAKE) | ||
| ++conn->super.stats.num_packets.handshake_sent; | ||
| if (get_epoch(*s->target.first_byte_at) == QUICLY_EPOCH_0RTT) | ||
| ++conn->super.stats.num_packets.zerortt_sent; |
There was a problem hiding this comment.
As stated above, maybe it would be better to change the type of num_packets.sent to uint64_t [QUICLY_NUM_EPOCHS] and increment the per-epoch counter.
| @@ -3198,6 +3226,7 @@ int quicly_send_stream(quicly_stream_t *stream, quicly_send_context_t *s) | |||
| UpdateState: | |||
| QUICLY_PROBE(STREAM_SEND, stream->conn, stream->conn->stash.now, stream, off, end_off - off, is_fin); | |||
| QUICLY_PROBE(QUICTRACE_SEND_STREAM, stream->conn, stream->conn->stash.now, stream, off, end_off - off, is_fin); | |||
| stream->conn->super.stats.num_bytes.stream_sent += end_off - off; | |||
There was a problem hiding this comment.
This metric would cover both the ordinary streams and CRYPTO streams, as we do not check the sign of the stream-id. Are we fine with that? I do not have a strong opinion but would like to confirm.
| @@ -4278,6 +4307,7 @@ static int handle_stream_frame(quicly_conn_t *conn, struct st_quicly_handle_payl | |||
| if ((ret = quicly_decode_stream_frame(state->frame_type, &state->src, state->end, &frame)) != 0) | |||
| return ret; | |||
| QUICLY_PROBE(QUICTRACE_RECV_STREAM, conn, conn->stash.now, frame.stream_id, frame.offset, frame.data.len, (int)frame.is_fin); | |||
| conn->super.stats.num_bytes.stream_received += frame.data.len; | |||
No description provided.