[PATCH] D90526: [clangd] Add -log=public to redact all request info from index server logs

Sat Oct 31 03:47:48 PDT 2020

sammccall added a comment.

Couple of notes on this patch.

There's no actual request logging here yet. My plan is something like `[public] request v1/Relations => SUCCESS: 3 results in 3ms`. Wanted to keep the patch a reasonable size.

There's also no testing, testing is definitely important here. I want to build something on top of Kirill's python tests once that lands. Probably in the patch that adds request logging.

The `[public]` thing is gross, but I think worse is better here.
I went through lots of iterations, including with explicit "Sensitivity" enum passed around, but it was pretty invasive. 
Moreover explicitly supporting it in `Logger.h` made me feel like all the callsites in all our libraries should consider whether info is public or not. I think spreading that cognitive load around the whole project is a net negative.
With the request-context detection (which was a last-minute realization actually), maybe we could even get away without `[public]`, but I didn't want to commit to contorting our code so that the request logging happens outside the request scope. Happy to drop this part if you think we can make it work.

Weird extension idea: we could have a completely synthetic request ID to stick on the request log lines and also the pattern-only elog()s to tie them together.

Changing the Logger API is a bit annoying, but has one side-benefit: error count grouped by format string is a nice thing to be able to monitor.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90526