sammccall updated this revision to Diff 302360.
sammccall added a comment.
Use separate flag for log-public vs log level
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90526/new/
https://reviews.llvm.org/D90526
Files:
clang-tools-extra/clangd/index/remote/server/Server.cpp
clang-tools-extra/clangd/support/Logger.cpp
clang-tools-extra/clangd/support/Logger.h
clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -67,7 +67,8 @@
private:
// Color logs so we can distinguish them from test output.
- void log(Level L, const llvm::formatv_object_base &Message) override {
+ void log(Level L, const char *Fmt,
+ const llvm::formatv_object_base &Message) override {
raw_ostream::Colors Color;
switch (L) {
case Level::Verbose:
Index: clang-tools-extra/clangd/support/Logger.h
===================================================================
--- clang-tools-extra/clangd/support/Logger.h
+++ clang-tools-extra/clangd/support/Logger.h
@@ -24,16 +24,19 @@
public:
virtual ~Logger() = default;
- enum Level { Debug, Verbose, Info, Error };
+ /// The significance or severity of this message.
+ /// Typically used to filter the output to an interesting level.
+ enum Level : unsigned char { Debug, Verbose, Info, Error };
static char indicator(Level L) { return "DVIE"[L]; }
/// Implementations of this method must be thread-safe.
- virtual void log(Level, const llvm::formatv_object_base &Message) = 0;
+ virtual void log(Level, const char *Fmt,
+ const llvm::formatv_object_base &Message) = 0;
};
namespace detail {
const char *debugType(const char *Filename);
-void log(Logger::Level, const llvm::formatv_object_base &);
+void logImpl(Logger::Level, const char *Fmt, const llvm::formatv_object_base &);
// We often want to consume llvm::Errors by value when passing them to log().
// We automatically wrap them in llvm::fmt_consume() as formatv requires.
@@ -43,7 +46,8 @@
}
template <typename... Ts>
void log(Logger::Level L, const char *Fmt, Ts &&... Vals) {
- detail::log(L, llvm::formatv(Fmt, detail::wrap(std::forward<Ts>(Vals))...));
+ detail::logImpl(L, Fmt,
+ llvm::formatv(Fmt, detail::wrap(std::forward<Ts>(Vals))...));
}
llvm::Error error(std::error_code, std::string &&);
@@ -119,7 +123,8 @@
: MinLevel(MinLevel), Logs(Logs) {}
/// Write a line to the logging stream.
- void log(Level, const llvm::formatv_object_base &Message) override;
+ void log(Level, const char *Fmt,
+ const llvm::formatv_object_base &Message) override;
private:
Logger::Level MinLevel;
Index: clang-tools-extra/clangd/support/Logger.cpp
===================================================================
--- clang-tools-extra/clangd/support/Logger.cpp
+++ clang-tools-extra/clangd/support/Logger.cpp
@@ -28,10 +28,10 @@
LoggingSession::~LoggingSession() { L = nullptr; }
-void detail::log(Logger::Level Level,
- const llvm::formatv_object_base &Message) {
+void detail::logImpl(Logger::Level Level, const char *Fmt,
+ const llvm::formatv_object_base &Message) {
if (L)
- L->log(Level, Message);
+ L->log(Level, Fmt, Message);
else {
static std::mutex Mu;
std::lock_guard<std::mutex> Guard(Mu);
@@ -47,7 +47,7 @@
return Filename;
}
-void StreamLogger::log(Logger::Level Level,
+void StreamLogger::log(Logger::Level Level, const char *Fmt,
const llvm::formatv_object_base &Message) {
if (Level < MinLevel)
return;
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
@@ -12,6 +12,7 @@
#include "index/Serialization.h"
#include "index/Symbol.h"
#include "index/remote/marshalling/Marshalling.h"
+#include "support/Context.h"
#include "support/Logger.h"
#include "support/Shutdown.h"
#include "support/ThreadsafeFS.h"
@@ -23,6 +24,7 @@
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/VirtualFileSystem.h"
@@ -30,6 +32,7 @@
#include <chrono>
#include <grpc++/grpc++.h>
#include <grpc++/health_check_service_interface.h>
+#include <grpcpp/impl/codegen/server_context.h>
#include <memory>
#include <thread>
@@ -58,6 +61,12 @@
llvm::cl::init(Logger::Info),
};
+llvm::cl::opt<bool> LogPublic{
+ "log-public",
+ llvm::cl::desc("Avoid logging potentially-sensitive request details"),
+ llvm::cl::init(false),
+};
+
llvm::cl::opt<std::string> TraceFile(
"trace-file",
llvm::cl::desc("Path to the file where tracer logs will be stored"));
@@ -72,6 +81,8 @@
"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"));
+static Key<grpc::ServerContext *> CurrentRequest;
+
class RemoteIndexServer final : public v1::SymbolIndex::Service {
public:
RemoteIndexServer(clangd::SymbolIndex &Index, llvm::StringRef IndexRoot)
@@ -87,6 +98,7 @@
grpc::Status Lookup(grpc::ServerContext *Context,
const LookupRequest *Request,
grpc::ServerWriter<LookupReply> *Reply) override {
+ WithContextValue(CurrentRequest, Context);
trace::Span Tracer("LookupRequest");
auto Req = ProtobufMarshaller->fromProtobuf(Request);
if (!Req) {
@@ -119,6 +131,7 @@
grpc::Status FuzzyFind(grpc::ServerContext *Context,
const FuzzyFindRequest *Request,
grpc::ServerWriter<FuzzyFindReply> *Reply) override {
+ WithContextValue(CurrentRequest, Context);
trace::Span Tracer("FuzzyFindRequest");
auto Req = ProtobufMarshaller->fromProtobuf(Request);
if (!Req) {
@@ -151,6 +164,7 @@
grpc::Status Refs(grpc::ServerContext *Context, const RefsRequest *Request,
grpc::ServerWriter<RefsReply> *Reply) override {
+ WithContextValue(CurrentRequest, Context);
trace::Span Tracer("RefsRequest");
auto Req = ProtobufMarshaller->fromProtobuf(Request);
if (!Req) {
@@ -183,6 +197,7 @@
grpc::Status Relations(grpc::ServerContext *Context,
const RelationsRequest *Request,
grpc::ServerWriter<RelationsReply> *Reply) override {
+ WithContextValue(CurrentRequest, Context);
trace::Span Tracer("RelationsRequest");
auto Req = ProtobufMarshaller->fromProtobuf(Request);
if (!Req) {
@@ -229,8 +244,8 @@
LastStatus.getLastModificationTime() &&
Status->getSize() == LastStatus.getSize()))
return;
- vlog("Found different index version: existing index was modified at {0}, new "
- "index was modified at {1}. Attempting to reload.",
+ vlog("Found different index version: existing index was modified at "
+ "{0}, new index was modified at {1}. Attempting to reload.",
LastStatus.getLastModificationTime(), Status->getLastModificationTime());
LastStatus = *Status;
std::unique_ptr<clang::clangd::SymbolIndex> NewIndex = loadIndex(IndexPath);
@@ -266,6 +281,29 @@
ServerShutdownWatcher.join();
}
+std::unique_ptr<Logger> makeLogger(llvm::raw_ostream &OS) {
+ if (!LogPublic)
+ return std::make_unique<StreamLogger>(OS, LogLevel);
+ // Redacted mode:
+ // - messages outside the scope of a request: log fully
+ // - messages tagged [public]: log fully
+ // - errors: log the format string
+ // - others: drop
+ class RedactedLogger : public StreamLogger {
+ public:
+ using StreamLogger::StreamLogger;
+ void log(Level L, const char *Fmt,
+ const llvm::formatv_object_base &Message) override {
+ if (Context::current().get(CurrentRequest) == nullptr ||
+ llvm::StringRef(Fmt).startswith("[public]"))
+ return StreamLogger::log(L, Fmt, Message);
+ if (L >= Error)
+ return StreamLogger::log(L, Fmt, llvm::formatv("[redacted] {0}", Fmt));
+ }
+ };
+ return std::make_unique<RedactedLogger>(OS, LogLevel);
+}
+
} // namespace
} // namespace remote
} // namespace clangd
@@ -287,8 +325,8 @@
llvm::errs().SetBuffered();
// Don't flush stdout when logging for thread safety.
llvm::errs().tie(nullptr);
- clang::clangd::StreamLogger Logger(llvm::errs(), LogLevel);
- clang::clangd::LoggingSession LoggingSession(Logger);
+ auto Logger = makeLogger(llvm::errs());
+ clang::clangd::LoggingSession LoggingSession(*Logger);
llvm::Optional<llvm::raw_fd_ostream> TracerStream;
std::unique_ptr<clang::clangd::trace::EventTracer> Tracer;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
