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
