[PATCH] D98246: [clangd] Add basic monitoring info request for remote index server

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/index/remote/Service.proto:16
 
+message MonitoringInfoReply { optional string info = 1; }
+
----------------
sammccall wrote:
> kadircet wrote:
> > i am not sure if having more structure here immediately vs. incrementally makes much difference, we should at least settle on the proto initially, and can fill in those fields in incremental steps if need be (but i don't expect those changes to be huge, so it should be fine to just cram them into this patch).
> > 
> > i think we need more structure here, rather than just string. for starters:
> > - an unsigned for uptime (probably in seconds)
> > - another unsgined (or timepoint object) for indicating the build time of currently used index
> > 
> > i am not sure if we got anything else we can dispatch immediately, but can probably incorporate things like QPS and more details about the loaded index if turns out to be useful/needed.
> it seems unlikely to me that `string info` is the right format, but I can't tell whether this is a placeholder or not
> we should at least settle on the proto initially, and can fill in those fields in incremental steps

Agree, incremental development is nice, but the scope/responsibility of RPCs is fairly important and not that easy to change later.

(Sorry, I hadn't seen this review wasn't actually assigned to me, happy to leave this with you!)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98246/new/

https://reviews.llvm.org/D98246