question: order of stream handlers

[dvic]

I noticed that when compress is set to true, the order of the stream handlers is: [:cowboy_compress_h | @default_stream_handlers], where @default_stream_handlers is [:cowboy_telemetry_h, :cowboy_stream_h].

Does this work as I understand it in that the telemetry does not take into account the compression handler? Is this working as intended or should the compression be taken into account? I can imagine this is a breaking change but we could put it behind a flag or support compress: [metrics: true] or something like that.

[josevalim]

Good catch. I think we can simply have two sets of default handlers, one for compressed and another for non compressed. Would that work?

[dvic]

yes for me it would :) and then just keep the current compress: boolean() as an option? i’m not sure if this is considered a breaking change (as the metric meaning will change)

[josevalim]

The only metric meaning that will change is that will measure more, right?! If so, that’s ok. /cc @binaryseed

[dvic]

The only metric meaning that will change is that will measure more, right?! If so, that’s ok. /cc @binaryseed

correct

[binaryseed]

The way cowboy_telemetry works is by wrapping cowboy_metrics_h. The metrics stream handler marks its ending measurement in the terminate cowboy stream handler callback. Since cowboy_compress_h does all its compression work in the data stream handler callback, I believe that the compression time is already included in the overall duration reported.

https://github.com/ninenines/cowboy/blob/8795233c57f1f472781a22ffbf186ce38cc5b049/src/cowboy_metrics_h.erl#L273-L303

https://github.com/ninenines/cowboy/blob/8795233c57f1f472781a22ffbf186ce38cc5b049/src/cowboy_compress_h.erl#L45-L47

[josevalim]

Thanks @binaryseed !

[dvic]

That clears it up, thanks!