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 <jpe...@apache.org> wrote:
> On Aug 6, 2016, at 7:16 AM, Susan Hinrichs <shinr...@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
>>> <shinr...@network-geographics.com> wrote:
>>>
>>>
>>>
>>> On 7/28/2016 5:05 AM, James Peach wrote:
>>>>> On Jul 28, 2016, at 7:33 PM, James Peach <jpe...@apache.org> wrote:
>>>>>
>>>>>
>>>>>> On Jul 28, 2016, at 5:50 AM, Petar Bozhidarov Penkov
>>>>>> <ppen...@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
>
