You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Susan Hinrichs <sh...@network-geographics.com> on 2016/08/05 21:16:51 UTC

Re: TS-4703 && PR-829

Bringing this conversation back to life.  We'd like to open source a 
plugin which relies on this API (or something like it). There has been 
discussion on the PR which James requests that we bring back to the 
mailing list.

To catch everyone up to speed with the PR discussion 
(https://github.com/apache/trafficserver/pull/829)

Alan said " I have to disagree with James. Once upon a time, this worked 
(if any one remember the "proto stack" thing). It was an explicit goal 
of the TS-3612 work to get this capability back. It is useful in a 
number of situations to be able to know what protocol the user agent is 
using to talk to ATS. I know I took no small amount of heat when I broke 
this originally."

James said "My main objection to this API is that is makes promises it 
doesn't (maybe can't) keep. It just tells you whether this is HTTP/1 or 
HTTP/2. That's fine, and an API that does that it also fine, but that 
should look different to this.

James, do you have a suggestion for a superior API to give the plugin 
information about the client protocol associated with the session?  What 
information does it not provide that it is promising?


On 7/28/2016 7:28 PM, James Peach wrote:
>> On Jul 29, 2016, at 12:29 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
>>
>>
>>
>> On 7/28/2016 5:05 AM, James Peach wrote:
>>>> On Jul 28, 2016, at 7:33 PM, James Peach <jp...@apache.org> wrote:
>>>>
>>>>
>>>>> On Jul 28, 2016, at 5:50 AM, Petar Bozhidarov Penkov <pp...@stanford.edu> wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> I am writing in accordance with the referenced Pull Request and JIRA issue.
>>>>> I am proposing a GET-er method for Transactions's underlying protocol. This
>>>>> relates to TS-2987 and is effectively one of the proposed solutions, a
>>>>> wrapper around ProxyClientTransaction::get_protocol_string() . The proposed
>>>>> API is as follows:
>>>>>
>>>>> *tsapi const char *TSHttpTxnClientProtocolGet(TSHttpTxn txnp);*
>>>>> **name credit goes to Susan Hinrichs and Alan Carroll*
>>>> Hi Petar,
>>>>
>>>> I\u2019m not sure that this is the right approach. get_protocol_string() simply distinguishes HTTP/2 sessions, so it it not something that I think is ready to become API. Since we already have an abstraction for HTTP versions (see TSHttpHdrVersionGet), maybe a better approach is to expose a convenience API that returns the transaction HTTP version as a TS_HTTP_VERSION() constant.
>>> We also have a pending review for TSHttpTxnIsWebsocket(), which seems to overlap with this proposal.
>> Yes, we would like to know whether the transaction is part of a Http/1.x session or a Http/2 session (or whatever else comes up in the future.
> Do you have a concrete use case for knowing this?
>
> J


Re: TS-4703 && PR-829

Posted by James Peach <jp...@apache.org>.
> On Sep 9, 2016, at 2:22 PM, Alan Carroll <so...@yahoo-inc.com.INVALID> wrote:
> 
> 1.

So why the output array then?

>  
> 
>    On Friday, September 9, 2016 1:19 PM, James Peach <jp...@apache.org> wrote:
> 
> 
> 
>> On Sep 9, 2016, at 10:55 AM, Alan Carroll <so...@yahoo-inc.com.INVALID> wrote:
>> 
>> No. I'm not completely sure what I meant when I wrote that (maybe it was a typo). My view on review was that it should be an in/out parameter where the tag (possibly just a prefix) is passed in and the actual tag found is passed back. Susan has convinced me it would be better to make split that in to two parameters, the second being optional (can be NULL). So the last arg becomes "char const* tag, char const** result" where
>> tag - the tag to search for, using an anchored prefix search.result - optional return value for the tag found (if NULL the found tag is not returned).
> 
> How many results can you get?
> 
>>   
>> 
>>     On Friday, September 9, 2016 12:18 PM, James Peach <jp...@apache.org> wrote:
>> 
>> 
>> 
>>> On Sep 9, 2016, at 10:15 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
>>> 
>>> 
>>> 
>>> On 9/9/2016 12:01 PM, James Peach wrote:
>>>>> On Sep 9, 2016, at 9:39 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
>>>>> 
>>>>> I'd like to propose a slight tweak for TSHttpTxnClientProtocolStackContains and TSHttpSsnClientProtocolStackContains
>>>>> 
>>>>> Replace the tag argument with two explicit input and output arguments, e.g.
>>>>> 
>>>>> int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const *contains_tag, char const **specific_tag_ptr)
>>>> The TSHttpTxnClientProtocolStackContains didn't mention anything about any output?
>>> 
>>> I was assuming that something was being returned via the tag argument since we are passing in a pointer to a pointer.
>> 
>> It takes a nul-terminated array of tags right?
> 
> 


Re: TS-4703 && PR-829

Posted by Alan Carroll <so...@yahoo-inc.com.INVALID>.
1. 

    On Friday, September 9, 2016 1:19 PM, James Peach <jp...@apache.org> wrote:
 

 
> On Sep 9, 2016, at 10:55 AM, Alan Carroll <so...@yahoo-inc.com.INVALID> wrote:
> 
> No. I'm not completely sure what I meant when I wrote that (maybe it was a typo). My view on review was that it should be an in/out parameter where the tag (possibly just a prefix) is passed in and the actual tag found is passed back. Susan has convinced me it would be better to make split that in to two parameters, the second being optional (can be NULL). So the last arg becomes "char const* tag, char const** result" where
> tag - the tag to search for, using an anchored prefix search.result - optional return value for the tag found (if NULL the found tag is not returned).

How many results can you get?

>  
> 
>    On Friday, September 9, 2016 12:18 PM, James Peach <jp...@apache.org> wrote:
> 
> 
> 
>> On Sep 9, 2016, at 10:15 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
>> 
>> 
>> 
>> On 9/9/2016 12:01 PM, James Peach wrote:
>>>> On Sep 9, 2016, at 9:39 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
>>>> 
>>>> I'd like to propose a slight tweak for TSHttpTxnClientProtocolStackContains and TSHttpSsnClientProtocolStackContains
>>>> 
>>>> Replace the tag argument with two explicit input and output arguments, e.g.
>>>> 
>>>> int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const *contains_tag, char const **specific_tag_ptr)
>>> The TSHttpTxnClientProtocolStackContains didn't mention anything about any output?
>> 
>> I was assuming that something was being returned via the tag argument since we are passing in a pointer to a pointer.
> 
> It takes a nul-terminated array of tags right?


   

Re: TS-4703 && PR-829

Posted by James Peach <jp...@apache.org>.
> On Sep 9, 2016, at 10:55 AM, Alan Carroll <so...@yahoo-inc.com.INVALID> wrote:
> 
> No. I'm not completely sure what I meant when I wrote that (maybe it was a typo). My view on review was that it should be an in/out parameter where the tag (possibly just a prefix) is passed in and the actual tag found is passed back. Susan has convinced me it would be better to make split that in to two parameters, the second being optional (can be NULL). So the last arg becomes "char const* tag, char const** result" where
> tag - the tag to search for, using an anchored prefix search.result - optional return value for the tag found (if NULL the found tag is not returned).

How many results can you get?

>  
> 
>    On Friday, September 9, 2016 12:18 PM, James Peach <jp...@apache.org> wrote:
> 
> 
> 
>> On Sep 9, 2016, at 10:15 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
>> 
>> 
>> 
>> On 9/9/2016 12:01 PM, James Peach wrote:
>>>> On Sep 9, 2016, at 9:39 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
>>>> 
>>>> I'd like to propose a slight tweak for TSHttpTxnClientProtocolStackContains and TSHttpSsnClientProtocolStackContains
>>>> 
>>>> Replace the tag argument with two explicit input and output arguments, e.g.
>>>> 
>>>> int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const *contains_tag, char const **specific_tag_ptr)
>>> The TSHttpTxnClientProtocolStackContains didn't mention anything about any output?
>> 
>> I was assuming that something was being returned via the tag argument since we are passing in a pointer to a pointer.
> 
> It takes a nul-terminated array of tags right?


Re: TS-4703 && PR-829

Posted by Alan Carroll <so...@yahoo-inc.com.INVALID>.
No. I'm not completely sure what I meant when I wrote that (maybe it was a typo). My view on review was that it should be an in/out parameter where the tag (possibly just a prefix) is passed in and the actual tag found is passed back. Susan has convinced me it would be better to make split that in to two parameters, the second being optional (can be NULL). So the last arg becomes "char const* tag, char const** result" where
tag - the tag to search for, using an anchored prefix search.result - optional return value for the tag found (if NULL the found tag is not returned). 

    On Friday, September 9, 2016 12:18 PM, James Peach <jp...@apache.org> wrote:
 

 
> On Sep 9, 2016, at 10:15 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
> 
> 
> 
> On 9/9/2016 12:01 PM, James Peach wrote:
>>> On Sep 9, 2016, at 9:39 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
>>> 
>>> I'd like to propose a slight tweak for TSHttpTxnClientProtocolStackContains and TSHttpSsnClientProtocolStackContains
>>> 
>>> Replace the tag argument with two explicit input and output arguments, e.g.
>>> 
>>> int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const *contains_tag, char const **specific_tag_ptr)
>> The TSHttpTxnClientProtocolStackContains didn't mention anything about any output?
> 
> I was assuming that something was being returned via the tag argument since we are passing in a pointer to a pointer.

It takes a nul-terminated array of tags right?
   

Re: TS-4703 && PR-829

Posted by James Peach <jp...@apache.org>.
> On Sep 9, 2016, at 10:15 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
> 
> 
> 
> On 9/9/2016 12:01 PM, James Peach wrote:
>>> On Sep 9, 2016, at 9:39 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
>>> 
>>> I'd like to propose a slight tweak for TSHttpTxnClientProtocolStackContains and TSHttpSsnClientProtocolStackContains
>>> 
>>> Replace the tag argument with two explicit input and output arguments, e.g.
>>> 
>>> int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const *contains_tag, char const **specific_tag_ptr)
>> The TSHttpTxnClientProtocolStackContains didn't mention anything about any output?
> 
> I was assuming that something was being returned via the tag argument since we are passing in a pointer to a pointer.

It takes a nul-terminated array of tags right?

> 
>> 
>>> I think having separate arguments for input and output arguments is clearer.  Also if I don't care about getting a pointer back to the specific tag string that ATS uses internally I don't have to declare additional variables to make this call, e.g.
>>> 
>>> if (TXHttpTxnClientProtocolStackContains(txnp, "h2", NULL)) {
>>>  // Do HTTP/2 stuff
>>> }
>>> 
>>> On 8/20/2016 10:20 AM, Alan Carroll wrote:
>>>> After discussions with Leif and Bryan, here is an updated proposal. This would subsume Oliver Goodman's API, which would be simply calling this and looking for / passing 'ws' as the protocol tag.
>>>> There is an unresolved point, which is the exact stack returned for a HTTP/2 connection.
>>>> 
>>>> .. include:: ../../../common.defs
>>>> 
>>>> .. default-domain:: c
>>>> 
>>>> TSClientProtocolStack
>>>> *********************
>>>> 
>>>> Synopsis
>>>> ========
>>>> 
>>>> `#include <ts/ts.h>`
>>>> 
>>>> .. function:: TSReturnCode TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const** result, int* actual)
>>>> 
>>>> .. function:: TSReturnCode TSHttpSsnClientProtocolStackGet(TSHttpSsn ssnp, int n, char const** result, int* actual)
>>>> 
>>>> .. function:: int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const** tag)
>>>> 
>>>> .. function:: int TSHttpSsnClientProtocolStackContains(TSHttpSsn ssnp, char const** tag)
>>>> 
>>>> .. function:: char const* TSNormalizedProtocolTag(char const* tag)
>>>> 
>>>> .. function:: char const* TSRegisterProtocolTag(char const* tag)
>>>> 
>>>> Description
>>>> ===========
>>>> 
>>>> These functions are used to explore the protocol stack of the client (user agent) connection to |TS|. The functions :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` can be used to retrieve the entire protocol stack for the user agent connection. :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` will check for a specific protocol :arg:`tag` being present in the stack.
>>>> 
>>>> Each protocol is represented by tag which is a null terminated string. A particular tag will always be returned as the same character pointer and so protocols can be reliably checked with pointer comparisons. :func:`TSNormalizedProtocolTag` will return this character pointer for a specific :arg:`tag`. A return value of :const:`NULL` indicates the provided :arg:`tag` is not registered as a known protocol tag. :func:`TSRegisterProtocolTag` registers the :arg:`tag` and then returns its normalized value. This is useful for plugins that provide custom protocols for user agents.
>>>> 
>>>> The protocols are ordered from higher level protocols to the lower level ones on which the higher operate. For instance a stack might look like "http/1.1,tls/1.2,tcp,ipv4". For :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` these values are placed in the array :arg:`result`. :arg:`count` is the maximum number of elements of :arg:`result` that may be modified by the function call. If :arg:`actual` is not :const:`NULL` then the actual number of elements in the protocol stack will be returned. If this is equal or less than :arg:`count` then all elements were returned. If it is larger then some layers were omitted from :arg:`result`. If the full stack is required :arg:`actual` can be used to resize :arg:`result` to be sufficient to hold all of the elements and the function called again with updated :arg:`count` and :arg:`result`. In practice the maximum number of elements will is almost certain to be less than 10 which therefore should suffice. These functions return :const:`TS_SUCCESS` on success and :const:`TS_ERROR` on failure which should only occurr if :arg:`txnp` or :arg:`ssnp` are invalid.
>>>> 
>>>> The :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` functions are provided for the convenience when only the presence of a protocol is of interest, not its location or the presence of other protocols. These functions return 0 if the protocol :arg:`tag` is not present, non-zero if it is present. The strings are matched with an anchor prefix search, as with debug tags. For instance if :arg:`tag` is "tls" then it will match "tls/1.2" or "tls/1.3". This makes checking for TLS or IP more convenient. If more precision is required the entire protocol stack can be retrieved and processed more thoroughly.
>>>> 
>>>> The protocol tags defined by |TS|.
>>>> 
>>>> =========== =========
>>>> Protocol    Tag
>>>> =========== =========
>>>> HTTP/1.1    http/1.1
>>>> HTTP/1.0    http/1.0
>>>> HTTP/2      h2
>>>> WebSocket   ws
>>>> TLS 1.3     tls/1.3
>>>> TLS 1.2     tls/1.2
>>>> TCP         tcp
>>>> UDP         udp
>>>> IPv4        ipv4
>>>> IPv6        ipv6
>>>> QUIC        quic
>>>> =========== =========
>>>> 
>>>> .. note::
>>>>    What should HTTP/2 connections return as the top protocol? There are several options
>>>>        *  "http/1.1,h2"
>>>>    *  "h2"
>>>>    *  "http/1.1,h2" for transctions and "h2" for sessions.
>>>> 
>>>> 
>>>>    
>>> 
> 
> 


Re: TS-4703 && PR-829

Posted by Susan Hinrichs <sh...@network-geographics.com>.

On 9/9/2016 12:01 PM, James Peach wrote:
>> On Sep 9, 2016, at 9:39 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
>>
>> I'd like to propose a slight tweak for TSHttpTxnClientProtocolStackContains and TSHttpSsnClientProtocolStackContains
>>
>> Replace the tag argument with two explicit input and output arguments, e.g.
>>
>> int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const *contains_tag, char const **specific_tag_ptr)
> The TSHttpTxnClientProtocolStackContains didn't mention anything about any output?

I was assuming that something was being returned via the tag argument 
since we are passing in a pointer to a pointer.

>
>> I think having separate arguments for input and output arguments is clearer.  Also if I don't care about getting a pointer back to the specific tag string that ATS uses internally I don't have to declare additional variables to make this call, e.g.
>>
>> if (TXHttpTxnClientProtocolStackContains(txnp, "h2", NULL)) {
>>   // Do HTTP/2 stuff
>> }
>>
>> On 8/20/2016 10:20 AM, Alan Carroll wrote:
>>> After discussions with Leif and Bryan, here is an updated proposal. This would subsume Oliver Goodman's API, which would be simply calling this and looking for / passing 'ws' as the protocol tag.
>>> There is an unresolved point, which is the exact stack returned for a HTTP/2 connection.
>>>
>>> .. include:: ../../../common.defs
>>>
>>> .. default-domain:: c
>>>
>>> TSClientProtocolStack
>>> *********************
>>>
>>> Synopsis
>>> ========
>>>
>>> `#include <ts/ts.h>`
>>>
>>> .. function:: TSReturnCode TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const** result, int* actual)
>>>
>>> .. function:: TSReturnCode TSHttpSsnClientProtocolStackGet(TSHttpSsn ssnp, int n, char const** result, int* actual)
>>>
>>> .. function:: int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const** tag)
>>>
>>> .. function:: int TSHttpSsnClientProtocolStackContains(TSHttpSsn ssnp, char const** tag)
>>>
>>> .. function:: char const* TSNormalizedProtocolTag(char const* tag)
>>>
>>> .. function:: char const* TSRegisterProtocolTag(char const* tag)
>>>
>>> Description
>>> ===========
>>>
>>> These functions are used to explore the protocol stack of the client (user agent) connection to |TS|. The functions :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` can be used to retrieve the entire protocol stack for the user agent connection. :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` will check for a specific protocol :arg:`tag` being present in the stack.
>>>
>>> Each protocol is represented by tag which is a null terminated string. A particular tag will always be returned as the same character pointer and so protocols can be reliably checked with pointer comparisons. :func:`TSNormalizedProtocolTag` will return this character pointer for a specific :arg:`tag`. A return value of :const:`NULL` indicates the provided :arg:`tag` is not registered as a known protocol tag. :func:`TSRegisterProtocolTag` registers the :arg:`tag` and then returns its normalized value. This is useful for plugins that provide custom protocols for user agents.
>>>
>>> The protocols are ordered from higher level protocols to the lower level ones on which the higher operate. For instance a stack might look like "http/1.1,tls/1.2,tcp,ipv4". For :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` these values are placed in the array :arg:`result`. :arg:`count` is the maximum number of elements of :arg:`result` that may be modified by the function call. If :arg:`actual` is not :const:`NULL` then the actual number of elements in the protocol stack will be returned. If this is equal or less than :arg:`count` then all elements were returned. If it is larger then some layers were omitted from :arg:`result`. If the full stack is required :arg:`actual` can be used to resize :arg:`result` to be sufficient to hold all of the elements and the function called again with updated :arg:`count` and :arg:`result`. In practice the maximum number of elements will is almost certain to be less than 10 which therefore should suffice. These functions return :const:`TS_SUCCESS` on success and :const:`TS_ERROR` on failure which should only occurr if :arg:`txnp` or :arg:`ssnp` are invalid.
>>>
>>> The :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` functions are provided for the convenience when only the presence of a protocol is of interest, not its location or the presence of other protocols. These functions return 0 if the protocol :arg:`tag` is not present, non-zero if it is present. The strings are matched with an anchor prefix search, as with debug tags. For instance if :arg:`tag` is "tls" then it will match "tls/1.2" or "tls/1.3". This makes checking for TLS or IP more convenient. If more precision is required the entire protocol stack can be retrieved and processed more thoroughly.
>>>
>>> The protocol tags defined by |TS|.
>>>
>>> =========== =========
>>> Protocol    Tag
>>> =========== =========
>>> HTTP/1.1    http/1.1
>>> HTTP/1.0    http/1.0
>>> HTTP/2      h2
>>> WebSocket   ws
>>> TLS 1.3     tls/1.3
>>> TLS 1.2     tls/1.2
>>> TCP         tcp
>>> UDP         udp
>>> IPv4        ipv4
>>> IPv6        ipv6
>>> QUIC        quic
>>> =========== =========
>>>
>>> .. note::
>>>     What should HTTP/2 connections return as the top protocol? There are several options
>>>         *  "http/1.1,h2"
>>>     *  "h2"
>>>     *  "http/1.1,h2" for transctions and "h2" for sessions.
>>>
>>>
>>>     
>>



Re: TS-4703 && PR-829

Posted by James Peach <jp...@apache.org>.
> On Sep 9, 2016, at 9:39 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
> 
> I'd like to propose a slight tweak for TSHttpTxnClientProtocolStackContains and TSHttpSsnClientProtocolStackContains
> 
> Replace the tag argument with two explicit input and output arguments, e.g.
> 
> int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const *contains_tag, char const **specific_tag_ptr)

The TSHttpTxnClientProtocolStackContains didn't mention anything about any output?

> 
> I think having separate arguments for input and output arguments is clearer.  Also if I don't care about getting a pointer back to the specific tag string that ATS uses internally I don't have to declare additional variables to make this call, e.g.
> 
> if (TXHttpTxnClientProtocolStackContains(txnp, "h2", NULL)) {
>  // Do HTTP/2 stuff
> }
> 
> On 8/20/2016 10:20 AM, Alan Carroll wrote:
>> After discussions with Leif and Bryan, here is an updated proposal. This would subsume Oliver Goodman's API, which would be simply calling this and looking for / passing 'ws' as the protocol tag.
>> There is an unresolved point, which is the exact stack returned for a HTTP/2 connection.
>> 
>> .. include:: ../../../common.defs
>> 
>> .. default-domain:: c
>> 
>> TSClientProtocolStack
>> *********************
>> 
>> Synopsis
>> ========
>> 
>> `#include <ts/ts.h>`
>> 
>> .. function:: TSReturnCode TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const** result, int* actual)
>> 
>> .. function:: TSReturnCode TSHttpSsnClientProtocolStackGet(TSHttpSsn ssnp, int n, char const** result, int* actual)
>> 
>> .. function:: int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const** tag)
>> 
>> .. function:: int TSHttpSsnClientProtocolStackContains(TSHttpSsn ssnp, char const** tag)
>> 
>> .. function:: char const* TSNormalizedProtocolTag(char const* tag)
>> 
>> .. function:: char const* TSRegisterProtocolTag(char const* tag)
>> 
>> Description
>> ===========
>> 
>> These functions are used to explore the protocol stack of the client (user agent) connection to |TS|. The functions :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` can be used to retrieve the entire protocol stack for the user agent connection. :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` will check for a specific protocol :arg:`tag` being present in the stack.
>> 
>> Each protocol is represented by tag which is a null terminated string. A particular tag will always be returned as the same character pointer and so protocols can be reliably checked with pointer comparisons. :func:`TSNormalizedProtocolTag` will return this character pointer for a specific :arg:`tag`. A return value of :const:`NULL` indicates the provided :arg:`tag` is not registered as a known protocol tag. :func:`TSRegisterProtocolTag` registers the :arg:`tag` and then returns its normalized value. This is useful for plugins that provide custom protocols for user agents.
>> 
>> The protocols are ordered from higher level protocols to the lower level ones on which the higher operate. For instance a stack might look like "http/1.1,tls/1.2,tcp,ipv4". For :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` these values are placed in the array :arg:`result`. :arg:`count` is the maximum number of elements of :arg:`result` that may be modified by the function call. If :arg:`actual` is not :const:`NULL` then the actual number of elements in the protocol stack will be returned. If this is equal or less than :arg:`count` then all elements were returned. If it is larger then some layers were omitted from :arg:`result`. If the full stack is required :arg:`actual` can be used to resize :arg:`result` to be sufficient to hold all of the elements and the function called again with updated :arg:`count` and :arg:`result`. In practice the maximum number of elements will is almost certain to be less than 10 which therefore should suffice. These functions return :const:`TS_SUCCESS` on success and :const:`TS_ERROR` on failure which should only occurr if :arg:`txnp` or :arg:`ssnp` are invalid.
>> 
>> The :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` functions are provided for the convenience when only the presence of a protocol is of interest, not its location or the presence of other protocols. These functions return 0 if the protocol :arg:`tag` is not present, non-zero if it is present. The strings are matched with an anchor prefix search, as with debug tags. For instance if :arg:`tag` is "tls" then it will match "tls/1.2" or "tls/1.3". This makes checking for TLS or IP more convenient. If more precision is required the entire protocol stack can be retrieved and processed more thoroughly.
>> 
>> The protocol tags defined by |TS|.
>> 
>> =========== =========
>> Protocol    Tag
>> =========== =========
>> HTTP/1.1    http/1.1
>> HTTP/1.0    http/1.0
>> HTTP/2      h2
>> WebSocket   ws
>> TLS 1.3     tls/1.3
>> TLS 1.2     tls/1.2
>> TCP         tcp
>> UDP         udp
>> IPv4        ipv4
>> IPv6        ipv6
>> QUIC        quic
>> =========== =========
>> 
>> .. note::
>>    What should HTTP/2 connections return as the top protocol? There are several options
>>        *  "http/1.1,h2"
>>    *  "h2"
>>    *  "http/1.1,h2" for transctions and "h2" for sessions.
>> 
>> 
>>    
> 
> 


Re: TS-4703 && PR-829

Posted by Susan Hinrichs <sh...@network-geographics.com>.
I'd like to propose a slight tweak for 
TSHttpTxnClientProtocolStackContains and 
TSHttpSsnClientProtocolStackContains

Replace the tag argument with two explicit input and output arguments, e.g.

int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const *contains_tag, char const **specific_tag_ptr)

I think having separate arguments for input and output arguments is clearer.  Also if I don't care about getting a pointer back to the specific tag string that ATS uses internally I don't have to declare additional variables to make this call, e.g.

if (TXHttpTxnClientProtocolStackContains(txnp, "h2", NULL)) {
   // Do HTTP/2 stuff
}

On 8/20/2016 10:20 AM, Alan Carroll wrote:
> After discussions with Leif and Bryan, here is an updated proposal. This would subsume Oliver Goodman's API, which would be simply calling this and looking for / passing 'ws' as the protocol tag.
> There is an unresolved point, which is the exact stack returned for a HTTP/2 connection.
>
> .. include:: ../../../common.defs
>
> .. default-domain:: c
>
> TSClientProtocolStack
> *********************
>
> Synopsis
> ========
>
> `#include <ts/ts.h>`
>
> .. function:: TSReturnCode TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const** result, int* actual)
>
> .. function:: TSReturnCode TSHttpSsnClientProtocolStackGet(TSHttpSsn ssnp, int n, char const** result, int* actual)
>
> .. function:: int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const** tag)
>
> .. function:: int TSHttpSsnClientProtocolStackContains(TSHttpSsn ssnp, char const** tag)
>
> .. function:: char const* TSNormalizedProtocolTag(char const* tag)
>
> .. function:: char const* TSRegisterProtocolTag(char const* tag)
>
> Description
> ===========
>
> These functions are used to explore the protocol stack of the client (user agent) connection to |TS|. The functions :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` can be used to retrieve the entire protocol stack for the user agent connection. :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` will check for a specific protocol :arg:`tag` being present in the stack.
>
> Each protocol is represented by tag which is a null terminated string. A particular tag will always be returned as the same character pointer and so protocols can be reliably checked with pointer comparisons. :func:`TSNormalizedProtocolTag` will return this character pointer for a specific :arg:`tag`. A return value of :const:`NULL` indicates the provided :arg:`tag` is not registered as a known protocol tag. :func:`TSRegisterProtocolTag` registers the :arg:`tag` and then returns its normalized value. This is useful for plugins that provide custom protocols for user agents.
>
> The protocols are ordered from higher level protocols to the lower level ones on which the higher operate. For instance a stack might look like "http/1.1,tls/1.2,tcp,ipv4". For :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` these values are placed in the array :arg:`result`. :arg:`count` is the maximum number of elements of :arg:`result` that may be modified by the function call. If :arg:`actual` is not :const:`NULL` then the actual number of elements in the protocol stack will be returned. If this is equal or less than :arg:`count` then all elements were returned. If it is larger then some layers were omitted from :arg:`result`. If the full stack is required :arg:`actual` can be used to resize :arg:`result` to be sufficient to hold all of the elements and the function called again with updated :arg:`count` and :arg:`result`. In practice the maximum number of elements will is almost certain to be less than 10 which therefore should suffice. These functions return :const:`TS_SUCCESS` on success and :const:`TS_ERROR` on failure which should only occurr if :arg:`txnp` or :arg:`ssnp` are invalid.
>
> The :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` functions are provided for the convenience when only the presence of a protocol is of interest, not its location or the presence of other protocols. These functions return 0 if the protocol :arg:`tag` is not present, non-zero if it is present. The strings are matched with an anchor prefix search, as with debug tags. For instance if :arg:`tag` is "tls" then it will match "tls/1.2" or "tls/1.3". This makes checking for TLS or IP more convenient. If more precision is required the entire protocol stack can be retrieved and processed more thoroughly.
>
> The protocol tags defined by |TS|.
>
> =========== =========
> Protocol    Tag
> =========== =========
> HTTP/1.1    http/1.1
> HTTP/1.0    http/1.0
> HTTP/2      h2
> WebSocket   ws
> TLS 1.3     tls/1.3
> TLS 1.2     tls/1.2
> TCP         tcp
> UDP         udp
> IPv4        ipv4
> IPv6        ipv6
> QUIC        quic
> =========== =========
>
> .. note::
>     What should HTTP/2 connections return as the top protocol? There are several options
>     
>     *  "http/1.1,h2"
>     *  "h2"
>     *  "http/1.1,h2" for transctions and "h2" for sessions.
>
>
>     



Re: TS-4703 && PR-829

Posted by Alan Carroll <so...@yahoo-inc.com.INVALID>.
After discussions with Leif and Bryan, here is an updated proposal. This would subsume Oliver Goodman's API, which would be simply calling this and looking for / passing 'ws' as the protocol tag.
There is an unresolved point, which is the exact stack returned for a HTTP/2 connection.

.. include:: ../../../common.defs

.. default-domain:: c

TSClientProtocolStack
*********************

Synopsis
========

`#include <ts/ts.h>`

.. function:: TSReturnCode TSHttpTxnClientProtocolStackGet(TSHttpTxn txnp, int n, char const** result, int* actual)

.. function:: TSReturnCode TSHttpSsnClientProtocolStackGet(TSHttpSsn ssnp, int n, char const** result, int* actual)

.. function:: int TSHttpTxnClientProtocolStackContains(TSHttpTxn txnp, char const** tag)

.. function:: int TSHttpSsnClientProtocolStackContains(TSHttpSsn ssnp, char const** tag)

.. function:: char const* TSNormalizedProtocolTag(char const* tag)

.. function:: char const* TSRegisterProtocolTag(char const* tag)

Description
===========

These functions are used to explore the protocol stack of the client (user agent) connection to |TS|. The functions :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` can be used to retrieve the entire protocol stack for the user agent connection. :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` will check for a specific protocol :arg:`tag` being present in the stack.

Each protocol is represented by tag which is a null terminated string. A particular tag will always be returned as the same character pointer and so protocols can be reliably checked with pointer comparisons. :func:`TSNormalizedProtocolTag` will return this character pointer for a specific :arg:`tag`. A return value of :const:`NULL` indicates the provided :arg:`tag` is not registered as a known protocol tag. :func:`TSRegisterProtocolTag` registers the :arg:`tag` and then returns its normalized value. This is useful for plugins that provide custom protocols for user agents.

The protocols are ordered from higher level protocols to the lower level ones on which the higher operate. For instance a stack might look like "http/1.1,tls/1.2,tcp,ipv4". For :func:`TSHttpTxnClientProtocolStackGet` and :func:`TSHttpSsnClientProtocolStackGet` these values are placed in the array :arg:`result`. :arg:`count` is the maximum number of elements of :arg:`result` that may be modified by the function call. If :arg:`actual` is not :const:`NULL` then the actual number of elements in the protocol stack will be returned. If this is equal or less than :arg:`count` then all elements were returned. If it is larger then some layers were omitted from :arg:`result`. If the full stack is required :arg:`actual` can be used to resize :arg:`result` to be sufficient to hold all of the elements and the function called again with updated :arg:`count` and :arg:`result`. In practice the maximum number of elements will is almost certain to be less than 10 which therefore should suffice. These functions return :const:`TS_SUCCESS` on success and :const:`TS_ERROR` on failure which should only occurr if :arg:`txnp` or :arg:`ssnp` are invalid.

The :func:`TSHttpTxnClientProtocolStackContains` and :func:`TSHttpSsnClientProtocolStackContains` functions are provided for the convenience when only the presence of a protocol is of interest, not its location or the presence of other protocols. These functions return 0 if the protocol :arg:`tag` is not present, non-zero if it is present. The strings are matched with an anchor prefix search, as with debug tags. For instance if :arg:`tag` is "tls" then it will match "tls/1.2" or "tls/1.3". This makes checking for TLS or IP more convenient. If more precision is required the entire protocol stack can be retrieved and processed more thoroughly.

The protocol tags defined by |TS|.

=========== =========
Protocol    Tag
=========== =========
HTTP/1.1    http/1.1
HTTP/1.0    http/1.0
HTTP/2      h2
WebSocket   ws
TLS 1.3     tls/1.3
TLS 1.2     tls/1.2
TCP         tcp
UDP         udp
IPv4        ipv4
IPv6        ipv6
QUIC        quic
=========== =========

.. note::
   What should HTTP/2 connections return as the top protocol? There are several options
   
   *  "http/1.1,h2"
   *  "h2"
   *  "http/1.1,h2" for transctions and "h2" for sessions.


   

Re: TS-4703 && PR-829

Posted by James Peach <jp...@apache.org>.
> On Aug 12, 2016, at 3:03 PM, James Peach <jp...@apache.org> wrote:
> 
> 
>> On Aug 12, 2016, at 11:55 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
>> 
>> Barring the poor formatting of this particular email, this API seems like a reasonable trade-off.
> 
> Yeh I think this is a superset of all the proposals we have seen so far :)
> 
> What do you think about the implementability/usability of this API?

Also, is this intended to publish websockets too? If so, could you please coordinate with Oliver Goodman wrt his API proposal? thanks :)

> 
>> 
>> 
>> On 8/9/2016 8:29 PM, Alan Carroll wrote:
>>> What if we made the API a bit more general.
>>> TSReturnCode TSTxnClientProtocolStackGet(TSHttpTxn txnp, int count, char **result, int* actual);
>>> This function returns an array of static character strings that represent the protocol stack for the user agent connection. *result* is an array of char const* pointers with at least *count* elements. The protocols are listed from higher to lower in *result*. If there is insufficient space in *result* the lower protocols are not stored. The number of protocols is return in *actual* if *actual* is not NULL. Note that *actual* can be larger than *count* - this indicates that protocols were discarded and not put in to *result*. Pointers in *result* that are not required are unchanged. No element of *result* past the first *count* elements will be modified.
>>> 
>>> Callers can detect the size of the return array by std::min(*actual*,*count*). Alternatively the array can be pre-zero'd and the caller can walk until a NULL pointer is found. If this is required then *count* can be set to be one less than the actual array size. Callers interested in just the top level protocol can pass *count* as 1 with the address of a char const* and NULL for *actual*.
>>> Defined strings:
>>> Protocol          Tag
>>> HTTP/2            h2HTTP/1.1          http/1.1HTTP/1.0          http/1.0QUIC              quicTCP               tcpUDP               udpTLS               tlsWeb Sockets       ws
>>> 
>>> 
>>> 
>>> 
>> 
> 


Re: TS-4703 && PR-829

Posted by James Peach <jp...@apache.org>.
> On Aug 12, 2016, at 11:55 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
> 
> Barring the poor formatting of this particular email, this API seems like a reasonable trade-off.

Yeh I think this is a superset of all the proposals we have seen so far :)

What do you think about the implementability/usability of this API?

> 
> 
> On 8/9/2016 8:29 PM, Alan Carroll wrote:
>>  What if we made the API a bit more general.
>> TSReturnCode TSTxnClientProtocolStackGet(TSHttpTxn txnp, int count, char **result, int* actual);
>> This function returns an array of static character strings that represent the protocol stack for the user agent connection. *result* is an array of char const* pointers with at least *count* elements. The protocols are listed from higher to lower in *result*. If there is insufficient space in *result* the lower protocols are not stored. The number of protocols is return in *actual* if *actual* is not NULL. Note that *actual* can be larger than *count* - this indicates that protocols were discarded and not put in to *result*. Pointers in *result* that are not required are unchanged. No element of *result* past the first *count* elements will be modified.
>> 
>> Callers can detect the size of the return array by std::min(*actual*,*count*). Alternatively the array can be pre-zero'd and the caller can walk until a NULL pointer is found. If this is required then *count* can be set to be one less than the actual array size. Callers interested in just the top level protocol can pass *count* as 1 with the address of a char const* and NULL for *actual*.
>> Defined strings:
>> Protocol          Tag
>> HTTP/2            h2HTTP/1.1          http/1.1HTTP/1.0          http/1.0QUIC              quicTCP               tcpUDP               udpTLS               tlsWeb Sockets       ws
>> 
>> 
>> 
>>   
> 


Re: TS-4703 && PR-829

Posted by Susan Hinrichs <sh...@network-geographics.com>.
Barring the poor formatting of this particular email, this API seems 
like a reasonable trade-off.


On 8/9/2016 8:29 PM, Alan Carroll wrote:
>   What if we made the API a bit more general.
> TSReturnCode TSTxnClientProtocolStackGet(TSHttpTxn txnp, int count, char **result, int* actual);
> This function returns an array of static character strings that represent the protocol stack for the user agent connection. *result* is an array of char const* pointers with at least *count* elements. The protocols are listed from higher to lower in *result*. If there is insufficient space in *result* the lower protocols are not stored. The number of protocols is return in *actual* if *actual* is not NULL. Note that *actual* can be larger than *count* - this indicates that protocols were discarded and not put in to *result*. Pointers in *result* that are not required are unchanged. No element of *result* past the first *count* elements will be modified.
>
> Callers can detect the size of the return array by std::min(*actual*,*count*). Alternatively the array can be pre-zero'd and the caller can walk until a NULL pointer is found. If this is required then *count* can be set to be one less than the actual array size. Callers interested in just the top level protocol can pass *count* as 1 with the address of a char const* and NULL for *actual*.
> Defined strings:
> Protocol          Tag
> HTTP/2            h2HTTP/1.1          http/1.1HTTP/1.0          http/1.0QUIC              quicTCP               tcpUDP               udpTLS               tlsWeb Sockets       ws
>
>
>
>    


Re: TS-4703 && PR-829

Posted by Alan Carroll <so...@yahoo-inc.com.INVALID>.
 What if we made the API a bit more general.
TSReturnCode TSTxnClientProtocolStackGet(TSHttpTxn txnp, int count, char **result, int* actual);
This function returns an array of static character strings that represent the protocol stack for the user agent connection. *result* is an array of char const* pointers with at least *count* elements. The protocols are listed from higher to lower in *result*. If there is insufficient space in *result* the lower protocols are not stored. The number of protocols is return in *actual* if *actual* is not NULL. Note that *actual* can be larger than *count* - this indicates that protocols were discarded and not put in to *result*. Pointers in *result* that are not required are unchanged. No element of *result* past the first *count* elements will be modified.

Callers can detect the size of the return array by std::min(*actual*,*count*). Alternatively the array can be pre-zero'd and the caller can walk until a NULL pointer is found. If this is required then *count* can be set to be one less than the actual array size. Callers interested in just the top level protocol can pass *count* as 1 with the address of a char const* and NULL for *actual*.
Defined strings:
Protocol          Tag
HTTP/2            h2HTTP/1.1          http/1.1HTTP/1.0          http/1.0QUIC              quicTCP               tcpUDP               udpTLS               tlsWeb Sockets       ws



  

Re: TS-4703 && PR-829

Posted by James Peach <jp...@apache.org>.
> On Aug 8, 2016, at 9:30 PM, Alan Carroll <so...@yahoo-inc.com.INVALID> wrote:
> 
> I have to say I'm not in favor having a very fat API where every possible protocol has an independent call to check. We have actually been doing something very similar to this with the plugin tag pre-TS-3612. This in effect maintains that capability except more reliably.
> I understand the concern with the string content but that seems easily fixed with a bit of documentation, starting with using NPN/ALPN names.

Can you explain how ALPN is related? My understanding of the proposal is that it could not every return an ALPN entry?

> QUIC is easily accomodated as well

The proposed API would have to return "HTTP/2" for QUIC.

> , along with web sockets.

If the API is returning the top protocol, then you would get "websocket", but have no way to know whether it was over HTTP, HTTP/2, HTTP/2+QUIC, etc.

> I may misunderstand how those work, but I think they're disjoint form HTTP/2. Perhaps simply changing the name to "TSHttpTxnGetClientTopProtocol"? Because what's really wanted here is the top/outermost protocol in use by the user agent connection.

Higher up in the thread it sounds like what is wanted is the HTTP protocol version, which I agree is a reasonable piece of information to have.

> I had considered going back to the original idea for this, where there is an API that lets you walk the entire protocol stack and see all the pieces. But I got push back from that as well because, the argument was, knowing the top level protocol implies all the underlying ones and so that's redundant and therefore useless.

I would disagree that it is redundant, since you can have HTTP on TLS, HTTP/2 on TLS or QUIC :)

> In addition having this API instead of a far binary one makes logging and logging output much more consistent, which is a feature.

Do you mean that the primary use of the string result is to be logged rather than inspected?

>    On Friday, August 5, 2016 5:37 PM, James Peach <jp...@apache.org> wrote:
> 
> 
> 
>> On Aug 6, 2016, at 7:16 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
>> 
>> Bringing this conversation back to life.  We'd like to open source a plugin which relies on this API (or something like it). There has been discussion on the PR which James requests that we bring back to the mailing list.
>> 
>> To catch everyone up to speed with the PR discussion (https://github.com/apache/trafficserver/pull/829)
>> 
>> Alan said " I have to disagree with James. Once upon a time, this worked (if any one remember the "proto stack" thing). It was an explicit goal of the TS-3612 work to get this capability back. It is useful in a number of situations to be able to know what protocol the user agent is using to talk to ATS. I know I took no small amount of heat when I broke this originally."
>> 
>> James said "My main objection to this API is that is makes promises it doesn't (maybe can't) keep. It just tells you whether this is HTTP/1 or HTTP/2. That's fine, and an API that does that it also fine, but that should look different to this.
>> 
>> James, do you have a suggestion for a superior API to give the plugin information about the client protocol associated with the session?  What information does it not provide that it is promising?
> 
> I made some suggestions on my first response on this thread.
> 
> TSHttpTxnClientProtocolGet() returns an undefined string. So, firstly I think that the notion of a protocol is not well defined by this API. Protocol could mean IP, TCP, TLS, UDP, QUIC, WebSockets, some custom protocol, some ALPN name, HTTP/1, HTTP/2 or really anything. The fact that this returns and unspecified string reinforces the impression that it is open ended.
> 
> This leads me to the reasons that I think it will not move gracefully into the future. One day we will have QUIC support and it is not clear to me what would happen the. If we still return "HTTP/2" (which would be consistent, then what is the point of returning a string? We might as well return the HTTP version number that can be cracked using TS_HTTP_VERSION().
> 
> Since the use case is to know whether this transaction is part of a HTTP/2 session or not, my suggestion is to about all ambiguity and simply define an API that tells you that. There’s plenty of precendent in the TSHttpTxnIs*() APIs, or, we already have a way to represent HTTP versions. I don’t see a good reason for the additional abstraction at this stage.
> 
> 
>> On 7/28/2016 7:28 PM, James Peach wrote:
>>>> On Jul 29, 2016, at 12:29 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
>>>> 
>>>> 
>>>> 
>>>> On 7/28/2016 5:05 AM, James Peach wrote:
>>>>>> On Jul 28, 2016, at 7:33 PM, James Peach <jp...@apache.org> wrote:
>>>>>> 
>>>>>> 
>>>>>>> On Jul 28, 2016, at 5:50 AM, Petar Bozhidarov Penkov <pp...@stanford.edu> wrote:
>>>>>>> 
>>>>>>> Hello,
>>>>>>> 
>>>>>>> I am writing in accordance with the referenced Pull Request and JIRA issue.
>>>>>>> I am proposing a GET-er method for Transactions's underlying protocol. This
>>>>>>> relates to TS-2987 and is effectively one of the proposed solutions, a
>>>>>>> wrapper around ProxyClientTransaction::get_protocol_string() . The proposed
>>>>>>> API is as follows:
>>>>>>> 
>>>>>>> *tsapi const char *TSHttpTxnClientProtocolGet(TSHttpTxn txnp);*
>>>>>>> **name credit goes to Susan Hinrichs and Alan Carroll*
>>>>>> Hi Petar,
>>>>>> 
>>>>>> I’m not sure that this is the right approach. get_protocol_string() simply distinguishes HTTP/2 sessions, so it it not something that I think is ready to become API. Since we already have an abstraction for HTTP versions (see TSHttpHdrVersionGet), maybe a better approach is to expose a convenience API that returns the transaction HTTP version as a TS_HTTP_VERSION() constant.
>>>>> We also have a pending review for TSHttpTxnIsWebsocket(), which seems to overlap with this proposal.
>>>> Yes, we would like to know whether the transaction is part of a Http/1.x session or a Http/2 session (or whatever else comes up in the future.
>>> Do you have a concrete use case for knowing this?
>>> 
>>> J
>> 
> 
> 


Re: TS-4703 && PR-829

Posted by Alan Carroll <so...@yahoo-inc.com.INVALID>.
I have to say I'm not in favor having a very fat API where every possible protocol has an independent call to check. We have actually been doing something very similar to this with the plugin tag pre-TS-3612. This in effect maintains that capability except more reliably.
I understand the concern with the string content but that seems easily fixed with a bit of documentation, starting with using NPN/ALPN names. QUIC is easily accomodated as well, along with web sockets. I may misunderstand how those work, but I think they're disjoint form HTTP/2. Perhaps simply changing the name to "TSHttpTxnGetClientTopProtocol"? Because what's really wanted here is the top/outermost protocol in use by the user agent connection.
I had considered going back to the original idea for this, where there is an API that lets you walk the entire protocol stack and see all the pieces. But I got push back from that as well because, the argument was, knowing the top level protocol implies all the underlying ones and so that's redundant and therefore useless.
In addition having this API instead of a far binary one makes logging and logging output much more consistent, which is a feature.
 

    On Friday, August 5, 2016 5:37 PM, James Peach <jp...@apache.org> wrote:
 

 
> On Aug 6, 2016, at 7:16 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
> 
> Bringing this conversation back to life.  We'd like to open source a plugin which relies on this API (or something like it). There has been discussion on the PR which James requests that we bring back to the mailing list.
> 
> To catch everyone up to speed with the PR discussion (https://github.com/apache/trafficserver/pull/829)
> 
> Alan said " I have to disagree with James. Once upon a time, this worked (if any one remember the "proto stack" thing). It was an explicit goal of the TS-3612 work to get this capability back. It is useful in a number of situations to be able to know what protocol the user agent is using to talk to ATS. I know I took no small amount of heat when I broke this originally."
> 
> James said "My main objection to this API is that is makes promises it doesn't (maybe can't) keep. It just tells you whether this is HTTP/1 or HTTP/2. That's fine, and an API that does that it also fine, but that should look different to this.
> 
> James, do you have a suggestion for a superior API to give the plugin information about the client protocol associated with the session?  What information does it not provide that it is promising?

I made some suggestions on my first response on this thread.

TSHttpTxnClientProtocolGet() returns an undefined string. So, firstly I think that the notion of a protocol is not well defined by this API. Protocol could mean IP, TCP, TLS, UDP, QUIC, WebSockets, some custom protocol, some ALPN name, HTTP/1, HTTP/2 or really anything. The fact that this returns and unspecified string reinforces the impression that it is open ended.

This leads me to the reasons that I think it will not move gracefully into the future. One day we will have QUIC support and it is not clear to me what would happen the. If we still return "HTTP/2" (which would be consistent, then what is the point of returning a string? We might as well return the HTTP version number that can be cracked using TS_HTTP_VERSION().

Since the use case is to know whether this transaction is part of a HTTP/2 session or not, my suggestion is to about all ambiguity and simply define an API that tells you that. There’s plenty of precendent in the TSHttpTxnIs*() APIs, or, we already have a way to represent HTTP versions. I don’t see a good reason for the additional abstraction at this stage.


> On 7/28/2016 7:28 PM, James Peach wrote:
>>> On Jul 29, 2016, at 12:29 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
>>> 
>>> 
>>> 
>>> On 7/28/2016 5:05 AM, James Peach wrote:
>>>>> On Jul 28, 2016, at 7:33 PM, James Peach <jp...@apache.org> wrote:
>>>>> 
>>>>> 
>>>>>> On Jul 28, 2016, at 5:50 AM, Petar Bozhidarov Penkov <pp...@stanford.edu> wrote:
>>>>>> 
>>>>>> Hello,
>>>>>> 
>>>>>> I am writing in accordance with the referenced Pull Request and JIRA issue.
>>>>>> I am proposing a GET-er method for Transactions's underlying protocol. This
>>>>>> relates to TS-2987 and is effectively one of the proposed solutions, a
>>>>>> wrapper around ProxyClientTransaction::get_protocol_string() . The proposed
>>>>>> API is as follows:
>>>>>> 
>>>>>> *tsapi const char *TSHttpTxnClientProtocolGet(TSHttpTxn txnp);*
>>>>>> **name credit goes to Susan Hinrichs and Alan Carroll*
>>>>> Hi Petar,
>>>>> 
>>>>> I’m not sure that this is the right approach. get_protocol_string() simply distinguishes HTTP/2 sessions, so it it not something that I think is ready to become API. Since we already have an abstraction for HTTP versions (see TSHttpHdrVersionGet), maybe a better approach is to expose a convenience API that returns the transaction HTTP version as a TS_HTTP_VERSION() constant.
>>>> We also have a pending review for TSHttpTxnIsWebsocket(), which seems to overlap with this proposal.
>>> Yes, we would like to know whether the transaction is part of a Http/1.x session or a Http/2 session (or whatever else comes up in the future.
>> Do you have a concrete use case for knowing this?
>> 
>> J
> 


  

Re: TS-4703 && PR-829

Posted by James Peach <jp...@apache.org>.
> On Aug 6, 2016, at 7:16 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
> 
> Bringing this conversation back to life.  We'd like to open source a plugin which relies on this API (or something like it). There has been discussion on the PR which James requests that we bring back to the mailing list.
> 
> To catch everyone up to speed with the PR discussion (https://github.com/apache/trafficserver/pull/829)
> 
> Alan said " I have to disagree with James. Once upon a time, this worked (if any one remember the "proto stack" thing). It was an explicit goal of the TS-3612 work to get this capability back. It is useful in a number of situations to be able to know what protocol the user agent is using to talk to ATS. I know I took no small amount of heat when I broke this originally."
> 
> James said "My main objection to this API is that is makes promises it doesn't (maybe can't) keep. It just tells you whether this is HTTP/1 or HTTP/2. That's fine, and an API that does that it also fine, but that should look different to this.
> 
> James, do you have a suggestion for a superior API to give the plugin information about the client protocol associated with the session?  What information does it not provide that it is promising?

I made some suggestions on my first response on this thread.

TSHttpTxnClientProtocolGet() returns an undefined string. So, firstly I think that the notion of a protocol is not well defined by this API. Protocol could mean IP, TCP, TLS, UDP, QUIC, WebSockets, some custom protocol, some ALPN name, HTTP/1, HTTP/2 or really anything. The fact that this returns and unspecified string reinforces the impression that it is open ended.

This leads me to the reasons that I think it will not move gracefully into the future. One day we will have QUIC support and it is not clear to me what would happen the. If we still return "HTTP/2" (which would be consistent, then what is the point of returning a string? We might as well return the HTTP version number that can be cracked using TS_HTTP_VERSION().

Since the use case is to know whether this transaction is part of a HTTP/2 session or not, my suggestion is to about all ambiguity and simply define an API that tells you that. There’s plenty of precendent in the TSHttpTxnIs*() APIs, or, we already have a way to represent HTTP versions. I don’t see a good reason for the additional abstraction at this stage.


> On 7/28/2016 7:28 PM, James Peach wrote:
>>> On Jul 29, 2016, at 12:29 AM, Susan Hinrichs <sh...@network-geographics.com> wrote:
>>> 
>>> 
>>> 
>>> On 7/28/2016 5:05 AM, James Peach wrote:
>>>>> On Jul 28, 2016, at 7:33 PM, James Peach <jp...@apache.org> wrote:
>>>>> 
>>>>> 
>>>>>> On Jul 28, 2016, at 5:50 AM, Petar Bozhidarov Penkov <pp...@stanford.edu> wrote:
>>>>>> 
>>>>>> Hello,
>>>>>> 
>>>>>> I am writing in accordance with the referenced Pull Request and JIRA issue.
>>>>>> I am proposing a GET-er method for Transactions's underlying protocol. This
>>>>>> relates to TS-2987 and is effectively one of the proposed solutions, a
>>>>>> wrapper around ProxyClientTransaction::get_protocol_string() . The proposed
>>>>>> API is as follows:
>>>>>> 
>>>>>> *tsapi const char *TSHttpTxnClientProtocolGet(TSHttpTxn txnp);*
>>>>>> **name credit goes to Susan Hinrichs and Alan Carroll*
>>>>> Hi Petar,
>>>>> 
>>>>> I’m not sure that this is the right approach. get_protocol_string() simply distinguishes HTTP/2 sessions, so it it not something that I think is ready to become API. Since we already have an abstraction for HTTP versions (see TSHttpHdrVersionGet), maybe a better approach is to expose a convenience API that returns the transaction HTTP version as a TS_HTTP_VERSION() constant.
>>>> We also have a pending review for TSHttpTxnIsWebsocket(), which seems to overlap with this proposal.
>>> Yes, we would like to know whether the transaction is part of a Http/1.x session or a Http/2 session (or whatever else comes up in the future.
>> Do you have a concrete use case for knowing this?
>> 
>> J
>