search

pixie-io / pixie

Instant Kubernetes-Native Application Observability
https://px.dev
Apache License 2.0
5.6k stars 430 forks source link

gRPC support for C based gRPC libraries #546

Open isala404 opened 2 years ago

isala404 commented 2 years ago

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.

yzhao1012 commented 2 years ago

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

htroisi commented 2 years ago

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

isala404 commented 2 years ago

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.

orishuss commented 2 years ago

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.

isala404 commented 2 years ago

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.

orishuss commented 2 years ago

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.

isala404 commented 2 years ago

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 :)

zasgar commented 2 years ago

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