Add hooks for distributed tracing #201

Activity

1. addaleax commented on Nov 26, 2019

   

   addaleax

   on Nov 26, 2019

   Member

   More actions

   I’m a fan of having hooks for these kinds of things … is there any chance you could put together a proposal for how an API for this should look like?
2. jasnell commented on Nov 26, 2019

   

   jasnell

   on Nov 26, 2019

   Member

   More actions

   I'm also happy to have these built in. A couple comments tho...

   1. I'm not sure the "static hook to notify about creation of new sockets" makes sense in this case. The "socket" in QUIC is just a local UDP binding that user code has to create -- therefore, user code is presumably already aware when they are created.

   2. The lifecycle events on sessions should be fairly straight forward as we already have most of that information available in the existing ready and secure events. Those can evolve a bit and we can make sure they are providing enough information.

   3. The hooks to record and modify outgoing headers is interesting for sure but would also have a performance impact. What I'm thinking is an event that can be triggered just immediately before the outgoing headers are serialized for internal handling that allows them to be augmented... e.g.

   // headers is the actual block of headers, type is an indication of
   // whether they are informational, initial, or trailing headers
   stream.on('outgoingHeaders', (headers, type) => {
     headers['NEW_HEADER_TO_ADD'] = 'trackingID'
   });

   Keep in mind, however, that with QUIC, header support is only enabled per Application protocol. Not all QUIC protocols will support headers so this will be best effort at best. It would always work when using HTTP/3, however.

   This approach is something we can easily backport to the http/2 and http/1 implementations as well, although with http/1 it would be a bit more difficult to support consistently due to how headers are handled there.

   I'm not sure about the need for an inbound header hook in the low level API, but if it is needed, it can be handled as stream.on('inboundHeaders', (headers, type) => {}).

   4. I really don't think we should provide any special API or hook for intercepting the data flow. That's precisely why we have Transforms in the streams API already.
3. Flarna commented on Nov 26, 2019

   

   Flarna

   on Nov 26, 2019

   MemberAuthor

   More actions

   Thanks for your answers. Sure, I can draft a more concrete API (but will take a while...)

   Regarding static hook and use of transform in stream. I agree that these are not needed if monitoring would be implemented as part of the application. But the typical use case is that you have an existing application and an additional, independent module from some other vendor is preloaded to add monitoring (e.g. via -r command line argument). This monitoring module needs to get the information about new sockets to install the followup listeners it uses to record transactions and to transform the data.

   And sure, monitoring like this has an overhead. The overhead should be mostly the same if hooks are used compared to monkey patching.
4. Flarna commented on Dec 6, 2019

   

   Flarna

   on Dec 6, 2019

   MemberAuthor

   More actions

   The QUIC API looks quite similar as the HTTP/2 API but has still some ToDos inside therefore I think it's a good starting point to describe how we capture data from HTTP/2.

   Server:

   • monkey patch createSecureServer and createSecureServer to get an Http2Server/Http2SecureServer (as they are not accessible via public API).
   • wrap "stream" listeners on Http2Server/Http2SecureServer
   • monkey patch ServerHttp2Stream.prototype.respond(), .respondWithFile() and .respondWithFD() once we see a ServerHttp2Stream.
   • install "error"/"close" listeners on each stream
   • for POST and content-type application/x-www-form-urlencoded we wrap/path incoming data stream

   Captured data:

   • a transaction is started once a "stream" event is emitted.
   • capture incoming headers, local bound socket info, remote socket info
   • its vital to get incoming headers at the beginning to allow reasonable smart sampling decisions/filtering
   • via monkey patched respondXXX() APIs we capture response headers
   • if applicable capture request parameters from x-www-form-urlencoded body
   • close/error events are used to end transaction and capture error.stack if applicable

   Advance features (not yet in use for HTTP/2 but in HTTP):

   • modify response headers
   • modify response body

   I think we could get the same functionality with following hooks:

   • incoming stream is created. Arguments: the stream object, headers, async id/resource
   • response is started. Arguments: the stream object, response headers
   • stream is closed. Arguments: the stream object, reason/error

   Client:

   • monkey patch http2.connect to get access to ClientHttp2Session
   • monkey patch ClientHttp2Session.request to get ClientHttp2Stream
   • wrap/install "response", "close" and "error" events on ClientHttp2Stream

   Captured data:

   • transaction is started with ClientHttp2Session.request
   • outgoing headers (tricky part are protocol and authority as they are stored private in session)
   • outgoing headers are also modified to add a tag for transaction linking
   • response headers in "response" listener
   • close/error events are used to end transaction and capture error.stack if applicable

   I think we could get the same functionality with following hooks:

   • client stream is created. Arguments: the stream object, headers, async id/resource
   • response is received. Arguments: the stream object, response headers
   • stream is closed. Arguments: the stream object, reason/error

   Context propagation
   There are also some details relevant regarding async hooks.
   Currently we wrap listeners like "stream" which allows us to propagate our current context to any async operation created. With hooks as indicated above it's not that easy. Think about following sample:

   function onStream(stream) {   // incoming stream is a new transaction (1)
     http3.request(firstUrl, cb1);   // outgoing request is a new child transaction (A) linked to (1)
     process.nextTick(someWork); // no new transaction, propagate context (1)
     http3.request(secondUrl, cb2); // outgoing request is a new child transaction (B) linked to (1)
   }
   

   I think this can work with above hooks if following conditions are met:

   • a HTTP/3 request/stream is associated with one async_id which is used in all callbacks/listeners
   • the hooks which indicates that a new transaction starts provides this async_id to allow to propagate the new transaction context instead that one from executing async_id

   👍React with 👍1Reacted by James M Snell