I totally agree with less API is better. > tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp, uint64_t *stream_id);
Stripping "Http2" looks reasonable. We can use this API for protocols that have stream id. > tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn txnp, uint64_t *stream_dependency, int *weight); I doubt removing "Http2" from this API makes sense. Because this API is assuming HTTP/2's priority scheme (dependency & weight). As Masakazu pointed out, a new priority scheme is proposed and it's completely different from HTTP/2's priority scheme. It's trying to make a priority scheme HTTP version-independent. So I'd like to leave "Http2" for this API and reserve "TSHttpTxnClientPriorityGet" for the new priority scheme. - Masaori On Tue, Sep 1, 2020 at 12:02 PM Brian Neradt <brian.ner...@gmail.com> wrote: > Thanks all, this is good feedback. It sounds like we're in agreement on > dropping the explicit reference to Http2 so this can accommodate future > protocols. Considering this and the observation that stream_id is 62 bits > for HTTP/3, the output parameters should be increased in size accordingly. > That would make the API look like the following: > > tsapi TSReturnCode TSHttpTxnClientStreamIdGet(TSHttpTxn txnp, uint64_t > *stream_id); > > tsapi TSReturnCode TSHttpTxnClientPriorityGet(TSHttpTxn txnp, uint64_t > *stream_dependency, int *weight); > > This will satisfy the Traffic Dump plugin needs. The concern raised about > protocol identification is not an issue for Traffic Dump because it will > know the protocol from either the protocol retrieval API > (TSHttpSsnClientProtocolStackGet) or from the version in the request > via TSHttpHdrVersionGet. That being the case, I'm content not adding a > separate output parameter for this. > > That said, I don't want to dismiss the option too quickly if others are > still interested. Sudheer, you seem to have the most developed ideas for > this. If, considering what I say above, you'd still like us to consider the > inclusion of a context parameter, could you please flesh out: > > - What the signatures for the stream id and priority getters would look > like. > - The structure of the context object. > - Would all HTTP/2 and above APIs contain this context parameter? > > Otherwise, if we're OK not having the context parameter, I can update the > draft PR with the above changes to the API. > > Thanks, > > On Mon, Aug 31, 2020 at 9:19 PM Sudheer Vinukonda > <sudheervinuko...@yahoo.com.invalid> wrote: > > > Right, it does make sense to have a generic API naming (and remove http2 > > reference from it), but I'm thinking it should still allow to pass in > some > > context object (we may define an interface for it, with "protocol" being > a > > mandatory field) to be able to continue using it in the future as well as > > to let user explicitly specify that they want stream id for http2 or > http2 > > for example? > > Thanks, > > Sudheer > > On Monday, August 31, 2020, 06:49:57 PM PDT, Masakazu Kitajo < > > m4s...@gmail.com> wrote: > > > > Sudheer, > > > > Would a separated API to get the actual protocol active for the given > > request satisfy you? I don't understand how indicating the actual > protocol > > helps something. > > > > I think I understand your concern. Future protocols may need different > > params, or might have different representations. However, H/2 is > extensible > > so any APIs that have "Http2" in their names still can be incompatible if > > we support extensions. The proposal for a new priority scheme is a good > > example. > > > > In that sense, passing a context object may be helpful, and more flexible > > than having fixed context in API names, but maybe it's too early to think > > about it. > > > > Masakazu > > > > > > On Mon, Aug 31, 2020 at 11:05 AM Sudheer Vinukonda > > <sudheervinuko...@yahoo.com.invalid> wrote: > > > > > Hmm, it does sound appealing to avoid having to duplicate API for > common > > > attributes between H/2 and H/3, but, I'm thinking the API should still > > > somehow explicitly indicate (either via an input param or an output > > param) > > > the actual protocol active for the given request. It seems a bit too > > > confusing (and potentially problematic depending on how things evolve > in > > > the future) to hide that aspect. > > > Thoughts? > > > > > > > > > On Sunday, August 30, 2020, 06:56:49 PM PDT, Masakazu Kitajo < > > > mas...@apache.org> wrote: > > > > > > And there is a proposal for a new priority scheme that may be used for > > > both > > > H2 and H3. > > > https://tools.ietf.org/html/draft-ietf-httpbis-priority-01 > > > > > > I haven't read it, but it would be nice if the TS API could be > compatible > > > with the new scheme. > > > > > > On Mon, Aug 31, 2020 at 10:48 AM Masakazu Kitajo <mas...@apache.org> > > > wrote: > > > > > > > I think we should avoid adding APIs just for HTTP/2 unless it's > really > > > > HTTP/2 specific. Are we going to add TSHttpTxnClientHttp3StreamIdGet > > and > > > > TSHttpTxnClientHttp3PriorityGet as well? > > > > > > > > I'd omit the "2" in the API names, and rephrase the API doc like: > > > > This API returns an error if the provided transaction does not have a > > > > stream id. > > > > > > > > Also, a stream id on HTTP/3 (QUIC) is 62 bit integer. > > > > > > > > Masakazu > > > > > > > > > > > > On Sat, Aug 29, 2020 at 5:24 AM SUSAN HINRICHS <shinr...@ieee.org> > > > wrote: > > > > > > > >> +1 > > > >> > > > >> On Fri, Aug 28, 2020 at 3:23 PM Sudheer Vinukonda > > > >> <sudheervinuko...@yahoo.com.invalid> wrote: > > > >> > > > >> > +1. > > > >> > Sounds reasonable to me. > > > >> > > > > >> > > > > >> > On Friday, August 28, 2020, 07:41:18 AM PDT, Brian Neradt < > > > >> > brian.ner...@gmail.com> wrote: > > > >> > > > > >> > Hi dev@trafficserver.apache.org, > > > >> > > > > >> > It would be helpful to add HTTP/2 stream and priority support to > > > Traffic > > > >> > Dump. In order for the plugin to access this information, I > propose > > > >> adding > > > >> > the following two functions to the TS API: > > > >> > > > > >> > tsapi TSReturnCode TSHttpTxnClientHttp2StreamIdGet(TSHttpTxn txnp, > > > >> uint32_t > > > >> > *stream_id); > > > >> > > > > >> > tsapi TSReturnCode TSHttpTxnClientHttp2PriorityGet(TSHttpTxn txnp, > > int > > > >> > *stream_dependency, int *weight); > > > >> > > > > >> > I've created a draft PR with this implemented along with Traffic > > > Dump's > > > >> use > > > >> > of it: > > > >> > https://github.com/apache/trafficserver/pull/7149 > > > >> > > > > >> > Please let me know if there are any concerns or suggestions for > > > >> improvement > > > >> > concerning this. > > > >> > > > > >> > Thanks! > > > >> > Brian > > > >> > > > > >> > -- > > > >> > "Come to Me, all who are weary and heavy-laden, and I will > > > >> > give you rest. Take My yoke upon you and learn from Me, for > > > >> > I am gentle and humble in heart, and you will find rest for > > > >> > your souls. For My yoke is easy and My burden is light." > > > >> > > > > >> > ~ Matthew 11:28-30 > > > >> > > > > >> > > > > > > > > > > > > > -- > "Come to Me, all who are weary and heavy-laden, and I will > give you rest. Take My yoke upon you and learn from Me, for > I am gentle and humble in heart, and you will find rest for > your souls. For My yoke is easy and My burden is light." > > ~ Matthew 11:28-30 >
