[PATCH] D83831: [clangd] Add more logs and attach tracers to remote index server routines

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83831

Files:
  clang-tools-extra/clangd/index/remote/server/Server.cpp


Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -9,6 +9,8 @@
 #include "index/Index.h"
 #include "index/Serialization.h"
 #include "index/remote/marshalling/Marshalling.h"
+#include "support/Logger.h"
+#include "support/Trace.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Path.h"
@@ -57,19 +59,26 @@
   grpc::Status Lookup(grpc::ServerContext *Context,
   const LookupRequest *Request,
   grpc::ServerWriter *Reply) override {
+trace::Span Tracer("Remote index server Lookup");
+SPAN_ATTACH(Tracer, "LookupRequest", Request->ShortDebugString());
 clangd::LookupRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Lookup request cancelled: invalid SymbolID {1}", 
SID.takeError());
 return grpc::Status::CANCELLED;
+  }
   Req.IDs.insert(*SID);
 }
+unsigned SentItems = 0;
 Index->lookup(Req, [&](const clangd::Symbol &Sym) {
   LookupReply NextMessage;
   *NextMessage.mutable_stream_result() =
   toProtobuf(Sym, IndexedProjectRoot);
   Reply->Write(NextMessage);
+  ++SentItems;
 });
+SPAN_ATTACH(Tracer, "SentItems", SentItems);
 LookupReply LastMessage;
 LastMessage.set_final_result(true);
 Reply->Write(LastMessage);
@@ -79,13 +88,18 @@
   grpc::Status FuzzyFind(grpc::ServerContext *Context,
  const FuzzyFindRequest *Request,
  grpc::ServerWriter *Reply) override {
+trace::Span Tracer("Remote index server FuzzyFind");
+SPAN_ATTACH(Tracer, "FuzzyFindRequest", Request->ShortDebugString());
 const auto Req = fromProtobuf(Request, IndexedProjectRoot);
+unsigned SentItems = 0;
 bool HasMore = Index->fuzzyFind(Req, [&](const clangd::Symbol &Sym) {
   FuzzyFindReply NextMessage;
   *NextMessage.mutable_stream_result() =
   toProtobuf(Sym, IndexedProjectRoot);
   Reply->Write(NextMessage);
+  ++SentItems;
 });
+SPAN_ATTACH(Tracer, "SentItems", SentItems);
 FuzzyFindReply LastMessage;
 LastMessage.set_final_result(HasMore);
 Reply->Write(LastMessage);
@@ -94,18 +108,25 @@
 
   grpc::Status Refs(grpc::ServerContext *Context, const RefsRequest *Request,
 grpc::ServerWriter *Reply) override {
+trace::Span Tracer("Remote index server Refs");
+SPAN_ATTACH(Tracer, "RefsRequest", Request->ShortDebugString());
 clangd::RefsRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Refs request cancelled: invalid SymbolID {1}", SID.takeError());
 return grpc::Status::CANCELLED;
+  }
   Req.IDs.insert(*SID);
 }
+unsigned SentItems = 0;
 bool HasMore = Index->refs(Req, [&](const clangd::Ref &Reference) {
   RefsReply NextMessage;
   *NextMessage.mutable_stream_result() = toProtobuf(Reference, IndexRoot);
   Reply->Write(NextMessage);
+  ++SentItems;
 });
+SPAN_ATTACH(Tracer, "SentItems", SentItems);
 RefsReply LastMessage;
 LastMessage.set_final_result(HasMore);
 Reply->Write(LastMessage);


Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -9,6 +9,8 @@
 #include "index/Index.h"
 #include "index/Serialization.h"
 #include "index/remote/marshalling/Marshalling.h"
+#include "support/Logger.h"
+#include "support/Trace.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Path.h"
@@ -57,19 +59,26 @@
   grpc::Status Lookup(grpc::ServerContext *Context,
   const LookupRequest *Request,
   grpc::ServerWriter *Reply) override {
+trace::Span Tracer("Remote index server Lookup");
+SPAN_ATTACH(Tracer, "LookupRequest", Request->ShortDebugString());
 clangd::LookupRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Lookup request cancelled: invalid SymbolID {1}", SID.takeError());
 return grpc::Status::CANCELLED;
+ 

[PATCH] D83831: [clangd] Add more logs and attach tracers to remote index server routines

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 278081.
kbobyrev added a comment.

Rebase on top of parent revision, add more logging on the client side.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83831

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp
  clang-tools-extra/clangd/index/remote/server/Server.cpp

Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -9,6 +9,8 @@
 #include "index/Index.h"
 #include "index/Serialization.h"
 #include "index/remote/marshalling/Marshalling.h"
+#include "support/Logger.h"
+#include "support/Trace.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Path.h"
@@ -57,21 +59,32 @@
   grpc::Status Lookup(grpc::ServerContext *Context,
   const LookupRequest *Request,
   grpc::ServerWriter *Reply) override {
+trace::Span Tracer("Remote index server Lookup");
+SPAN_ATTACH(Tracer, "LookupRequest", Request->ShortDebugString());
 clangd::LookupRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Lookup request cancelled: invalid SymbolID {1}", SID.takeError());
 return grpc::Status::CANCELLED;
+  }
   Req.IDs.insert(*SID);
 }
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
 Index->lookup(Req, [&](const clangd::Symbol &Sym) {
   auto SerializedSymbol = toProtobuf(Sym, IndexedProjectRoot);
-  if (!SerializedSymbol)
+  if (!SerializedSymbol) {
+++FailedToSend;
 return;
+  }
   LookupReply NextMessage;
   *NextMessage.mutable_stream_result() = *SerializedSymbol;
   Reply->Write(NextMessage);
+  ++Sent;
 });
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
 LookupReply LastMessage;
 LastMessage.set_final_result(true);
 Reply->Write(LastMessage);
@@ -81,15 +94,24 @@
   grpc::Status FuzzyFind(grpc::ServerContext *Context,
  const FuzzyFindRequest *Request,
  grpc::ServerWriter *Reply) override {
+trace::Span Tracer("Remote index server FuzzyFind");
+SPAN_ATTACH(Tracer, "FuzzyFindRequest", Request->ShortDebugString());
 const auto Req = fromProtobuf(Request, IndexedProjectRoot);
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
 bool HasMore = Index->fuzzyFind(Req, [&](const clangd::Symbol &Sym) {
   auto SerializedSymbol = toProtobuf(Sym, IndexedProjectRoot);
-  if (!SerializedSymbol)
+  if (!SerializedSymbol) {
+++FailedToSend;
 return;
+  }
   FuzzyFindReply NextMessage;
   *NextMessage.mutable_stream_result() = *SerializedSymbol;
   Reply->Write(NextMessage);
+  ++Sent;
 });
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
 FuzzyFindReply LastMessage;
 LastMessage.set_final_result(HasMore);
 Reply->Write(LastMessage);
@@ -98,21 +120,32 @@
 
   grpc::Status Refs(grpc::ServerContext *Context, const RefsRequest *Request,
 grpc::ServerWriter *Reply) override {
+trace::Span Tracer("Remote index server Refs");
+SPAN_ATTACH(Tracer, "RefsRequest", Request->ShortDebugString());
 clangd::RefsRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Refs request cancelled: invalid SymbolID {1}", SID.takeError());
 return grpc::Status::CANCELLED;
+  }
   Req.IDs.insert(*SID);
 }
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
 bool HasMore = Index->refs(Req, [&](const clangd::Ref &Reference) {
   auto SerializedRef = toProtobuf(Reference, IndexedProjectRoot);
-  if (!SerializedRef)
+  if (!SerializedRef) {
+++FailedToSend;
 return;
+  }
   RefsReply NextMessage;
   *NextMessage.mutable_stream_result() = *SerializedRef;
   Reply->Write(NextMessage);
+  ++Sent;
 });
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
 RefsReply LastMessage;
 LastMessage.set_final_result(HasMore);
 Reply->Write(LastMessage);
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -55,6 +55,8 @@
 llvm::BumpPtrAllocator Arena;
 llvm::UniqueStringSaver Strings(Arena);
 ReplyT Reply;
+unsigned Received = 0;
+unsigned FailedToParse = 0;
 wh

[PATCH] D83831: [clangd] Add more logs and attach tracers to remote index server routines

[Sam McCall via Phabricator via cfe-commits]

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 *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

[PATCH] D83831: [clangd] Add more logs and attach tracers to remote index server routines

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 278237.
kbobyrev marked 5 inline comments as done.
kbobyrev added a comment.

Resolve the comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83831

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp
  clang-tools-extra/clangd/index/remote/server/Server.cpp

Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -9,6 +9,8 @@
 #include "index/Index.h"
 #include "index/Serialization.h"
 #include "index/remote/marshalling/Marshalling.h"
+#include "support/Logger.h"
+#include "support/Trace.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Path.h"
@@ -35,6 +37,16 @@
 llvm::cl::opt IndexRoot(llvm::cl::desc(""),
  llvm::cl::Positional, llvm::cl::Required);
 
+llvm::cl::opt TracerFile(
+"tracer-file",
+llvm::cl::desc("Path to the file where tracer logs will be stored"));
+
+llvm::cl::opt PrettyPrint{
+"pretty",
+llvm::cl::desc("Pretty-print JSON output"),
+llvm::cl::init(true),
+};
+
 llvm::cl::opt ServerAddress(
 "server-address", llvm::cl::init("0.0.0.0:50051"),
 llvm::cl::desc("Address of the invoked server. Defaults to 0.0.0.0:50051"));
@@ -57,21 +69,32 @@
   grpc::Status Lookup(grpc::ServerContext *Context,
   const LookupRequest *Request,
   grpc::ServerWriter *Reply) override {
+trace::Span Tracer("LookupRequest");
+SPAN_ATTACH(Tracer, "Request", Request->DebugString());
 clangd::LookupRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Lookup request cancelled: invalid SymbolID {1}", SID.takeError());
 return grpc::Status::CANCELLED;
+  }
   Req.IDs.insert(*SID);
 }
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
 Index->lookup(Req, [&](const clangd::Symbol &Sym) {
   auto SerializedSymbol = toProtobuf(Sym, IndexedProjectRoot);
-  if (!SerializedSymbol)
+  if (!SerializedSymbol) {
+++FailedToSend;
 return;
+  }
   LookupReply NextMessage;
   *NextMessage.mutable_stream_result() = *SerializedSymbol;
   Reply->Write(NextMessage);
+  ++Sent;
 });
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
 LookupReply LastMessage;
 LastMessage.set_final_result(true);
 Reply->Write(LastMessage);
@@ -81,15 +104,24 @@
   grpc::Status FuzzyFind(grpc::ServerContext *Context,
  const FuzzyFindRequest *Request,
  grpc::ServerWriter *Reply) override {
+trace::Span Tracer("FuzzyFind");
+SPAN_ATTACH(Tracer, "Request", Request->DebugString());
 const auto Req = fromProtobuf(Request, IndexedProjectRoot);
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
 bool HasMore = Index->fuzzyFind(Req, [&](const clangd::Symbol &Sym) {
   auto SerializedSymbol = toProtobuf(Sym, IndexedProjectRoot);
-  if (!SerializedSymbol)
+  if (!SerializedSymbol) {
+++FailedToSend;
 return;
+  }
   FuzzyFindReply NextMessage;
   *NextMessage.mutable_stream_result() = *SerializedSymbol;
   Reply->Write(NextMessage);
+  ++Sent;
 });
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
 FuzzyFindReply LastMessage;
 LastMessage.set_final_result(HasMore);
 Reply->Write(LastMessage);
@@ -98,21 +130,32 @@
 
   grpc::Status Refs(grpc::ServerContext *Context, const RefsRequest *Request,
 grpc::ServerWriter *Reply) override {
+trace::Span Tracer("Refs");
+SPAN_ATTACH(Tracer, "Request", Request->DebugString());
 clangd::RefsRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Refs request cancelled: invalid SymbolID {1}", SID.takeError());
 return grpc::Status::CANCELLED;
+  }
   Req.IDs.insert(*SID);
 }
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
 bool HasMore = Index->refs(Req, [&](const clangd::Ref &Reference) {
   auto SerializedRef = toProtobuf(Reference, IndexedProjectRoot);
-  if (!SerializedRef)
+  if (!SerializedRef) {
+++FailedToSend;
 return;
+  }
   RefsReply NextMessage;
   *NextMessage.mutable_stream_result() = *SerializedRef;
   Reply->Write(NextMessage);
+  ++Sent;
 });
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
 RefsReply LastMessage;
 LastMessage.se

[PATCH] D83831: [clangd] Add more logs and attach tracers to remote index server routines

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



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:
> 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
Do you mean logging `RPCRequest` there? Done now.


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

[PATCH] D83831: [clangd] Add more logs and attach tracers to remote index server routines

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 280365.
kbobyrev added a comment.

Rebase on top of master


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

https://reviews.llvm.org/D83831

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp
  clang-tools-extra/clangd/index/remote/server/Server.cpp

Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -9,6 +9,8 @@
 #include "index/Index.h"
 #include "index/Serialization.h"
 #include "index/remote/marshalling/Marshalling.h"
+#include "support/Logger.h"
+#include "support/Trace.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Path.h"
@@ -35,6 +37,16 @@
 llvm::cl::opt IndexRoot(llvm::cl::desc(""),
  llvm::cl::Positional, llvm::cl::Required);

+llvm::cl::opt TracerFile(
+"tracer-file",
+llvm::cl::desc("Path to the file where tracer logs will be stored"));
+
+llvm::cl::opt PrettyPrint{
+"pretty",
+llvm::cl::desc("Pretty-print JSON output"),
+llvm::cl::init(true),
+};
+
 llvm::cl::opt ServerAddress(
 "server-address", llvm::cl::init("0.0.0.0:50051"),
 llvm::cl::desc("Address of the invoked server. Defaults to 0.0.0.0:50051"));
@@ -59,65 +71,96 @@
   grpc::Status Lookup(grpc::ServerContext *Context,
   const LookupRequest *Request,
   grpc::ServerWriter *Reply) override {
+trace::Span Tracer("LookupRequest");
+SPAN_ATTACH(Tracer, "Request", Request->DebugString());
 clangd::LookupRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Lookup request cancelled: invalid SymbolID {1}", SID.takeError());
 return grpc::Status::CANCELLED;
+  }
   Req.IDs.insert(*SID);
 }
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
 Index->lookup(Req, [&](const clangd::Symbol &Sym) {
   auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
-  if (!SerializedSymbol)
+  if (!SerializedSymbol) {
+++FailedToSend;
 return;
+  }
   LookupReply NextMessage;
   *NextMessage.mutable_stream_result() = *SerializedSymbol;
   Reply->Write(NextMessage);
+  ++Sent;
 });
 LookupReply LastMessage;
 LastMessage.set_final_result(true);
 Reply->Write(LastMessage);
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
 return grpc::Status::OK;
   }

   grpc::Status FuzzyFind(grpc::ServerContext *Context,
  const FuzzyFindRequest *Request,
  grpc::ServerWriter *Reply) override {
+trace::Span Tracer("FuzzyFind");
+SPAN_ATTACH(Tracer, "Request", Request->DebugString());
 const auto Req = ProtobufMarshaller->fromProtobuf(Request);
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
 bool HasMore = Index->fuzzyFind(Req, [&](const clangd::Symbol &Sym) {
   auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
-  if (!SerializedSymbol)
+  if (!SerializedSymbol) {
+++FailedToSend;
 return;
+  }
   FuzzyFindReply NextMessage;
   *NextMessage.mutable_stream_result() = *SerializedSymbol;
   Reply->Write(NextMessage);
+  ++Sent;
 });
 FuzzyFindReply LastMessage;
 LastMessage.set_final_result(HasMore);
 Reply->Write(LastMessage);
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
 return grpc::Status::OK;
   }

   grpc::Status Refs(grpc::ServerContext *Context, const RefsRequest *Request,
 grpc::ServerWriter *Reply) override {
+trace::Span Tracer("Refs");
+SPAN_ATTACH(Tracer, "Request", Request->DebugString());
 clangd::RefsRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Refs request cancelled: invalid SymbolID {1}", SID.takeError());
 return grpc::Status::CANCELLED;
+  }
   Req.IDs.insert(*SID);
 }
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
 bool HasMore = Index->refs(Req, [&](const clangd::Ref &Reference) {
   auto SerializedRef = ProtobufMarshaller->toProtobuf(Reference);
-  if (!SerializedRef)
+  if (!SerializedRef) {
+++FailedToSend;
 return;
+  }
   RefsReply NextMessage;
   *NextMessage.mutable_stream_result() = *SerializedRef;
   Reply->Write(NextMessage);
+  ++Sent;
 });
 RefsReply LastMessage;
 LastMessage.set_final_result(HasMore);
 Reply->Write(LastMessage);
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSen

[PATCH] D83831: [clangd] Add more logs and attach tracers to remote index server routines

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev abandoned this revision.
kbobyrev added a comment.

In favor of D84499  with clean diffs.


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

[PATCH] D83831: [clangd] Add more logs and attach tracers to remote index server routines

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 280383.
kbobyrev added a comment.

Reset D83831  to master.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83831

Files:
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
  clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp


Index: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
===
--- clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -282,10 +282,17 @@
 
 TEST(RemoteMarshallingTest, FuzzyFindRequestSerialization) {
   clangd::FuzzyFindRequest Request;
+<<< HEAD
   Request.ProximityPaths = {testPath("local/Header.h"),
 testPath("local/subdir/OtherHeader.h"),
 testPath("remote/File.h"), "Not a Path."};
   Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
+===
+  Request.ProximityPaths = {testPath("remote/Header.h"),
+testPath("remote/subdir/OtherHeader.h"),
+testPath("notremote/File.h"), "Not a Path."};
+  Marshaller ProtobufMarshaller(testPath("remote/"), testPath("home/"));
+>>> 529b83e6cf3... [clangd] Don't send invalid messages from remote index
   auto Serialized = ProtobufMarshaller.toProtobuf(Request);
   EXPECT_EQ(Serialized.proximity_paths_size(), 2);
   auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -51,7 +51,11 @@
 
 clangd::FuzzyFindRequest
 Marshaller::fromProtobuf(const FuzzyFindRequest *Request) {
+<<< HEAD
   assert(RemoteIndexRoot);
+===
+  assert(LocalIndexRoot);
+>>> 529b83e6cf3... [clangd] Don't send invalid messages from remote index
   clangd::FuzzyFindRequest Result;
   Result.Query = Request->query();
   for (const auto &Scope : Request->scopes())
@@ -61,7 +65,11 @@
 Result.Limit = Request->limit();
   Result.RestrictForCodeCompletion = Request->restricted_for_code_completion();
   for (const auto &Path : Request->proximity_paths()) {
+<<< HEAD
 llvm::SmallString<256> LocalPath = llvm::StringRef(*RemoteIndexRoot);
+===
+llvm::SmallString<256> LocalPath = llvm::StringRef(*LocalIndexRoot);
+>>> 529b83e6cf3... [clangd] Don't send invalid messages from remote index
 llvm::sys::path::append(LocalPath, Path);
 Result.ProximityPaths.push_back(std::string(LocalPath));
   }
@@ -146,7 +154,11 @@
 }
 
 FuzzyFindRequest Marshaller::toProtobuf(const clangd::FuzzyFindRequest &From) {
+<<< HEAD
   assert(LocalIndexRoot);
+===
+  assert(RemoteIndexRoot);
+>>> 529b83e6cf3... [clangd] Don't send invalid messages from remote index
   FuzzyFindRequest RPCRequest;
   RPCRequest.set_query(From.Query);
   for (const auto &Scope : From.Scopes)
@@ -157,7 +169,11 @@
   
RPCRequest.set_restricted_for_code_completion(From.RestrictForCodeCompletion);
   for (const auto &Path : From.ProximityPaths) {
 llvm::SmallString<256> RelativePath = llvm::StringRef(Path);
+<<< HEAD
 if (llvm::sys::path::replace_path_prefix(RelativePath, *LocalIndexRoot,
+===
+if (llvm::sys::path::replace_path_prefix(RelativePath, *RemoteIndexRoot,
+>>> 529b83e6cf3... [clangd] Don't send invalid messages from remote index
  ""))
   RPCRequest.add_proximity_paths(llvm::sys::path::convert_to_slash(
   RelativePath, llvm::sys::path::Style::posix));


Index: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
===
--- clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -282,10 +282,17 @@
 
 TEST(RemoteMarshallingTest, FuzzyFindRequestSerialization) {
   clangd::FuzzyFindRequest Request;
+<<< HEAD
   Request.ProximityPaths = {testPath("local/Header.h"),
 testPath("local/subdir/OtherHeader.h"),
 testPath("remote/File.h"), "Not a Path."};
   Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
+===
+  Request.ProximityPaths = {testPath("remote/Header.h"),
+testPath("remote/subdir/OtherHeader.h"),
+testPath("notremote/File.h"), "Not a Path."};
+  Marshaller ProtobufMarshaller(testPath("remote/"), testPath("home/"));
+>>> 529b83e6cf3... [clangd] Don't send invalid messages from remote index
   auto Serialized = ProtobufMarshaller.toProtobu