stats: set response compression codec on stats.InHeader and stats.OutHeader

[MatthewDolan]

Currently, the request compression is recorded (on stats.OutHeader client-side and stats.InHeader server-side), but the response compression is never recorded. Although these compression codecs are often the same, the gRPC compression specification (https://github.com/grpc/grpc/blob/master/doc/compression.md) states that it's valid for them to be different.

• The pull request captures the response compression (on stats.InHeader client-side and stats.OutHeader server-side).
• This pull request moves the two fields (Compression and Header) above the comment The following fields are valid only if Client is true. because they are now both valid on the server-side and client-side.

[menghanl]

Thanks for the fix. Can you change the tests to cover the new field as well? The following diff should work.

Also, I think you might have missed http2_server.go. The test hopefully should catch that.

Thanks!

diff --git a/stats/stats_test.go b/stats/stats_test.go
index ff16c182..d047d48b 100644
--- a/stats/stats_test.go
+++ b/stats/stats_test.go
@@ -446,6 +446,9 @@ func checkInHeader(t *testing.T, d *gotData, e *expectedData) {
        if d.ctx == nil {
                t.Fatalf("d.ctx = nil, want <non-nil>")
        }
+       if st.Compression != e.compression {
+               t.Fatalf("st.Compression = %v, want %v", st.Compression, e.compression)
+       }
        if d.client {
                // additional headers might be injected so instead of testing equality, test that all the
                // expected headers keys have the expected header values.
@@ -461,9 +464,6 @@ func checkInHeader(t *testing.T, d *gotData, e *expectedData) {
                if st.LocalAddr.String() != e.serverAddr {
                        t.Fatalf("st.LocalAddr = %v, want %v", st.LocalAddr, e.serverAddr)
                }
-               if st.Compression != e.compression {
-                       t.Fatalf("st.Compression = %v, want %v", st.Compression, e.compression)
-               }
                // additional headers might be injected so instead of testing equality, test that all the
                // expected headers keys have the expected header values.
                for key := range testMetadata {
@@ -575,6 +575,9 @@ func checkOutHeader(t *testing.T, d *gotData, e *expectedData) {
        if d.ctx == nil {
                t.Fatalf("d.ctx = nil, want <non-nil>")
        }
+       if st.Compression != e.compression {
+               t.Fatalf("st.Compression = %v, want %v", st.Compression, e.compression)
+       }
        if d.client {
                if st.FullMethod != e.method {
                        t.Fatalf("st.FullMethod = %s, want %v", st.FullMethod, e.method)
@@ -582,9 +585,6 @@ func checkOutHeader(t *testing.T, d *gotData, e *expectedData) {
                if st.RemoteAddr.String() != e.serverAddr {
                        t.Fatalf("st.RemoteAddr = %v, want %v", st.RemoteAddr, e.serverAddr)
                }
-               if st.Compression != e.compression {
-                       t.Fatalf("st.Compression = %v, want %v", st.Compression, e.compression)
-               }
                // additional headers might be injected so instead of testing equality, test that all the
                // expected headers keys have the expected header values.
                for key := range testMetadata {

[MatthewDolan]

Thank you for the feedback, @menghanl.

You were right about the tests and missed case. I have updated both, rebased on latest, and updated the pull request.

[menghanl]

LGTM. Thanks for the changes!