icerpc / icerpc-csharp

A C# RPC framework built for QUIC, with bidirectional streaming, first-class async/await, and Protobuf support.
https://docs.icerpc.dev
Apache License 2.0
94 stars 13 forks source link

Telemetry added to `IceRpc.Slice.Tools` and `IceRpc.Protobuf.Tools` #4006

Closed ReeceHumphreys closed 2 weeks ago

ReeceHumphreys commented 1 month ago

This PR introduces a simple telemetry client to the IceRpc.Slice.Tools and IceRpc.Protobuf.Tools build pipeline. This initial proof of concept will require additional cleanup and refinement in subsequent PRs.

Notes

Future Planned PRs


Edit: Opening this as a draft while I add a README.md, do my own final checks, and open the telemetry server PR. Edit 2: The CI failing will be resolved once we update slicec-cs to not use the slicec version on GitHub. No idea why the CI does not like it so much.

ReeceHumphreys commented 1 month ago

Going to spin off the slicec-cs + the relevant IceRpc.Slice.Tools part for the verbose output into a separate PR!

InsertCreativityHere commented 1 month ago

Whoops, I only just saw your comment about splitting up the PRs.

InsertCreativityHere commented 1 month ago

After thinking for a little bit, I have a potentially radical proposal: I think we should make every field in the Slice Telemetry struct tagged.

In the future, we'll probably add extra fields, and they'll have to be tagged anyways for backwards compatibility. And who knows, maybe some of the information which we think is useful to collect now, may end up not being useful later, and then it'd be nice to remove it from the struct. Tagging everything gives us maximum flexibility, and I think that's useful for this Slice definition.

ReeceHumphreys commented 1 month ago

XML formatting in this PR https://github.com/icerpc/icerpc-csharp/pull/4007

ReeceHumphreys commented 1 month ago

slicec-cs metrics output PR https://github.com/icerpc/icerpc-csharp/pull/4008

ReeceHumphreys commented 1 month ago

Okay, most of the relevant review feedback for this PR has now been implemented! The only stuff for future PRs should be refining the slice definitions and data we collect, removing the testing certificate code once the telemetry server is deployed, and potentially removing the hashing code once we decide if we want to collect it or not.

The goal of this PR should just be to get the underlying structure in for the build telemetry client.

ReeceHumphreys commented 2 weeks ago

After talking about this on Monday with @bernardnormier and @InsertCreativityHere I want to emphasize this is really just an "Alpha" for the build telemetry uploading. Eventually we will shift to a plugin system and refine the fields we collect.

ReeceHumphreys commented 2 weeks ago

Thanks @pepone! This build system stuff is definitely not my expertise haha.