[PATCH 03 of 14] HTTP/2: add debug logging of control frames

[Piotr Sikora]

Hey Valentin,
comments below. I commented only on the first occurrence, but
obviously those comments apply to all similar changes.

>          ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
> -                       "http2 SETTINGS frame ack:1");
> +                       "http2 SETTINGS ACK frame");

I don't particularly like this change, since it makes it look like
this is a "SETTINGS ACK" frame, not a "SETTINGS" frame with ACK flag
set. This is even more confusing since QUIC drafts had "SETTINGS_ACK"
frame at some point.

Maybe "http2 SETTINGS frame ACK" would be more acceptable?

>      ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
> -                   "http2 SETTINGS frame params:%uz",
> +                   "http2 SETTINGS frame with %uz params",
>                     h2c->state.length / NGX_HTTP_V2_SETTINGS_PARAM_SIZE);

The extra "with" doesn't add any information, and it doesn't seem to
be inline with other debug logs, but I don't mind this change too
much.

>              ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
> -                           "http2 SETTINGS param HEADER_TABLE_SIZE:%ui "
> +                           "http2 SETTINGS param HEADER_TABLE_SIZE: %ui "
>                             "(ignored)", value);

This is fine with me, but again, the extra space doesn't seem to be
inline with other debug logs, which usually don't include space
between "key:" and "value".

>              ngx_log_debug2(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
> -                           "http2 SETTINGS param 0x%Xi:%ui "
> -                           "(ignored)", id, value);
> +                           "http2 unknown SETTINGS param id:0x%Xi value:%ui",
> +                           id, value);

I'd prefer if we could stick to the "http2 SETTINGS" prefix here,
otherwise it won't look like part of the same frame, i.e.:

    http2 SETTINGS frame with 3 params
    http2 SETTINGS param HEADER_TABLE_SIZE: 4096
    http2 SETTINGS param INITIAL_WINDOW_SIZE: 65536
    http2 unknown SETTINGS param id:0xAA value:1234

Also, " (ignored)" part was removed, which makes it inconsistent with
other ignored parameters.

Maybe "http2 SETTINGS unknown param id:0x%Xi value:%ui (ignored)" or
"http2 SETTINGS param id:0x%Xi value:%ui (unknown, ignored)"?

Thanks!

Best regards,
Piotr Sikora