You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Yunkai Zhang <yu...@gmail.com> on 2014/03/04 09:15:25 UTC

[API Review] Introduce a new API: TSHttpConnectWithProtoType()

I have opened a ticket(TS-2610<https://issues.apache.org/jira/browse/TS-2610>)
that add %<cqpt>(client_req_proto_type) field into LogFormat, so that we
can distinguish the protocol type of each log records.

But for plugins that using TSHttpConnect() API to do request, the Logging
module can't know which protocol type is used, so I add a new API:
      TSHttpConnectWithProtoType(struct sockaddr const* addr,
TSNetProtoType proto_type);

After introducing TSHttpConnectWithProtoType() API, TSHttpConnect() API
would be a special case of it:
      TSVConn
      TSHttpConnect(sockaddr const* addr)
      {
        return TSHttpConnectWithProtoType(addr, TS_NET_PROTO_HTTP);
      }

The old plugins needn't to change anything if they do not care %<cqpt>
field, so the new API wouldn't cause compatibility issue.

-- 
Yunkai Zhang
Work at Taobao

Re: [API Review] Introduce a new API: TSHttpConnectWithProtoType()

Posted by Yunkai Zhang <yu...@gmail.com>.
This API was moved to a new separated
ticket(TS-2612<https://issues.apache.org/jira/browse/TS-2612>
).


On Tue, Mar 4, 2014 at 4:15 PM, Yunkai Zhang <yu...@gmail.com> wrote:

> I have opened a ticket(TS-2610<https://issues.apache.org/jira/browse/TS-2610>)
> that add %<cqpt>(client_req_proto_type) field into LogFormat, so that we
> can distinguish the protocol type of each log records.
>
> But for plugins that using TSHttpConnect() API to do request, the Logging
> module can't know which protocol type is used, so I add a new API:
>       TSHttpConnectWithProtoType(struct sockaddr const* addr,
> TSNetProtoType proto_type);
>
> After introducing TSHttpConnectWithProtoType() API, TSHttpConnect() API
> would be a special case of it:
>       TSVConn
>       TSHttpConnect(sockaddr const* addr)
>       {
>         return TSHttpConnectWithProtoType(addr, TS_NET_PROTO_HTTP);
>       }
>
> The old plugins needn't to change anything if they do not care %<cqpt>
> field, so the new API wouldn't cause compatibility issue.
>
> --
> Yunkai Zhang
> Work at Taobao
>



-- 
Yunkai Zhang
Work at Taobao

Re: [API Review] Introduce a new API: TSHttpConnectWithProtoType()

Posted by Yunkai Zhang <yu...@gmail.com>.
Maybe rename "ClientProtoType" to "ClientProtocolStack" would be more
descriptive.


On Fri, Mar 7, 2014 at 10:16 AM, Yunkai Zhang <yu...@gmail.com> wrote:

>
>
>
> On Fri, Mar 7, 2014 at 7:37 AM, Leif Hedstrom <zw...@apache.org> wrote:
>
>>
>> On Mar 6, 2014, at 7:25 PM, Alan M. Carroll <am...@network-geographics.com>
>> wrote:
>>
>> > Perhaps "scheme" for the HTTP vs. HTTPS vs. SPDY - I think that's what
>> the internal use. "Proto" and "Protocol" are somewhat vague.
>>
>> "scheme" is certainly the correct nomenclature according to e.g. RFC3986.
>>
>
> For HTTP/HTTPS/SPDY, I agree that "scheme" would be better.
>
> But here, we want to describe *protocol stack*, so HTTPS == TLS+HTTP, just
> as James mentioned above, using *protocol stack* can avoid unnecessary the
> risk of combinatorial explosion.
>
>
>
>
>>
>> -- leif
>>
>>
>
>
> --
> Yunkai Zhang
> Work at Taobao
>



-- 
Yunkai Zhang
Work at Taobao

Re: [API Review] Introduce a new API: TSHttpConnectWithProtoType()

Posted by Yunkai Zhang <yu...@gmail.com>.
On Fri, Mar 7, 2014 at 7:37 AM, Leif Hedstrom <zw...@apache.org> wrote:

>
> On Mar 6, 2014, at 7:25 PM, Alan M. Carroll <am...@network-geographics.com>
> wrote:
>
> > Perhaps "scheme" for the HTTP vs. HTTPS vs. SPDY - I think that's what
> the internal use. "Proto" and "Protocol" are somewhat vague.
>
> "scheme" is certainly the correct nomenclature according to e.g. RFC3986.
>

For HTTP/HTTPS/SPDY, I agree that "scheme" would be better.

But here, we want to describe *protocol stack*, so HTTPS == TLS+HTTP, just
as James mentioned above, using *protocol stack* can avoid unnecessary the
risk of combinatorial explosion.




>
> -- leif
>
>


-- 
Yunkai Zhang
Work at Taobao

Re: [API Review] Introduce a new API: TSHttpConnectWithProtoType()

Posted by Leif Hedstrom <zw...@apache.org>.
On Mar 6, 2014, at 7:25 PM, Alan M. Carroll <am...@network-geographics.com> wrote:

> Perhaps "scheme" for the HTTP vs. HTTPS vs. SPDY - I think that's what the internal use. "Proto" and "Protocol" are somewhat vague.

“scheme” is certainly the correct nomenclature according to e.g. RFC3986.

— leif


Re: [API Review] Introduce a new API: TSHttpConnectWithProtoType()

Posted by "Alan M. Carroll" <am...@network-geographics.com>.
Perhaps "scheme" for the HTTP vs. HTTPS vs. SPDY - I think that's what the internal use. "Proto" and "Protocol" are somewhat vague.

Thursday, March 6, 2014, 12:01:55 AM, you wrote:

>> > But for plugins that using TSHttpConnect() API to do request, the Logging
>> > module can't know which protocol type is used, so I add a new API:
>> >      TSHttpConnectWithProtoType(struct sockaddr const* addr,
>> > TSNetProtoType proto_type);


Re: [API Review] Introduce a new API: TSHttpConnectWithProtoType()

Posted by Yunkai Zhang <yu...@gmail.com>.
On Thu, Mar 6, 2014 at 3:17 AM, James Peach <jp...@apache.org> wrote:

> On Mar 4, 2014, at 12:15 AM, Yunkai Zhang <yu...@gmail.com> wrote:
>
> > I have opened a ticket(TS-2610<
> https://issues.apache.org/jira/browse/TS-2610>)
> > that add %<cqpt>(client_req_proto_type) field into LogFormat, so that we
> > can distinguish the protocol type of each log records.
> >
> > But for plugins that using TSHttpConnect() API to do request, the Logging
> > module can't know which protocol type is used, so I add a new API:
> >      TSHttpConnectWithProtoType(struct sockaddr const* addr,
> > TSNetProtoType proto_type);
> >
> > After introducing TSHttpConnectWithProtoType() API, TSHttpConnect() API
> > would be a special case of it:
> >      TSVConn
> >      TSHttpConnect(sockaddr const* addr)
> >      {
> >        return TSHttpConnectWithProtoType(addr, TS_NET_PROTO_HTTP);
> >      }
> >
> > The old plugins needn't to change anything if they do not care %<cqpt>
> > field, so the new API wouldn't cause compatibility issue.
>
> I think the TSNetProtoType name is confusing, it definitely confused me
> for a long time :) The purpose of this is to track the original protocol
> stack that we are doing work on behalf of, so I think that a name similar
> to TSClientProtoType would be clearer.
>
> This way of tracking the client protocol stack has the risk of
> combinatorial explosion. I think that we should separate out the transport
> and application protocols to make it easier to express TCP+HTTP, TLS+HTTP,
> TCP+WEBSOCKET, TLS+WEBSOCKET, TCP+SPDY, TLS+SPDY, etc.


Nice!


>
> Finally, it seems that this is a property of the virtual circuit, so
> perhaps a better approach would be an API that can get and set the bitmask
> on a TSVConn?
>

Yes, just as *proto_type* in the patch is a member variable of
NetVConnection, if we have two or more property, bitmask is suitable.

I'll give new patch(es) with the following changes:

1) Add a member variable in NetVConnection to hold bitmask.
2) Separate out the transport and application protocols in bitmask.
3) Rename *TSNetProtoType* to *TSClientProtoType*, and reconsider the
naming of elemets of it.



> J
>



-- 
Yunkai Zhang
Work at Taobao

Re: [API Review] Introduce a new API: TSHttpConnectWithProtoType()

Posted by James Peach <jp...@apache.org>.
On Mar 4, 2014, at 12:15 AM, Yunkai Zhang <yu...@gmail.com> wrote:

> I have opened a ticket(TS-2610<https://issues.apache.org/jira/browse/TS-2610>)
> that add %<cqpt>(client_req_proto_type) field into LogFormat, so that we
> can distinguish the protocol type of each log records.
> 
> But for plugins that using TSHttpConnect() API to do request, the Logging
> module can't know which protocol type is used, so I add a new API:
>      TSHttpConnectWithProtoType(struct sockaddr const* addr,
> TSNetProtoType proto_type);
> 
> After introducing TSHttpConnectWithProtoType() API, TSHttpConnect() API
> would be a special case of it:
>      TSVConn
>      TSHttpConnect(sockaddr const* addr)
>      {
>        return TSHttpConnectWithProtoType(addr, TS_NET_PROTO_HTTP);
>      }
> 
> The old plugins needn't to change anything if they do not care %<cqpt>
> field, so the new API wouldn't cause compatibility issue.

I think the TSNetProtoType name is confusing, it definitely confused me for a long time :) The purpose of this is to track the original protocol stack that we are doing work on behalf of, so I think that a name similar to TSClientProtoType would be clearer.

This way of tracking the client protocol stack has the risk of combinatorial explosion. I think that we should separate out the transport and application protocols to make it easier to express TCP+HTTP, TLS+HTTP, TCP+WEBSOCKET, TLS+WEBSOCKET, TCP+SPDY, TLS+SPDY, etc.

Finally, it seems that this is a property of the virtual circuit, so perhaps a better approach would be an API that can get and set the bitmask on a TSVConn?

J