Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support HTTP/2 protocol. #138

Merged
merged 5 commits into from
May 29, 2018
Merged

Support HTTP/2 protocol. #138

merged 5 commits into from
May 29, 2018

Conversation

ujjboy
Copy link
Member

@ujjboy ujjboy commented May 26, 2018

Motivation:

Support HTTP/1.1 & HTTP/2 protocol.

Modification:

  • Add HTTP/1.1 server & HTTP/2 Clear text server
  • Add HTTP/2 client transport.

Result:

Fix #25

@ujjboy ujjboy added enhancement New feature or request extension Integrate with 3rd component labels May 26, 2018
@ujjboy ujjboy added this to the 5.4.0 milestone May 26, 2018
@codecov-io
Copy link

codecov-io commented May 26, 2018

Codecov Report

Merging #138 into master will increase coverage by 0.54%.
The diff coverage is 69.14%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #138      +/-   ##
============================================
+ Coverage     68.03%   68.58%   +0.54%     
- Complexity      714      944     +230     
============================================
  Files           304      339      +35     
  Lines         12743    14156    +1413     
  Branches       2088     2268     +180     
============================================
+ Hits           8670     9709    +1039     
- Misses         3026     3278     +252     
- Partials       1047     1169     +122
Impacted Files Coverage Δ Complexity Δ
.../java/com/alipay/sofa/rpc/common/RpcConstants.java 66.66% <ø> (ø) 0 <0> (ø) ⬇️
...ootstrap/http/Http2ClearTextProviderBootstrap.java 0% <0%> (ø) 0 <0> (?)
.../sofa/rpc/transport/http/Http1ServerTransport.java 100% <100%> (ø) 1 <1> (?)
...a/com/alipay/sofa/rpc/server/http/Http1Server.java 100% <100%> (ø) 1 <1> (?)
.../transport/http/Http2ClearTextClientTransport.java 100% <100%> (ø) 1 <1> (?)
.../com/alipay/sofa/rpc/context/RpcInvokeContext.java 69.62% <100%> (+5.44%) 0 <0> (ø) ⬇️
.../sofa/rpc/transport/http/Http2ClientTransport.java 100% <100%> (ø) 1 <1> (?)
.../transport/http/Http2ClearTextServerTransport.java 100% <100%> (ø) 1 <1> (?)
...ootstrap/http/Http2ClearTextConsumerBootstrap.java 100% <100%> (ø) 2 <2> (?)
...pay/sofa/rpc/server/http/Http2ClearTextServer.java 100% <100%> (ø) 1 <1> (?)
... and 80 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7190a92...523b030. Read the comment docs.

@leizhiyuan
Copy link
Contributor

leizhiyuan commented May 27, 2018

It seems tracer is not ok
client is ok

{"timestamp":"2018-05-27 18:43:48.681","tracerId":"0a4187b11527417826069100199533","spanId":"0","span.kind":"client","local.app":"test-client","protocol":"h2c","service":"com.alipay.sofa.rpc.protobuf.ProtoService","method":"echoObj","current.thread.name":"main","invoke.type":"sync","router.record":"DIRECT","remote.ip":"127.0.0.1:12300","local.client.ip":"127.0.0.1","result.code":"00","req.serialize.time":"1206","req.size":"5","client.elapse.time":"1844","local.client.port":"54560","baggage":""}

server lost tracerId and span Id, I will check

{"timestamp":"2018-05-27 18:43:47.253","tracerId":"","spanId":"","span.kind":"server","service":"com.alipay.sofa.rpc.protobuf.ProtoService","method":"echoObj","remote.ip":"127.0.0.1","remote.app":"test-client","protocol":"h2c","local.app":"test-server","current.thread.name":"SOFA-SEV-H2C-BIZ-12300-3-T1","result.code":"00","biz.impl.time":"3","baggage":""}

@leizhiyuan
Copy link
Contributor

leizhiyuan commented May 27, 2018

io.netty.handler.codec.http2.HttpConversionUtil#toHttp2Headers(io.netty.handler.codec.http.HttpHeaders, io.netty.handler.codec.http2.Http2Headers)

image

netty convert header to lowercase. when we receive in server.

rpc_trace_context.sofaRpcId will become rpc_trace_context.sofarpcid

this is a http2 sepc. we need to get lower case key when we resolve

com.alipay.sofa.rpc.transport.http.Http2ServerChannelHandler#parseHttp2Request
 // 解析trace信息
        Map<String, String> traceMap = new HashMap<String, String>(8);
        String prefix = RemotingConstants.RPC_TRACE_NAME + ".";
        Iterator<Map.Entry<CharSequence, CharSequence>> it = headers.iterator();
        while (it.hasNext()) {
            Map.Entry<CharSequence, CharSequence> entry = it.next();
            String key = entry.getKey().toString();
            if (key.startsWith(prefix)) {
                traceMap.put(key.substring(prefix.length()), StringUtils.toString(entry.getValue()));
                // it.remove(); readonly
            }
            if (!key.startsWith(":")) {
                sofaRequest.addRequestProp(key, StringUtils.toString(entry.getValue()));
            }
        }

maybe we need hold a convert relationship to fetch ?

String http2Key=key.substring(prefix.length());
traceMap.put(transMap.get(http2Key), StringUtils.toString(entry.getValue()));

`

@ujjboy
Copy link
Member Author

ujjboy commented May 27, 2018

@leizhiyuan I have fixed it. See com.alipay.sofa.rpc.transport.http.HttpTracerUtils.

@leizhiyuan
Copy link
Contributor

@ujjboy

{"timestamp":"2018-05-28 06:34:01.173","tracerId":"c0a8016c152746044116910138939","spanId":"0","span.kind":"server","service":"com.alipay.sofa.rpc.protobuf.ProtoService","method":"echoObj","remote.ip":"127.0.0.1","remote.app":"test-client","protocol":"h2c","local.app":"test-server","current.thread.name":"SOFA-SEV-H2C-BIZ-12300-3-T2","result.code":"00","biz.impl.time":"0","baggage":""}

now, tracer is ok

@ujjboy ujjboy merged commit 5573e28 into sofastack:master May 29, 2018
@ujjboy ujjboy deleted the support_http2 branch May 29, 2018 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request extension Integrate with 3rd component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants