grpc-go icon indicating copy to clipboard operation
grpc-go copied to clipboard

channelz: major cleanup / reorganization

Open dfawley opened this issue 1 year ago • 1 comments

Several major changes are included:

  • Stop using an "ID" to reference the channelz object from the channel/etc that contains it. Instead get a pointer to the object directly.
  • Stop storing the channelz metrics in the channel/etc and instead store them in the channelz struct representation of that thing.
  • Reorganize files so they are categorized by type instead of "func" vs. "types"
  • Lots of small things

To be done:

  • Represent tree of channelz objects using a tree rooted on the top-level types (channel, server) instead of a bunch of flat buffers storing each type with references between them. We still need the flat buffers to look things up through the channelz service, but this might remove the need for manual reference counting, e.g.
  • Cleanup the trace logs to be simpler. There's a pretty complex interaction between the trace logs and the things they reference right now.

dfawley avatar Feb 07 '24 22:02 dfawley

Codecov Report

Merging #6969 (3de5f23) into master (4f43d2e) will increase coverage by 0.01%. The diff coverage is 87.60%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6969      +/-   ##
==========================================
+ Coverage   82.44%   82.45%   +0.01%     
==========================================
  Files         296      299       +3     
  Lines       31475    31317     -158     
==========================================
- Hits        25948    25822     -126     
+ Misses       4471     4450      -21     
+ Partials     1056     1045      -11     
Files Coverage Δ
balancer/balancer.go 95.83% <ø> (ø)
balancer/grpclb/grpclb_remote_balancer.go 83.69% <100.00%> (ø)
channelz/service/func_linux.go 85.88% <100.00%> (+0.51%) :arrow_up:
dialoptions.go 89.07% <100.00%> (ø)
internal/channelz/funcs.go 100.00% <100.00%> (+4.97%) :arrow_up:
internal/transport/http2_client.go 90.82% <100.00%> (-0.13%) :arrow_down:
internal/transport/http2_server.go 90.74% <100.00%> (-0.06%) :arrow_down:
internal/transport/transport.go 94.07% <ø> (+1.18%) :arrow_up:
resolver_wrapper.go 85.18% <100.00%> (ø)
rpc_util.go 77.77% <ø> (ø)
... and 14 more

... and 14 files with indirect coverage changes

codecov[bot] avatar Feb 07 '24 22:02 codecov[bot]

I discussed with @dfawley offline. The general refactor and the changes look good to me. There is however still more scope of cleanup - which can be categorized as future improvements to channelz in general.

LGTM, you can ignore the nit formatting comments from above. Also the branch requires a rebase.

arvindbr8 avatar Mar 13 '24 18:03 arvindbr8

I've resolved the convos for nits. Could you take a look at the rest and see if makes sense?

arvindbr8 avatar Mar 13 '24 18:03 arvindbr8

Thanks for the review! I also merged from master ( :exploding_head:) to fix the conflicts.

dfawley avatar Mar 15 '24 16:03 dfawley

:crossed_fingers:

dfawley avatar Mar 15 '24 18:03 dfawley