pixie icon indicating copy to clipboard operation
pixie copied to clipboard

gRPC support for C based gRPC libraries

Open isala404 opened this issue 3 years ago • 8 comments

Is your feature request related to a problem? Please describe. As of now Pixie only supports go-gRPC tracing. Since the gRPC protocol itself is supported by many widely used languages such as Python and Node, there is big gap in pixie's observability capabilities when it comes to no go languages.

Describe the solution you'd like As mentioned on Pixie's slack channel, C++, Python, Ruby, Objective-C, PHP and C#, all relies on c-gRPC library. So covering this library with UProbes would give a huge boost to pixie's observability capabilities.

isala404 avatar Jul 30 '22 08:07 isala404

@orishuss from Groundcover is working on a c-gRPC library tracing. Ori feel free to provide updates to @MrSupiri yourself.

yzhao1012 avatar Aug 01 '22 17:08 yzhao1012

@MrSupiri you can follow this PR for updates on gRPC-C support: https://github.com/pixie-io/pixie/pull/547

htroisi avatar Aug 01 '22 20:08 htroisi

Thanks for the update!, I will keep and eye on it. Also sorry I have no idea why github created the same issue twice, it might have bugged out when I pressed submit.

isala404 avatar Aug 02 '22 04:08 isala404

Hey @MrSupiri, @avivzgroundcover and I have been working on it. This feature contains 3 large PRs into pixie, the last one is now under review. 6 new uprobes are required in order to trace this library.

Once the 3rd PR is merged, the feature will not be instantly usable due to:

  1. Only 4 versions are supported at first: 1.19.0, 1.24.1, 1.33.2, 1.41.1. Versions in-between, or close to these versions, should be covered as well with minimal effort: checking offsets of fields in other versions.
  2. Only a few specific docker images will be covered at first. This is because the way we currently use to find the library's version is the hash of the library, which might change between docker images. Adding support for a docker image only requires starting it and checking the hash of the library.

This PR will finish laying the groundwork for gRPC-c support in Pixie. Supporting more versions, and more docker images, is expected to be really easy, and will hopefully not take long after this PR is merged.

orishuss avatar Aug 03 '22 09:08 orishuss

Hey, Thanks for the update, looking forward to testing this out. BTW is there is a particular reason for choosing those specific version numbers? since 1.19.0 is version released 3 years ago and 1.41.1 was also released almost an year ago.

isala404 avatar Aug 05 '22 05:08 isala404

When developing eBPF observability solutions, we take a look many versions backwards, to see our solution works for a lot of versions, to make sure its stable enough. This is why we chose 4 versions which are relatively far apart.

This feature's development took ~3 weeks, at February-March this year, and it simply isn't fully integrated into Pixie yet. Back then we chose 1.41.1 as one of the newest versions out there. Also, if you take a look at the library's code, their mechanism for storing metadata changed a bit after that to use a map (which maps the header name to its value) instead of a linked list (which this solution works with). This is one of the reasons we built this solution with a mechanism to change logic according to the library's version, so making this work on the newer metadata storage is just a matter of mapping the new data structure (which will result in changing the logic of our eBPF fill_metadata_from_mdelem_list function). The "old" linked-list mechanism was the one they used for a very long time, which is why we decided to start with that, while the "new" mechanism was only present in one new version back then, and we couldn't even be sure it's going to stay this way.

We went back down to 1.19.0 because we needed it for a specific groundcover use-case.

orishuss avatar Aug 05 '22 09:08 orishuss

Ah yeah that makes sense and thank you for the well detailed explanation. I saw pixie team was also using version based method for nodejs TLS library since they changed a namespace of it which mess up the entire probe. Again thank for all the work you folks are putting out, excited to get our hands on this :)

isala404 avatar Aug 05 '22 12:08 isala404

@orishuss, assigning this to you for now since you implemented the first pass at this. CC: @oazizi000

zasgar avatar Sep 23 '22 00:09 zasgar