[clang-tools-extra] 9b88a19 - [clangd] Add CSV export for trace metrics
Voss, Matthew via cfe-commits
cfe-commits at lists.llvm.org
Tue May 19 10:39:24 PDT 2020
Looks good. Thanks!
Matthew
________________________________
From: Sam McCall <sam.mccall at gmail.com>
Sent: Tuesday, May 19, 2020 9:19 AM
To: Voss, Matthew <Matthew.Voss at sony.com>
Cc: cfe-commits <cfe-commits at lists.llvm.org>; Sam McCall <llvmlistbot at llvm.org>
Subject: Re: [clang-tools-extra] 9b88a19 - [clangd] Add CSV export for trace metrics
Sorry about that Matthew, I think 5bc0c8f009261425a should have fixed this (giving up on stringview entirely). It fixed the other windows based bots.
If it's no good, please do revert if you can, I'll check on it shortly too.
On Tue, May 19, 2020, 6:14 PM Voss, Matthew <Matthew.Voss at sony.com<mailto:Matthew.Voss at sony.com>> wrote:
Hi Sam,
This commit's tests are failing to compile on the PS4 Windows bot and our internal CI. Could you take a look? Just FYI, I may end up reverting this after an hour or so to unclog our CI.
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/32433
Thanks,
Matthew
________________________________
From: cfe-commits <cfe-commits-bounces at lists.llvm.org<mailto:cfe-commits-bounces at lists.llvm.org>> on behalf of Sam McCall via cfe-commits <cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>>
Sent: Tuesday, May 19, 2020 4:36 AM
To: cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org> <cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>>
Subject: [clang-tools-extra] 9b88a19 - [clangd] Add CSV export for trace metrics
Author: Sam McCall
Date: 2020-05-19T13:35:31+02:00
New Revision: 9b88a190b42a03753b9c49ccea34514cb40ba4ab
URL: https://github.com/llvm/llvm-project/commit/9b88a190b42a03753b9c49ccea34514cb40ba4ab
DIFF: https://github.com/llvm/llvm-project/commit/9b88a190b42a03753b9c49ccea34514cb40ba4ab.diff
LOG: [clangd] Add CSV export for trace metrics
Summary: Example: https://docs.google.com/spreadsheets/d/1VZKGetSUTTDe9p4ooIETmdcwUod1_aE3vgD0E9x7HhI/edit
Reviewers: kadircet
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, jfb, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D79678
Added:
clang-tools-extra/clangd/test/metrics.test
Modified:
clang-tools-extra/clangd/support/Trace.cpp
clang-tools-extra/clangd/support/Trace.h
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/support/TraceTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/support/Trace.cpp b/clang-tools-extra/clangd/support/Trace.cpp
index 6bf4816268e5..10ae461221ee 100644
--- a/clang-tools-extra/clangd/support/Trace.cpp
+++ b/clang-tools-extra/clangd/support/Trace.cpp
@@ -189,6 +189,66 @@ class JSONTracer : public EventTracer {
const llvm::sys::TimePoint<> Start;
};
+// We emit CSV as specified in RFC 4180: https://www.ietf.org/rfc/rfc4180.txt.
+// \r\n line endings are used, cells with \r\n," are quoted, quotes are doubled.
+class CSVMetricTracer : public EventTracer {
+public:
+ CSVMetricTracer(llvm::raw_ostream &Out) : Out(Out) {
+ Start = std::chrono::steady_clock::now();
+
+ Out.SetUnbuffered(); // We write each line atomically.
+ Out << "Kind,Metric,Label,Value,Timestamp\r\n";
+ }
+
+ void record(const Metric &Metric, double Value,
+ llvm::StringRef Label) override {
+ assert(!needsQuote(Metric.Name));
+ std::string QuotedLabel;
+ if (needsQuote(Label))
+ Label = QuotedLabel = quote(Label);
+ uint64_t Micros = std::chrono::duration_cast<std::chrono::microseconds>(
+ std::chrono::steady_clock::now() - Start)
+ .count();
+ std::lock_guard<std::mutex> Lock(Mu);
+ Out << llvm::formatv("{0},{1},{2},{3:e},{4}.{5:6}\r\n",
+ typeName(Metric.Type), Metric.Name, Label, Value,
+ Micros / 1000000, Micros % 1000000);
+ }
+
+private:
+ llvm::StringRef typeName(Metric::MetricType T) {
+ switch (T) {
+ case Metric::Value:
+ return "v";
+ case Metric::Counter:
+ return "c";
+ case Metric::Distribution:
+ return "d";
+ }
+ }
+
+ static bool needsQuote(llvm::StringRef Text) {
+ // https://www.ietf.org/rfc/rfc4180.txt section 2.6
+ return Text.find_first_of(",\"\r\n") != llvm::StringRef::npos;
+ }
+
+ std::string quote(llvm::StringRef Text) {
+ std::string Result = "\"";
+ for (char C : Text) {
+ Result.push_back(C);
+ if (C == '"')
+ Result.push_back('"');
+ }
+ Result.push_back('"');
+ return Result;
+ }
+
+private:
+ std::mutex Mu;
+ llvm::raw_ostream &Out /*GUARDED_BY(Mu)*/;
+ std::chrono::steady_clock::time_point Start;
+};
+
Key<std::unique_ptr<JSONTracer::JSONSpan>> JSONTracer::SpanKey;
EventTracer *T = nullptr;
@@ -206,6 +266,10 @@ std::unique_ptr<EventTracer> createJSONTracer(llvm::raw_ostream &OS,
return std::make_unique<JSONTracer>(OS, Pretty);
}
+std::unique_ptr<EventTracer> createCSVMetricTracer(llvm::raw_ostream &OS) {
+ return std::make_unique<CSVMetricTracer>(OS);
+}
+
void log(const llvm::Twine &Message) {
if (!T)
return;
diff --git a/clang-tools-extra/clangd/support/Trace.h b/clang-tools-extra/clangd/support/Trace.h
index 90a11bb1feb4..9dc397a84b74 100644
--- a/clang-tools-extra/clangd/support/Trace.h
+++ b/clang-tools-extra/clangd/support/Trace.h
@@ -108,11 +108,18 @@ class Session {
/// Create an instance of EventTracer that produces an output in the Trace Event
/// format supported by Chrome's trace viewer (chrome://tracing).
///
+/// FIXME: Metrics are not recorded, some could become counter events.
+///
/// The format is documented here:
/// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview
std::unique_ptr<EventTracer> createJSONTracer(llvm::raw_ostream &OS,
bool Pretty = false);
+/// Create an instance of EventTracer that outputs metric measurements as CSV.
+///
+/// Trace spans and instant events are ignored.
+std::unique_ptr<EventTracer> createCSVMetricTracer(llvm::raw_ostream &OS);
+
/// Records a single instant event, associated with the current thread.
void log(const llvm::Twine &Name);
diff --git a/clang-tools-extra/clangd/test/metrics.test b/clang-tools-extra/clangd/test/metrics.test
new file mode 100644
index 000000000000..874a5dad8308
--- /dev/null
+++ b/clang-tools-extra/clangd/test/metrics.test
@@ -0,0 +1,11 @@
+# RUN: env CLANGD_METRICS=%t clangd -lit-test < %s && FileCheck %s < %t
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void main() {}"}}}
+# Don't verify value, timestamp, or order.
+# CHECK-DAG: d,lsp_latency,textDocument/didOpen,
+# CHECK-DAG: c,ast_access_diag,miss,
+---
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 566430167a51..e59aecb2d752 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -539,18 +539,23 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
// Setup tracing facilities if CLANGD_TRACE is set. In practice enabling a
// trace flag in your editor's config is annoying, launching with
// `CLANGD_TRACE=trace.json vim` is easier.
- llvm::Optional<llvm::raw_fd_ostream> TraceStream;
+ llvm::Optional<llvm::raw_fd_ostream> TracerStream;
std::unique_ptr<trace::EventTracer> Tracer;
- if (auto *TraceFile = getenv("CLANGD_TRACE")) {
+ const char *JSONTraceFile = getenv("CLANGD_TRACE");
+ const char *MetricsCSVFile = getenv("CLANGD_METRICS");
+ const char *TracerFile = JSONTraceFile ? JSONTraceFile : MetricsCSVFile;
+ if (TracerFile) {
std::error_code EC;
- TraceStream.emplace(TraceFile, /*ref*/ EC,
- llvm::sys::fs::FA_Read | llvm::sys::fs::FA_Write);
+ TracerStream.emplace(TracerFile, /*ref*/ EC,
+ llvm::sys::fs::FA_Read | llvm::sys::fs::FA_Write);
if (EC) {
- TraceStream.reset();
- llvm::errs() << "Error while opening trace file " << TraceFile << ": "
+ TracerStream.reset();
+ llvm::errs() << "Error while opening trace file " << TracerFile << ": "
<< EC.message();
} else {
- Tracer = trace::createJSONTracer(*TraceStream, PrettyPrint);
+ Tracer = (TracerFile == JSONTraceFile)
+ ? trace::createJSONTracer(*TracerStream, PrettyPrint)
+ : trace::createCSVMetricTracer(*TracerStream);
}
}
diff --git a/clang-tools-extra/clangd/unittests/support/TraceTests.cpp b/clang-tools-extra/clangd/unittests/support/TraceTests.cpp
index 10670f79be1b..cee43f73f211 100644
--- a/clang-tools-extra/clangd/unittests/support/TraceTests.cpp
+++ b/clang-tools-extra/clangd/unittests/support/TraceTests.cpp
@@ -11,6 +11,7 @@
#include "support/Trace.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/SourceMgr.h"
#include "llvm/Support/Threading.h"
@@ -22,7 +23,11 @@ namespace clang {
namespace clangd {
namespace {
+using testing::_;
+using testing::ElementsAre;
+using testing::MatchesRegex;
using testing::SizeIs;
+using testing::StartsWith;
MATCHER_P(StringNode, Val, "") {
if (arg->getType() != llvm::yaml::Node::NK_Scalar) {
@@ -138,6 +143,51 @@ TEST(MetricsTracer, LatencyTest) {
EXPECT_THAT(Tracer.takeMetric(MetricName, OpName), SizeIs(1));
}
+class CSVMetricsTracerTest : public ::testing::Test {
+protected:
+ CSVMetricsTracerTest()
+ : OS(Output), Tracer(trace::createCSVMetricTracer(OS)), Session(*Tracer) {
+ }
+ trace::Metric Dist = {"dist", trace::Metric::Distribution, "lbl"};
+ trace::Metric Counter = {"cnt", trace::Metric::Counter};
+
+ std::vector<llvm::StringRef> outputLines() {
+ // Deliberately don't flush output stream, the tracer should do that.
+ // This is important when clangd crashes.
+ llvm::SmallVector<llvm::StringRef, 4> Lines;
+ llvm::StringRef(Output).split(Lines, "\r\n");
+ return {Lines.begin(), Lines.end()};
+ }
+
+ std::string Output;
+ llvm::raw_string_ostream OS;
+ std::unique_ptr<trace::EventTracer> Tracer;
+ trace::Session Session;
+};
+
+TEST_F(CSVMetricsTracerTest, RecordsValues) {
+ Dist.record(1, "x");
+ Counter.record(1, "");
+ Dist.record(2, "y");
+
+ EXPECT_THAT(
+ outputLines(),
+ ElementsAre("Kind,Metric,Label,Value,Timestamp",
+ MatchesRegex(R"(d,dist,x,1\.000000e\+00,[0-9]+\.[0-9]{6})"),
+ StartsWith("c,cnt,,1.000000e+00,"),
+ StartsWith("d,dist,y,2.000000e+00,"), ""));
+}
+
+TEST_F(CSVMetricsTracerTest, Escaping) {
+ Dist.record(1, ",");
+ Dist.record(1, "a\"b");
+ Dist.record(1, "a\nb");
+
+ EXPECT_THAT(outputLines(), ElementsAre(_, StartsWith(R"(d,dist,",",1)"),
+ StartsWith(R"(d,dist,"a""b",1)"),
+ StartsWith("d,dist,\"a\nb\",1"), ""));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200519/0eae4b16/attachment-0001.html>
More information about the cfe-commits
mailing list