Merge pull request #13012 from github/redsun82/swift-json

redsun82 · web-flow · commit b511c5f355f7 · 2023-05-04T17:04:45.000+02:00
Swift: add infrastructure for emitting JSON diagnostics
diff --git a/misc/bazel/workspace.bzl b/misc/bazel/workspace.bzl
@@ -43,13 +43,3 @@ def codeql_workspace(repository_name = "codeql"):
             "https://github.com/bazelbuild/bazel-skylib/releases/download/1.4.1/bazel-skylib-1.4.1.tar.gz",
         ],
     )
-
-    maybe(
-        repo_rule = http_archive,
-        name = "absl",
-        sha256 = "cec2e5bf780532bd0ac672eb8d43c0f8bbe84ca5df8718320184034b7f59a398",
-        urls = [
-            "https://github.com/abseil/abseil-cpp/archive/d2c5297a3c3948de765100cb7e5cccca1210d23c.tar.gz",
-        ],
-        strip_prefix = "abseil-cpp-d2c5297a3c3948de765100cb7e5cccca1210d23c",
-    )
diff --git a/swift/extractor/SwiftExtractor.cpp b/swift/extractor/SwiftExtractor.cpp
@@ -15,7 +15,7 @@
 #include "swift/extractor/infra/SwiftLocationExtractor.h"
 #include "swift/extractor/infra/SwiftBodyEmissionStrategy.h"
 #include "swift/extractor/mangler/SwiftMangler.h"
-#include "swift/extractor/infra/log/SwiftAssert.h"
+#include "swift/logging/SwiftAssert.h"
 
 using namespace codeql;
 using namespace std::string_literals;
diff --git a/swift/extractor/infra/BUILD.bazel b/swift/extractor/infra/BUILD.bazel
@@ -8,8 +8,8 @@ swift_cc_library(
     deps = [
         "//swift/extractor/config",
         "//swift/extractor/infra/file",
-        "//swift/extractor/infra/log",
         "//swift/extractor/trap",
+        "//swift/logging",
         "//swift/third_party/swift-llvm-support",
     ],
 )
diff --git a/swift/extractor/infra/SwiftDispatcher.h b/swift/extractor/infra/SwiftDispatcher.h
@@ -13,7 +13,7 @@
 #include "swift/extractor/infra/SwiftBodyEmissionStrategy.h"
 #include "swift/extractor/infra/SwiftMangledName.h"
 #include "swift/extractor/config/SwiftExtractorState.h"
-#include "swift/extractor/infra/log/SwiftAssert.h"
+#include "swift/logging/SwiftAssert.h"
 
 namespace codeql {
 
diff --git a/swift/extractor/infra/file/BUILD.bazel b/swift/extractor/infra/file/BUILD.bazel
@@ -2,10 +2,16 @@ load("//swift:rules.bzl", "swift_cc_library")
 
 swift_cc_library(
     name = "file",
-    srcs = glob(["*.cpp", "FsLogger.h"]),
-    hdrs = glob(["*.h"], exclude=["FsLogger.h"]) + [":path_hash_workaround"],
+    srcs = glob([
+        "*.cpp",
+        "FsLogger.h",
+    ]),
+    hdrs = glob(
+        ["*.h"],
+        exclude = ["FsLogger.h"],
+    ) + [":path_hash_workaround"],
     visibility = ["//swift:__subpackages__"],
-    deps = ["//swift/extractor/infra/log"],
+    deps = ["//swift/logging"],
 )
 
 genrule(
diff --git a/swift/extractor/infra/file/FsLogger.h b/swift/extractor/infra/file/FsLogger.h
@@ -1,4 +1,4 @@
-#include "swift/extractor/infra/log/SwiftLogging.h"
+#include "swift/logging/SwiftLogging.h"
 
 namespace codeql {
 namespace fs_logger {
diff --git a/swift/extractor/infra/file/TargetFile.cpp b/swift/extractor/infra/file/TargetFile.cpp
@@ -1,7 +1,7 @@
 #include "swift/extractor/infra/file/TargetFile.h"
 #include "swift/extractor/infra/file/FsLogger.h"
-#include "swift/extractor/infra/log/SwiftLogging.h"
-#include "swift/extractor/infra/log/SwiftAssert.h"
+#include "swift/logging/SwiftLogging.h"
+#include "swift/logging/SwiftAssert.h"
 
 #include <cassert>
 #include <cstdio>
diff --git a/swift/extractor/invocation/SwiftInvocationExtractor.cpp b/swift/extractor/invocation/SwiftInvocationExtractor.cpp
@@ -4,7 +4,7 @@
 #include "swift/extractor/trap/generated/TrapTags.h"
 #include "swift/extractor/infra/file/TargetFile.h"
 #include "swift/extractor/infra/file/Path.h"
-#include "swift/extractor/infra/log/SwiftAssert.h"
+#include "swift/logging/SwiftAssert.h"
 #include "swift/extractor/mangler/SwiftMangler.h"
 
 namespace fs = std::filesystem;
diff --git a/swift/extractor/main.cpp b/swift/extractor/main.cpp
@@ -18,12 +18,12 @@
 #include "swift/extractor/invocation/SwiftInvocationExtractor.h"
 #include "swift/extractor/trap/TrapDomain.h"
 #include "swift/extractor/infra/file/Path.h"
-#include "swift/extractor/infra/log/SwiftAssert.h"
+#include "swift/logging/SwiftAssert.h"
 
 using namespace std::string_literals;
 using namespace codeql::main_logger;
 
-const std::string_view codeql::logRootName = "extractor";
+const std::string_view codeql::programName = "extractor";
 
 // must be called before processFrontendOptions modifies output paths
 static void lockOutputSwiftModuleTraps(codeql::SwiftExtractorState& state,
diff --git a/swift/extractor/remapping/SwiftFileInterception.cpp b/swift/extractor/remapping/SwiftFileInterception.cpp
@@ -14,7 +14,7 @@
 
 #include "swift/extractor/infra/file/PathHash.h"
 #include "swift/extractor/infra/file/Path.h"
-#include "swift/extractor/infra/log/SwiftAssert.h"
+#include "swift/logging/SwiftAssert.h"
 
 #ifdef __APPLE__
 // path is hardcoded as otherwise redirection could break when setting DYLD_FALLBACK_LIBRARY_PATH
diff --git a/swift/extractor/trap/BUILD.bazel b/swift/extractor/trap/BUILD.bazel
@@ -23,7 +23,10 @@ genrule(
         "--schema=$(location //swift:schema)",
         "--script-name=codegen/codegen.py",
     ]),
-    exec_tools = ["//misc/codegen", "//swift:schema"],
+    exec_tools = [
+        "//misc/codegen",
+        "//swift:schema",
+    ],
 )
 
 filegroup(
@@ -49,7 +52,7 @@ swift_cc_library(
     visibility = ["//visibility:public"],
     deps = [
         "//swift/extractor/infra/file",
-        "//swift/extractor/infra/log",
+        "//swift/logging",
         "@absl//absl/numeric:bits",
     ],
 )
diff --git a/swift/extractor/trap/TrapDomain.h b/swift/extractor/trap/TrapDomain.h
@@ -5,7 +5,7 @@
 
 #include "swift/extractor/trap/TrapLabel.h"
 #include "swift/extractor/infra/file/TargetFile.h"
-#include "swift/extractor/infra/log/SwiftLogging.h"
+#include "swift/logging/SwiftLogging.h"
 #include "swift/extractor/infra/SwiftMangledName.h"
 
 namespace codeql {
diff --git a/swift/logging/BUILD.bazel b/swift/logging/BUILD.bazel
@@ -1,7 +1,11 @@
 cc_library(
-    name = "log",
+    name = "logging",
     srcs = glob(["*.cpp"]),
     hdrs = glob(["*.h"]),
     visibility = ["//visibility:public"],
-    deps = ["@binlog"],
+    deps = [
+        "@absl//absl/strings",
+        "@binlog",
+        "@json",
+    ],
 )
diff --git a/swift/logging/SwiftAssert.h b/swift/logging/SwiftAssert.h
@@ -2,7 +2,7 @@
 
 #include <cstdlib>
 
-#include "swift/extractor/infra/log/SwiftLogging.h"
+#include "swift/logging/SwiftLogging.h"
 
 // assert CONDITION, which is always evaluated (once) regardless of the build type. If
 // CONDITION is not satisfied, emit a critical log optionally using provided format and arguments,
diff --git a/swift/logging/SwiftDiagnostics.cpp b/swift/logging/SwiftDiagnostics.cpp
@@ -0,0 +1,68 @@
+#include "swift/logging/SwiftDiagnostics.h"
+
+#include <binlog/Entries.hpp>
+#include <nlohmann/json.hpp>
+#include "absl/strings/str_join.h"
+#include "absl/strings/str_cat.h"
+#include "absl/strings/str_split.h"
+#include "swift/logging/SwiftAssert.h"
+
+namespace codeql {
+
+namespace {
+Logger& logger() {
+  static Logger ret{"diagnostics"};
+  return ret;
+}
+}  // namespace
+
+void SwiftDiagnosticsSource::emit(std::ostream& out,
+                                  std::string_view timestamp,
+                                  std::string_view message) const {
+  nlohmann::json entry = {
+      {"source",
+       {
+           {"id", sourceId()},
+           {"name", name},
+           {"extractorName", extractorName},
+       }},
+      {"visibility",
+       {
+           {"statusPage", true},
+           {"cliSummaryTable", true},
+           {"telemetry", true},
+       }},
+      {"severity", "error"},
+      {"helpLinks", std::vector<std::string_view>(absl::StrSplit(helpLinks, ' '))},
+      {"plaintextMessage", absl::StrCat(message, ".\n\n", action, ".")},
+      {"timestamp", timestamp},
+  };
+  out << entry << '\n';
+}
+
+std::string SwiftDiagnosticsSource::sourceId() const {
+  auto ret = absl::StrJoin({extractorName, programName, id}, "/");
+  std::replace(ret.begin(), ret.end(), '_', '-');
+  return ret;
+}
+void SwiftDiagnosticsSource::registerImpl(const SwiftDiagnosticsSource* source) {
+  auto [it, inserted] = map().emplace(source->id, source);
+  CODEQL_ASSERT(inserted, "duplicate diagnostics source detected with id {}", source->id);
+}
+
+void SwiftDiagnosticsDumper::write(const char* buffer, std::size_t bufferSize) {
+  binlog::Range range{buffer, bufferSize};
+  binlog::RangeEntryStream input{range};
+  while (auto event = events.nextEvent(input)) {
+    const auto& source = SwiftDiagnosticsSource::get(event->source->category);
+    std::ostringstream oss;
+    timestampedMessagePrinter.printEvent(oss, *event, events.writerProp(), events.clockSync());
+    // TODO(C++20) use oss.view() directly
+    auto data = oss.str();
+    std::string_view view = data;
+    using ViewPair = std::pair<std::string_view, std::string_view>;
+    auto [timestamp, message] = ViewPair(absl::StrSplit(view, absl::MaxSplits(' ', 1)));
+    source.emit(output, timestamp, message);
+  }
+}
+}  // namespace codeql
diff --git a/swift/logging/SwiftDiagnostics.h b/swift/logging/SwiftDiagnostics.h
@@ -0,0 +1,88 @@
+#pragma once
+
+#include <binlog/EventStream.hpp>
+#include <binlog/PrettyPrinter.hpp>
+#include <string>
+#include <vector>
+#include <unordered_map>
+#include <cassert>
+#include <fstream>
+#include <filesystem>
+#include <sstream>
+#include <mutex>
+
+namespace codeql {
+
+extern const std::string_view programName;
+
+// Models a diagnostic source for Swift, holding static information that goes out into a diagnostic
+// These are internally stored into a map on id's. A specific error log can use binlog's category
+// as id, which will then be used to recover the diagnostic source while dumping.
+struct SwiftDiagnosticsSource {
+  std::string_view id;
+  std::string_view name;
+  static constexpr std::string_view extractorName = "swift";
+  std::string_view action;
+  // space separated if more than 1. Not a vector to allow constexpr
+  // TODO(C++20) with vector going constexpr this can be turned to `std::vector<std::string_view>`
+  std::string_view helpLinks;
+
+  // for the moment, we only output errors, so no need to store the severity
+
+  // registers a diagnostics source for later retrieval with get, if not done yet
+  template <const SwiftDiagnosticsSource* Source>
+  static void ensureRegistered() {
+    static std::once_flag once;
+    std::call_once(once, registerImpl, Source);
+  }
+
+  // gets a previously inscribed SwiftDiagnosticsSource for the given id. Will abort if none exists
+  static const SwiftDiagnosticsSource& get(const std::string& id) { return *map().at(id); }
+
+  // emit a JSON diagnostics for this source with the given timestamp and message to out
+  // A plaintextMessage is used that includes both the message and the action to take. Dots are
+  // appended to both. The id is used to construct the source id in the form
+  // `swift/<prog name>/<id with '-' replacing '_'>`
+  void emit(std::ostream& out, std::string_view timestamp, std::string_view message) const;
+
+ private:
+  static void registerImpl(const SwiftDiagnosticsSource* source);
+
+  std::string sourceId() const;
+  using Map = std::unordered_map<std::string, const SwiftDiagnosticsSource*>;
+
+  static Map& map() {
+    static Map ret;
+    return ret;
+  }
+};
+
+// An output modeling binlog's output stream concept that intercepts binlog entries and translates
+// them to appropriate diagnostics JSON entries
+class SwiftDiagnosticsDumper {
+ public:
+  // opens path for writing out JSON entries. Returns whether the operation was successful.
+  bool open(const std::filesystem::path& path) {
+    output.open(path);
+    return output.good();
+  }
+
+  // write out binlog entries as corresponding JSON diagnostics entries. Expects all entries to have
+  // a category equal to an id of a previously created SwiftDiagnosticSource.
+  void write(const char* buffer, std::size_t bufferSize);
+
+ private:
+  binlog::EventStream events;
+  std::ofstream output;
+  binlog::PrettyPrinter timestampedMessagePrinter{"%u %m", "%Y-%m-%dT%H:%M:%S.%NZ"};
+};
+
+}  // namespace codeql
+
+namespace codeql_diagnostics {
+constexpr codeql::SwiftDiagnosticsSource internal_error{
+    "internal_error",
+    "Internal error",
+    "Contact us about this issue",
+};
+}  // namespace codeql_diagnostics
diff --git a/swift/logging/SwiftLogging.cpp b/swift/logging/SwiftLogging.cpp
@@ -1,4 +1,4 @@
-#include "swift/extractor/infra/log/SwiftLogging.h"
+#include "swift/logging/SwiftLogging.h"
 
 #include <filesystem>
 #include <stdlib.h>
@@ -50,7 +50,7 @@ Log::Level matchToLevel(std::csub_match m) {
 
 }  // namespace
 
-std::vector<std::string> Log::collectSeverityRulesAndReturnProblems(const char* envVar) {
+std::vector<std::string> Log::collectLevelRulesAndReturnProblems(const char* envVar) {
   std::vector<std::string> problems;
   if (auto levels = getEnvOr(envVar, nullptr)) {
     // expect comma-separated <glob pattern>:<log severity>
@@ -92,12 +92,13 @@ std::vector<std::string> Log::collectSeverityRulesAndReturnProblems(const char*
 
 void Log::configure() {
   // as we are configuring logging right now, we collect problems and log them at the end
-  auto problems = collectSeverityRulesAndReturnProblems("CODEQL_EXTRACTOR_SWIFT_LOG_LEVELS");
+  auto problems = collectLevelRulesAndReturnProblems("CODEQL_EXTRACTOR_SWIFT_LOG_LEVELS");
+  auto now = std::to_string(std::chrono::system_clock::now().time_since_epoch().count());
   if (text || binary) {
     std::filesystem::path logFile = getEnvOr("CODEQL_EXTRACTOR_SWIFT_LOG_DIR", "extractor-out/log");
     logFile /= "swift";
-    logFile /= logRootName;
-    logFile /= std::to_string(std::chrono::system_clock::now().time_since_epoch().count());
+    logFile /= programName;
+    logFile /= now;
     std::error_code ec;
     std::filesystem::create_directories(logFile.parent_path(), ec);
     if (!ec) {
@@ -123,6 +124,25 @@ void Log::configure() {
       binary.level = Level::no_logs;
       text.level = Level::no_logs;
     }
+    if (diagnostics) {
+      std::filesystem::path diagFile =
+          getEnvOr("CODEQL_EXTRACTOR_SWIFT_DIAGNOSTIC_DIR", "extractor-out/diagnostics");
+      diagFile /= programName;
+      diagFile /= now;
+      diagFile.replace_extension(".jsonl");
+      std::error_code ec;
+      std::filesystem::create_directories(diagFile.parent_path(), ec);
+      if (!ec) {
+        if (!diagnostics.output.open(diagFile)) {
+          problems.emplace_back("Unable to open diagnostics json file " + diagFile.string());
+          diagnostics.level = Level::no_logs;
+        }
+      } else {
+        problems.emplace_back("Unable to create diagnostics directory " +
+                              diagFile.parent_path().string() + ": " + ec.message());
+        diagnostics.level = Level::no_logs;
+      }
+    }
   }
   for (const auto& problem : problems) {
     LOG_ERROR("{}", problem);
@@ -143,7 +163,7 @@ void Log::flushImpl() {
 }
 
 Log::LoggerConfiguration Log::getLoggerConfigurationImpl(std::string_view name) {
-  LoggerConfiguration ret{session, std::string{logRootName}};
+  LoggerConfiguration ret{session, std::string{programName}};
   ret.fullyQualifiedName += '/';
   ret.fullyQualifiedName += name;
   ret.level = std::min({binary.level, text.level, console.level});
@@ -159,6 +179,7 @@ Log& Log::write(const char* buffer, std::streamsize size) {
   if (text) text.write(buffer, size);
   if (binary) binary.write(buffer, size);
   if (console) console.write(buffer, size);
+  if (diagnostics) diagnostics.write(buffer, size);
   return *this;
 }
 
diff --git a/swift/logging/SwiftLogging.h b/swift/logging/SwiftLogging.h
diff --git a/swift/third_party/BUILD.binlog.bazel b/swift/third_party/BUILD.binlog.bazel
diff --git a/swift/third_party/BUILD.picosha2.bazel b/swift/third_party/BUILD.picosha2.bazel
diff --git a/swift/third_party/BUILD.swift-llvm-support.bazel b/swift/third_party/BUILD.swift-llvm-support.bazel
diff --git a/swift/third_party/binlog/BUILD.bazel b/swift/third_party/binlog/BUILD.bazel
diff --git a/swift/third_party/load.bzl b/swift/third_party/load.bzl
diff --git a/swift/third_party/picosha2/BUILD.bazel b/swift/third_party/picosha2/BUILD.bazel

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-#include "swift/extractor/infra/log/SwiftLogging.h"`
	`1`	`+#include "swift/logging/SwiftLogging.h"`
`2`	`2`
`3`	`3`	`namespace codeql {`
`4`	`4`	`namespace fs_logger {`