Merge pull request #19457 from vespa-engine/havardpe/capture-issues

capture issues by binding handlers to threads

6 files changed, 210 insertions, 0 deletions

diff --git a/vespalib/CMakeLists.txt b/vespalib/CMakeLists.txt
index 1b226c84753..472696131d2 100644
--- a/vespalib/CMakeLists.txt
+++ b/vespalib/CMakeLists.txt
@@ -64,6 +64,7 @@ vespa_define_module(
     src/tests/hwaccelrated
     src/tests/io/fileutil
     src/tests/io/mapped_file_input
+    src/tests/issue
     src/tests/latch
     src/tests/left_right_heap
     src/tests/make_fixture_macros
diff --git a/vespalib/src/tests/issue/CMakeLists.txt b/vespalib/src/tests/issue/CMakeLists.txt
new file mode 100644
index 00000000000..7c8147979d7
--- /dev/null
+++ b/vespalib/src/tests/issue/CMakeLists.txt
@@ -0,0 +1,9 @@
+# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+vespa_add_executable(vespalib_issue_test_app TEST
+    SOURCES
+    issue_test.cpp
+    DEPENDS
+    vespalib
+    GTest::GTest
+)
+vespa_add_test(NAME vespalib_issue_test_app COMMAND vespalib_issue_test_app)
diff --git a/vespalib/src/tests/issue/issue_test.cpp b/vespalib/src/tests/issue/issue_test.cpp
new file mode 100644
index 00000000000..01b151da482
--- /dev/null
+++ b/vespalib/src/tests/issue/issue_test.cpp
@@ -0,0 +1,68 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include <vespa/vespalib/util/issue.h>
+#include <vespa/vespalib/gtest/gtest.h>
+
+using namespace vespalib;
+
+struct MyHandler : Issue::Handler {
+    std::vector<vespalib::string> list;
+    void handle(const Issue &issue) override {
+        list.push_back(issue.message());
+    }
+};
+
+std::vector<vespalib::string> make_list(std::vector<vespalib::string> list) {
+    return list;
+}
+
+TEST(IssueTest, log_issue_not_captured) {
+    Issue::report(Issue("this should be logged"));
+}
+
+TEST(IssueTest, capture_an_issue) {
+    MyHandler my_handler;
+    {
+        Issue::report(Issue("this should be logged"));
+        Issue::Binding my_binding = Issue::listen(my_handler);
+        Issue::report(Issue("this should be captured"));
+    }
+    Issue::report(Issue("this should also be logged"));
+    EXPECT_EQ(my_handler.list, make_list({"this should be captured"}));
+}
+
+TEST(IssueTest, capture_issues_with_nested_bindings) {
+    MyHandler my_handler1;
+    MyHandler my_handler2;
+    {
+        Issue::report(Issue("this should be logged"));
+        auto my_binding1 = Issue::listen(my_handler1);
+        Issue::report(Issue("issue1"));
+        {
+            auto my_binding2 = Issue::listen(my_handler2);
+            Issue::report(Issue("issue2"));
+        }
+        Issue::report(Issue("issue3"));
+    }
+    Issue::report(Issue("this should also be logged"));
+    EXPECT_EQ(my_handler1.list, make_list({"issue1", "issue3"}));
+    EXPECT_EQ(my_handler2.list, make_list({"issue2"}));
+}
+
+TEST(IssueTest, handler_can_be_bound_multiple_times) {
+    MyHandler my_handler;
+    {
+        Issue::report(Issue("this should be logged"));
+        auto my_binding1 = Issue::listen(my_handler);
+        Issue::report(Issue("issue1"));
+        {
+            auto my_binding2 = Issue::listen(my_handler);
+            Issue::report(Issue("issue2"));
+        }
+        Issue::report(Issue("issue3"));
+    }
+    Issue::report(Issue("this should also be logged"));
+    EXPECT_EQ(my_handler.list, make_list({"issue1", "issue2", "issue3"}));
+}
+
+GTEST_MAIN_RUN_ALL_TESTS()
diff --git a/vespalib/src/vespa/vespalib/util/CMakeLists.txt b/vespalib/src/vespa/vespalib/util/CMakeLists.txt
index 8db65b58b04..9e2917775f0 100644
--- a/vespalib/src/vespa/vespalib/util/CMakeLists.txt
+++ b/vespalib/src/vespa/vespalib/util/CMakeLists.txt
@@ -29,6 +29,7 @@ vespa_add_library(vespalib_vespalib_util OBJECT
     generationholder.cpp
     hdr_abort.cpp
     host_name.cpp
+    issue.cpp
     joinable.cpp
     latch.cpp
     left_right_heap.cpp
diff --git a/vespalib/src/vespa/vespalib/util/issue.cpp b/vespalib/src/vespa/vespalib/util/issue.cpp
new file mode 100644
index 00000000000..9526d8d6bcd
--- /dev/null
+++ b/vespalib/src/vespa/vespalib/util/issue.cpp
@@ -0,0 +1,65 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include "issue.h"
+
+#include <vespa/log/log.h>
+LOG_SETUP(".vespalib.issue");
+
+namespace vespalib {
+
+namespace {
+
+using Link = Issue::Binding::Link;
+
+struct LogIssues : Issue::Handler {
+    void handle(const Issue &issue) override {
+        LOG(warning, "issue not captured: %s", issue.message().c_str());
+    }
+};
+
+Link *get_root() {
+    static LogIssues log_issues;
+    static Link root{log_issues, nullptr};
+    return &root;
+}
+
+Link **get_head() {
+    thread_local Link *head = get_root();
+    return &head;
+}
+
+} // <unnamed>
+
+Issue::Issue(const vespalib::string &message)
+  : _message(message)
+{
+}
+
+Issue::Binding::Binding(Handler &handler)
+  : _link{handler, nullptr}
+{
+    Link **head = get_head();
+    _link.next = *head;
+    *head = &_link;
+}
+
+Issue::Binding::~Binding()
+{
+    Link **head = get_head();
+    LOG_ASSERT(*head == &_link);
+    *head = (*head)->next;
+}
+
+void
+Issue::report(const Issue &issue)
+{
+    (*get_head())->handler.handle(issue);
+}
+
+Issue::Binding
+Issue::listen(Handler &handler)
+{
+    return Binding(handler);
+}
+
+}
diff --git a/vespalib/src/vespa/vespalib/util/issue.h b/vespalib/src/vespa/vespalib/util/issue.h
new file mode 100644
index 00000000000..7cb4ebaff59
--- /dev/null
+++ b/vespalib/src/vespa/vespalib/util/issue.h
@@ -0,0 +1,66 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#pragma once
+
+#include <vespa/vespalib/stllike/string.h>
+
+namespace vespalib {
+
+/**
+ * An Issue is an error encountered during program execution that will
+ * not affect the program flow. It is like an exception that is not
+ * (re-)thrown, but rather logged as a warning, or like an error that
+ * augments the computation result instead of replacing it. Issues are
+ * reported by the code identifying that something is wrong; similar
+ * to logging a warning. Issues are handled by an Issue::Handler that
+ * is bound to the current thread; similar to try/catch used by
+ * exceptions, but rather than disrupting the program flow we only
+ * capture any issues encountered along the way. Thread-local data
+ * structures are used to match the reporting of an issue with its
+ * appropriate handler, making it possible to handle issues across
+ * arbitrary synchronous API boundaries without changing the APIs to
+ * explicitly wire all issues through them.
+ *
+ * An Issue object represents a single issue. The static 'report'
+ * function is used to report an issue. The static 'listen' function
+ * is used to bind an Issue::Handler to the current thread using a
+ * special object that should reside on the stack and unbinds the
+ * handler when destructed. Handler bindings can be nested. Issue
+ * reports will be routed to the last handler bound to the thread.
+ *
+ * Note that the objects binding handlers to threads must not be
+ * destructed out of order; just let them go out of scope.
+ **/
+class Issue
+{
+private:
+    vespalib::string _message;
+public:
+    Issue(const vespalib::string &message);
+    const vespalib::string &message() const { return _message; }
+    struct Handler {
+        virtual void handle(const Issue &issue) = 0;
+        virtual ~Handler() = default;
+    };
+    class Binding
+    {
+    public:
+        struct Link {
+            Handler &handler;
+            Link *next;
+        };
+    private:
+        Link _link;
+    public:
+        Binding(Handler &handler);
+        Binding(Binding &&) = delete;
+        Binding(const Binding &) = delete;
+        Binding &operator=(Binding &&) = delete;
+        Binding &operator=(const Binding &) = delete;
+        ~Binding();
+    };
+    static void report(const Issue &issue);
+    static Binding listen(Handler &handler);
+};
+
+}