After a bit of a hiatus, I am going to start work on this back up. I addressed all of your comments and updated the doc on github.
I also added a new field to the subchannel tracer called num_nodes_logged, which tracks the total number of nodes logged. This will be useful as many nodes may get overwritten since we are going to use a circular buffer to hold the trace. Similarly, I added num_subchannels_logged to the channel tracer since subchannel tracers will be retired and freed as they go inactive. On Monday, January 30, 2017 at 9:46:26 AM UTC-8, Mark D. Roth wrote: > > (+ctiller) > > Overall, this looks pretty decent. Here are a few initial thoughts... > > I like the idea of using JSON for the returned tracing data for C-core, > especially since it means less overhead in wrapped languages that want to > expose the new tracing APIs. However, JSON may not be the desired form for > this data in all languages; the Java and Go folks may prefer some > language-specific data structure. I suggest checking with folks on those > teams to see what they think. (If we are going to have a > language-independent server UI for displaying trace data, then that may be > a good argument for all languages using JSON, but we need to make sure > everyone is on board with that.) > > The gRFC should document the schema for the JSON data. In particular, we > should probably make sure that the JSON data is in a form that can be > automatically converted into a protobuf (which we'll want to define), as > per https://developers.google.com/protocol-buffers/docs/proto3#json. > > In terms of the C-core implementation, as you and I and Craig discussed > last week, the grpc_subchannel_tracer struct will probably need a refcount, > since it may be referenced by multiple parent channels. Whenever a parent > channel gets a trace node indicating that a subchannel has been added or > removed from the parent channel, that trace node should hold a reference to > the subchannel trace. Thus, the subchannel trace will live until the last > node referencing it is removed from the parent channels' buffers. (Update: > Ah, I see you mentioned this at the very end of the doc. It might be > useful to make this clear earlier, when the data structures themselves are > presented.) > > You might also consider making the list of subchannel tracers a > doubly-linked list, so that it's easier to delete entries from the middle > of the list. > > It might be advantageous to use grpc_channel_tracer for both parent > channels and subchannels, so that you don't need a separate internal API > for adding nodes to each type. Or perhaps simply create some common base > class for the head_trace and tail_trace fields, and > have grpc_channel_tracer_add_trace() operate on that base class. > > Please let me know if you have any questions or concerns about any of this. > > On Wed, Jan 25, 2017 at 11:18 AM, ncteisen via grpc.io < > [email protected] <javascript:>> wrote: > >> My first link was to the blob, so it is stale. >> >> Instead use this link <https://github.com/grpc/proposal/pull/7> to the >> pull request itself. >> >> On Wednesday, January 25, 2017 at 10:16:46 AM UTC-8, [email protected] >> wrote: >>> >>> I've created a gRFC describing the design and implementation plan for >>> gRPC Channel Tracing >>> >>> Take a look at the planning doc. >>> <https://github.com/ncteisen/proposal/blob/b80ca8410120fdae99ef0ad54bc3005cff87bea4/A3.md> >>> >>> Would love to hear some feedback on the design! >>> >> -- >> You received this message because you are subscribed to the Google Groups >> "grpc.io" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected] <javascript:>. >> To post to this group, send email to [email protected] >> <javascript:>. >> Visit this group at https://groups.google.com/group/grpc-io. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/grpc-io/90af5752-0d28-41ad-8887-372070ad2430%40googlegroups.com >> >> <https://groups.google.com/d/msgid/grpc-io/90af5752-0d28-41ad-8887-372070ad2430%40googlegroups.com?utm_medium=email&utm_source=footer> >> . >> >> For more options, visit https://groups.google.com/d/optout. >> > > > > -- > Mark D. Roth <[email protected] <javascript:>> > Software Engineer > Google, Inc. > -- You received this message because you are subscribed to the Google Groups "grpc.io" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at https://groups.google.com/group/grpc-io. To view this discussion on the web visit https://groups.google.com/d/msgid/grpc-io/2e29a3c8-0464-4c6b-b27a-3e5b2f3c8ea8%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
