CXX-2342 add serviceid to events (#823)

Jesse Williamson · web-flow · commit fd84da871c35 · 2021-09-17T11:44:54.000-07:00
* Set up bsoncxx spuport for catch, etc..

Signed-off-by: Jesse Williamson &lt;jesse.williamson@mongodb.com&gt;

* Update event commands.

Signed-off-by: Jesse Williamson &lt;jesse.williamson@mongodb.com&gt;

* Import rewritten mongoc_symbols

Signed-off-by: Jesse Williamson &lt;jesse.williamson@mongodb.com&gt;

* Add unit test for serviceId availability.

Signed-off-by: Jesse Williamson &lt;jesse.williamson@mongodb.com&gt;

* Various cleanup; fix nullopt_t usage; remove is_load_balanced()

Signed-off-by: Jesse Williamson &lt;jesse.williamson@mongodb.com&gt;

* Tests for mocked load-balanced or non-load-balanced service_id.

Signed-off-by: Jesse Williamson &lt;jesse.williamson@mongodb.com&gt;

* Apply clang-format.

Signed-off-by: Jesse Williamson &lt;jesse.williamson@mongodb.com&gt;

* Cleanups in response to review comments.

Signed-off-by: Jesse Williamson &lt;jesse.williamson@mongodb.com&gt;

* Small fixups in response to review.

Signed-off-by: Jesse Williamson &lt;jesse.williamson@mongodb.com&gt;

* Update test, move function object to lambda.

Signed-off-by: Jesse Williamson &lt;jesse.williamson@mongodb.com&gt;
diff --git a/src/bsoncxx/private/helpers.hh b/src/bsoncxx/private/helpers.hh
@@ -1,4 +1,4 @@
-// Copyright 2015 MongoDB Inc.
+// Copyright 2015-present MongoDB Inc.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
 
 #include <bsoncxx/document/value.hpp>
 #include <bsoncxx/document/view.hpp>
+#include <bsoncxx/oid.hpp>
 #include <bsoncxx/private/libbson.hh>
 
 #include <bsoncxx/config/private/prelude.hh>
@@ -33,6 +34,20 @@ inline document::value value_from_bson_t(const bson_t* bson) {
     return document::value{view_from_bson_t(bson)};
 }
 
+/*
+Construct an oid from a bson_oid_t (which is a C API type that we don't want to
+expose to the world).
+
+Note: passing a nullptr is unguarded
+Note: Deduction guides aren't yet available to us, so a factory it is! This is
+something that can be improved as part of CXX-2350 (migration to more recent C++
+standards).
+*/
+inline bsoncxx::oid make_oid(const bson_oid_t* bson_oid) {
+    return bsoncxx::oid(reinterpret_cast<const char*>(bson_oid),
+                        bsoncxx::oid::size());
+}
+
 }  // namespace helpers
 BSONCXX_INLINE_NAMESPACE_END
 }  // namespace bsoncxx
diff --git a/src/bsoncxx/stdx/optional.hpp b/src/bsoncxx/stdx/optional.hpp
@@ -40,6 +40,7 @@ BSONCXX_INLINE_NAMESPACE_END
 
 #include <boost/none.hpp>
 #include <boost/optional/optional.hpp>
+#include <boost/optional/optional_io.hpp>
 
 namespace bsoncxx {
 BSONCXX_INLINE_NAMESPACE_BEGIN
diff --git a/src/bsoncxx/test_util/catch.hh b/src/bsoncxx/test_util/catch.hh
@@ -17,6 +17,7 @@
 #include "catch.hpp"
 #include <bsoncxx/document/value.hpp>
 #include <bsoncxx/json.hpp>
+#include <bsoncxx/oid.hpp>
 #include <bsoncxx/stdx/optional.hpp>
 
 #include <bsoncxx/config/private/prelude.hh>
@@ -25,6 +26,14 @@ namespace Catch {
 using namespace bsoncxx;
 
 // Catch2 must be able to stringify documents, optionals, etc. if they're used in Catch2 macros.
+
+template <>
+struct StringMaker<bsoncxx::oid> {
+    static std::string convert(const bsoncxx::oid& value) {
+        return value.to_string();
+    }
+};
+
 template <>
 struct StringMaker<bsoncxx::document::view> {
     static std::string convert(const bsoncxx::document::view& value) {
diff --git a/src/mongocxx/events/command_failed_event.cpp b/src/mongocxx/events/command_failed_event.cpp
@@ -12,7 +12,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <bsoncxx/oid.hpp>
+#include <bsoncxx/private/helpers.hh>
+#include <bsoncxx/private/libbson.hh>
 #include <bsoncxx/stdx/make_unique.hpp>
+#include <bsoncxx/stdx/optional.hpp>
 #include <mongocxx/events/command_failed_event.hpp>
 #include <mongocxx/private/libmongoc.hh>
 
@@ -52,6 +56,16 @@ std::int64_t command_failed_event::operation_id() const {
         static_cast<const mongoc_apm_command_failed_t*>(_failed_event));
 }
 
+bsoncxx::stdx::optional<bsoncxx::oid> command_failed_event::service_id() const {
+    const bson_oid_t* bson_oid = libmongoc::apm_command_failed_get_service_id(
+        static_cast<const mongoc_apm_command_failed_t*>(_failed_event));
+
+    if (nullptr == bson_oid)
+        return {bsoncxx::stdx::nullopt};
+
+    return {bsoncxx::helpers::make_oid(bson_oid)};
+}
+
 bsoncxx::stdx::string_view command_failed_event::host() const {
     return libmongoc::apm_command_failed_get_host(
                static_cast<const mongoc_apm_command_failed_t*>(_failed_event))
diff --git a/src/mongocxx/events/command_failed_event.hpp b/src/mongocxx/events/command_failed_event.hpp
@@ -17,6 +17,8 @@
 #include <memory>
 
 #include <bsoncxx/document/view.hpp>
+#include <bsoncxx/oid.hpp>
+#include <bsoncxx/stdx/optional.hpp>
 
 #include <mongocxx/config/prelude.hpp>
 
@@ -75,6 +77,13 @@ class MONGOCXX_API command_failed_event {
     ///
     std::int64_t operation_id() const;
 
+    ///
+    /// Optionally returns the service id.
+    ///
+    /// @return No contained value, or contains the service id if load balancing is enabled.
+    ///
+    bsoncxx::stdx::optional<bsoncxx::oid> service_id() const;
+
     ///
     /// Returns the host name.
     ///
diff --git a/src/mongocxx/events/command_started_event.cpp b/src/mongocxx/events/command_started_event.cpp
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <bsoncxx/private/helpers.hh>
 #include <bsoncxx/stdx/make_unique.hpp>
 #include <mongocxx/events/command_started_event.hpp>
 #include <mongocxx/private/libmongoc.hh>
@@ -52,6 +53,16 @@ std::int64_t command_started_event::operation_id() const {
         static_cast<const mongoc_apm_command_started_t*>(_started_event));
 }
 
+bsoncxx::stdx::optional<bsoncxx::oid> command_started_event::service_id() const {
+    const bson_oid_t* bson_oid = libmongoc::apm_command_started_get_service_id(
+        static_cast<const mongoc_apm_command_started_t*>(_started_event));
+
+    if (nullptr == bson_oid)
+        return {bsoncxx::stdx::nullopt};
+
+    return {bsoncxx::helpers::make_oid(bson_oid)};
+}
+
 bsoncxx::stdx::string_view command_started_event::host() const {
     return libmongoc::apm_command_started_get_host(
                static_cast<const mongoc_apm_command_started_t*>(_started_event))
diff --git a/src/mongocxx/events/command_started_event.hpp b/src/mongocxx/events/command_started_event.hpp
@@ -17,6 +17,8 @@
 #include <memory>
 
 #include <bsoncxx/document/view.hpp>
+#include <bsoncxx/oid.hpp>
+#include <bsoncxx/stdx/optional.hpp>
 
 #include <mongocxx/config/prelude.hpp>
 
@@ -75,6 +77,13 @@ class MONGOCXX_API command_started_event {
     ///
     std::int64_t operation_id() const;
 
+    ///
+    /// Optionally returns the service id.
+    ///
+    /// @return No contained value, or contains the service id if load balancing is enabled.
+    ///
+    bsoncxx::stdx::optional<bsoncxx::oid> service_id() const;
+
     ///
     /// Returns the host name.
     ///
diff --git a/src/mongocxx/events/command_succeeded_event.cpp b/src/mongocxx/events/command_succeeded_event.cpp
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <bsoncxx/private/helpers.hh>
 #include <bsoncxx/stdx/make_unique.hpp>
 #include <mongocxx/events/command_succeeded_event.hpp>
 #include <mongocxx/private/libmongoc.hh>
@@ -52,6 +53,16 @@ std::int64_t command_succeeded_event::operation_id() const {
         static_cast<const mongoc_apm_command_succeeded_t*>(_succeeded_event));
 }
 
+bsoncxx::stdx::optional<bsoncxx::oid> command_succeeded_event::service_id() const {
+    const bson_oid_t* bson_oid = libmongoc::apm_command_succeeded_get_service_id(
+        static_cast<const mongoc_apm_command_succeeded_t*>(_succeeded_event));
+
+    if (nullptr == bson_oid)
+        return {bsoncxx::stdx::nullopt};
+
+    return {bsoncxx::helpers::make_oid(bson_oid)};
+}
+
 bsoncxx::stdx::string_view command_succeeded_event::host() const {
     return libmongoc::apm_command_succeeded_get_host(
                static_cast<const mongoc_apm_command_succeeded_t*>(_succeeded_event))
diff --git a/src/mongocxx/events/command_succeeded_event.hpp b/src/mongocxx/events/command_succeeded_event.hpp
@@ -17,6 +17,8 @@
 #include <memory>
 
 #include <bsoncxx/document/view.hpp>
+#include <bsoncxx/oid.hpp>
+#include <bsoncxx/stdx/optional.hpp>
 
 #include <mongocxx/config/prelude.hpp>
 
@@ -75,6 +77,13 @@ class MONGOCXX_API command_succeeded_event {
     ///
     std::int64_t operation_id() const;
 
+    ///
+    /// Optionally returns the service id.
+    ///
+    /// @return No contained value, or contains the service id if load balancing is enabled.
+    ///
+    bsoncxx::stdx::optional<bsoncxx::oid> service_id() const;
+
     ///
     /// Returns the host name.
     ///
diff --git a/src/mongocxx/private/libmongoc_symbols.hh b/src/mongocxx/private/libmongoc_symbols.hh
@@ -25,6 +25,7 @@ MONGOCXX_LIBMONGOC_SYMBOL(apm_command_failed_get_operation_id)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_command_failed_get_reply)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_command_failed_get_request_id)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_command_failed_get_server_id)
+MONGOCXX_LIBMONGOC_SYMBOL(apm_command_failed_get_service_id)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_command_started_get_command)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_command_started_get_command_name)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_command_started_get_context)
@@ -33,6 +34,7 @@ MONGOCXX_LIBMONGOC_SYMBOL(apm_command_started_get_host)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_command_started_get_operation_id)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_command_started_get_request_id)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_command_started_get_server_id)
+MONGOCXX_LIBMONGOC_SYMBOL(apm_command_started_get_service_id)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_command_succeeded_get_command_name)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_command_succeeded_get_context)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_command_succeeded_get_duration)
@@ -41,6 +43,7 @@ MONGOCXX_LIBMONGOC_SYMBOL(apm_command_succeeded_get_operation_id)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_command_succeeded_get_reply)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_command_succeeded_get_request_id)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_command_succeeded_get_server_id)
+MONGOCXX_LIBMONGOC_SYMBOL(apm_command_succeeded_get_service_id)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_server_changed_get_context)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_server_changed_get_host)
 MONGOCXX_LIBMONGOC_SYMBOL(apm_server_changed_get_new_description)
diff --git a/src/mongocxx/test/database.cpp b/src/mongocxx/test/database.cpp
@@ -1,4 +1,4 @@
-// Copyright 2014 MongoDB Inc.
+// Copyright 2014-present MongoDB Inc.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -492,4 +492,93 @@ TEST_CASE("Database integration tests", "[database]") {
     }
 }
 
+// As C++11 lacks generic lambdas, and "ordinary" templates can't appear at block scope,
+// we'll have to define our helper for the serviceId tests here. This implementation would
+// be more straightforward in newer versions of C++ (C++14 and on allow generic lambdas),
+// see CXX-2350 (migration to more recent C++ standards); our C++17 optional<> implementation 
+// also has a few inconsistencies with the standard, which appear to vary across platforms.
+template <typename EventT>
+struct check_service_id {
+    const bool expect_service_id;
+
+    check_service_id(const bool expect_service_id) : expect_service_id(expect_service_id) {}
+
+    void operator()(const EventT& event) {
+        INFO("checking for service_id()")
+        CAPTURE(event.command_name(), expect_service_id);
+
+        auto service_id = event.service_id();
+
+        if (expect_service_id)
+            CHECK(service_id);
+        else
+            CHECK_FALSE(service_id);
+    }
+};
+
+TEST_CASE("serviceId presence depends on load-balancing mode") {
+    // Repeat this test case for a situation in which service_id should and should not be returned:
+    auto expect_service_id = GENERATE(true, false);
+
+    instance::current();
+
+    auto client_opts = test_util::add_test_server_api();
+
+    // Set Application Performance Monitoring options (APM):
+    mongocxx::options::apm apm_opts;
+    apm_opts.on_command_started(
+        check_service_id<mongocxx::events::command_started_event>{expect_service_id});
+    apm_opts.on_command_succeeded(
+        check_service_id<mongocxx::events::command_succeeded_event>{expect_service_id});
+    apm_opts.on_command_failed(
+        check_service_id<mongocxx::events::command_failed_event>{expect_service_id});
+
+    // Set up mocking for get_service_id events:
+    auto apm_command_started_get_service_id =
+        libmongoc::apm_command_started_get_service_id.create_instance();
+    auto apm_command_succeeded_get_service_id =
+        libmongoc::apm_command_succeeded_get_service_id.create_instance();
+    auto apm_command_failed_get_service_id =
+        libmongoc::apm_command_failed_get_service_id.create_instance();
+
+    // Set up mocked functions that DO emit a service_id:
+    if (expect_service_id) {
+        // Return a bson_oid_t with data where the service_id has some value:
+	const auto make_service_id_bson_oid_t = [](const void *) -> const bson_oid_t* {
+		static bson_oid_t tmp = { 0x65 };
+		return &tmp;
+	};
+
+        // Add forever() so that the mock function also extends to endSession:
+        apm_command_started_get_service_id->interpose(make_service_id_bson_oid_t).forever();
+        apm_command_succeeded_get_service_id->interpose(make_service_id_bson_oid_t).forever();
+        apm_command_failed_get_service_id->interpose(make_service_id_bson_oid_t).forever();
+    }
+
+    // Set up mocked functions that DO NOT emit a service_id:
+    if (!expect_service_id) {
+        auto make_empty_bson_oid_t = [](const void*) -> bson_oid_t* { return nullptr; };
+
+        apm_command_started_get_service_id->interpose(make_empty_bson_oid_t).forever();
+        apm_command_succeeded_get_service_id->interpose(make_empty_bson_oid_t).forever();
+        apm_command_failed_get_service_id->interpose(make_empty_bson_oid_t).forever();
+    }
+
+    // Set up our connection:
+    client_opts.apm_opts(apm_opts);
+    // Adding ?loadBalanced=true results in an error in the C driver because
+    // the initial hello response from the server does not include serviceId.
+    client mongo_client(uri("mongodb://localhost:27017/"), client_opts);
+    stdx::string_view database_name{"database"};
+    database database = mongo_client[database_name];
+
+    // Run a command, triggering start and completion events:
+    auto cmd = make_document(kvp("ping", 1));
+    database.run_command(cmd.view());
+
+    // Attempt to trigger failure:
+    cmd = make_document(kvp("some_sort_of_invalid_command_that_should_never_happen", 1));
+    CHECK_THROWS(database.run_command(cmd.view()));
+}
+
 }  // namespace
