sammccall added a comment.

Server-side tracing looks nice, but you'll need some flags to allow actually 
extracting a trace, right?



================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:62
                       grpc::ServerWriter<LookupReply> *Reply) override {
+    trace::Span Tracer("Remote index server Lookup");
+    SPAN_ATTACH(Tracer, "LookupRequest", Request->ShortDebugString());
----------------
Nit: this is "LookupRequest" on the client side. I'd suggest just the same on 
the server, or "Recv.LookupRequest" or so? to make it easier to correspond them


================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:63
+    trace::Span Tracer("Remote index server Lookup");
+    SPAN_ATTACH(Tracer, "LookupRequest", Request->ShortDebugString());
     clangd::LookupRequest Req;
----------------
you don't do this on the client side, but could


================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:63
+    trace::Span Tracer("Remote index server Lookup");
+    SPAN_ATTACH(Tracer, "LookupRequest", Request->ShortDebugString());
     clangd::LookupRequest Req;
----------------
sammccall wrote:
> you don't do this on the client side, but could
prefer DebugString over ShortDebugString, the missing newlines can make it hard 
to follow


================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:98
+    trace::Span Tracer("Remote index server FuzzyFind");
+    SPAN_ATTACH(Tracer, "FuzzyFindRequest", Request->ShortDebugString());
     const auto Req = fromProtobuf(Request, IndexedProjectRoot);
----------------
you don't do this on the client side, but should


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83831



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to