[Audit Logging] Logger and factory APIs in C-Core and C++.

src/core/lib/security/authorization/audit_logging.cc

@@ -25,6 +25,7 @@
 #include <memory>
 #include <utility>
 
+#include "absl/base/thread_annotations.h"
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/str_format.h"
@@ -39,62 +40,65 @@
 namespace grpc_core {
 namespace experimental {
 
-void AuditLoggerRegistry::RegisterAuditLoggerFactory(
+namespace {
+
+Mutex* g_mu = new Mutex();
+AuditLoggerRegistry* g_registry ABSL_GUARDED_BY(g_mu) =
+    new AuditLoggerRegistry();
+
+}  // namespace
+
+void AuditLoggerRegistry::RegisterFactory(
     std::unique_ptr<AuditLoggerFactory> factory) {
-  auto& registry = GetAuditLoggerRegistry();
-  MutexLock lock(&registry.mu_);
-  registry.logger_factories_map_[factory->name()] = std::move(factory);
+  if (factory == nullptr) return;
+  MutexLock lock(g_mu);
+  GPR_ASSERT(g_registry->logger_factories_map_.find(factory->name()) ==
+             g_registry->logger_factories_map_.end());
+  g_registry->logger_factories_map_[factory->name()] = std::move(factory);
 }
 
-bool AuditLoggerRegistry::AuditLoggerFactoryExists(absl::string_view name) {
-  auto& registry = AuditLoggerRegistry::GetAuditLoggerRegistry();
-  MutexLock lock(&registry.mu_);
-  return registry.logger_factories_map_.find(name) !=
-         registry.logger_factories_map_.end();
+bool AuditLoggerRegistry::FactoryExists(absl::string_view name) {
+  MutexLock lock(g_mu);
+  return g_registry->logger_factories_map_.find(name) !=
+         g_registry->logger_factories_map_.end();
 }
 
 absl::StatusOr<std::unique_ptr<AuditLoggerFactory::Config>>
-AuditLoggerRegistry::ParseAuditLoggerConfig(absl::string_view name,
-                                            const Json& json) {
-  auto factory_or = AuditLoggerRegistry::GetAuditLoggerFactory(name);
+AuditLoggerRegistry::ParseConfig(absl::string_view name, const Json& json) {
+  MutexLock lock(g_mu);
+  auto factory_or = g_registry->GetAuditLoggerFactory(name);
   if (!factory_or.ok()) {
-    return absl::InvalidArgumentError("unsupported audit logger type");
+    return factory_or.status();
   }
   return factory_or.value()->ParseAuditLoggerConfig(json);
 }
 
 std::unique_ptr<AuditLogger> AuditLoggerRegistry::CreateAuditLogger(
     std::unique_ptr<AuditLoggerFactory::Config> config) {
-  auto factory_or = AuditLoggerRegistry::GetAuditLoggerFactory(config->name());
+  MutexLock lock(g_mu);
+  auto factory_or = g_registry->GetAuditLoggerFactory(config->name());
   GPR_ASSERT(factory_or.ok());
   return factory_or.value()->CreateAuditLogger(std::move(config));
 }
 
-AuditLoggerRegistry& AuditLoggerRegistry::GetAuditLoggerRegistry() {
-  static AuditLoggerRegistry& registry = *new AuditLoggerRegistry();
-  return registry;
-}
-
 absl::StatusOr<AuditLoggerFactory*> AuditLoggerRegistry::GetAuditLoggerFactory(
     absl::string_view name) {
-  auto& registry = GetAuditLoggerRegistry();
-  MutexLock lock(&registry.mu_);
-  auto it = registry.logger_factories_map_.find(name);
-  if (it != registry.logger_factories_map_.end()) {
-    return it->second.get();
+  auto it = logger_factories_map_.find(name);
+  if (it == logger_factories_map_.end()) {
+    return absl::NotFoundError(
+        absl::StrFormat("audit logger factory for %s does not exist", name));
   }
-  return absl::NotFoundError(
-      absl::StrFormat("audit logger factory %s does not exist", name));
+  return it->second.get();
 }
 
 void AuditLoggerRegistry::TestOnlyResetRegistry() {
-  auto& registry = GetAuditLoggerRegistry();
-  MutexLock lock(&registry.mu_);
-  registry.logger_factories_map_.clear();
+  MutexLock lock(g_mu);
+  delete g_registry;
+  g_registry = new AuditLoggerRegistry();
 }
 
 void RegisterAuditLoggerFactory(std::unique_ptr<AuditLoggerFactory> factory) {
-  AuditLoggerRegistry::RegisterAuditLoggerFactory(std::move(factory));
+  AuditLoggerRegistry::RegisterFactory(std::move(factory));
 }
 
 }  // namespace experimental

src/core/lib/security/authorization/audit_logging.h

@@ -24,26 +24,26 @@
 #include <map>
 #include <memory>
 
-#include "absl/base/thread_annotations.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
 
 #include <grpc/grpc_audit_logging.h>
 
-#include "src/core/lib/gprpp/sync.h"
 #include "src/core/lib/json/json.h"
 
 namespace grpc_core {
 namespace experimental {
 
 class AuditLoggerRegistry {
  public:
-  static void RegisterAuditLoggerFactory(std::unique_ptr<AuditLoggerFactory>);
+  AuditLoggerRegistry() = default;
+
+  static void RegisterFactory(std::unique_ptr<AuditLoggerFactory>);
 
-  static bool AuditLoggerFactoryExists(absl::string_view name);
+  static bool FactoryExists(absl::string_view name);
 
   static absl::StatusOr<std::unique_ptr<AuditLoggerFactory::Config>>
-  ParseAuditLoggerConfig(absl::string_view name, const Json& json);
+  ParseConfig(absl::string_view name, const Json& json);
 
   // This assume the given config is parsed and validated already.
   // Therefore, it should always succeed in creating a logger.
@@ -56,18 +56,12 @@ class AuditLoggerRegistry {
   static void TestOnlyResetRegistry();
 
  private:
-  AuditLoggerRegistry() = default;
-
-  static AuditLoggerRegistry& GetAuditLoggerRegistry();
-
-  static absl::StatusOr<AuditLoggerFactory*> GetAuditLoggerFactory(
+  absl::StatusOr<AuditLoggerFactory*> GetAuditLoggerFactory(
       absl::string_view name);
 
-  Mutex mu_;
-
   // The key is owned by the factory.
   std::map<absl::string_view, std::unique_ptr<AuditLoggerFactory>>
-      logger_factories_map_ ABSL_GUARDED_BY(mu_);
+      logger_factories_map_;
 };
 
 }  // namespace experimental

test/core/security/grpc_audit_logging_test.cc

@@ -82,23 +82,23 @@ class AuditLoggingTest : public ::testing::Test {
 }  // namespace
 
 TEST_F(AuditLoggingTest, SuccessfulLoggerCreation) {
-  auto result = AuditLoggerRegistry::ParseAuditLoggerConfig(kName, Json());
+  auto result = AuditLoggerRegistry::ParseConfig(kName, Json());
   ASSERT_TRUE(result.ok());
   ASSERT_NE(AuditLoggerRegistry::CreateAuditLogger(std::move(result.value())),
             nullptr);
 }
 
 TEST_F(AuditLoggingTest, UnknownLogger) {
-  auto result =
-      AuditLoggerRegistry::ParseAuditLoggerConfig("unknown_logger", Json());
-  EXPECT_EQ(result.status().code(), absl::StatusCode::kInvalidArgument);
-  EXPECT_EQ(result.status().message(), "unsupported audit logger type")
+  auto result = AuditLoggerRegistry::ParseConfig("unknown_logger", Json());
+  EXPECT_EQ(result.status().code(), absl::StatusCode::kNotFound);
+  EXPECT_EQ(result.status().message(),
+            "audit logger factory for unknown_logger does not exist")
       << result.status();
 }
 
 TEST_F(AuditLoggingTest, AuditLoggerFactoryExistenceChecks) {
-  EXPECT_TRUE(AuditLoggerRegistry::AuditLoggerFactoryExists(kName));
-  EXPECT_FALSE(AuditLoggerRegistry::AuditLoggerFactoryExists("unknown_logger"));
+  EXPECT_TRUE(AuditLoggerRegistry::FactoryExists(kName));
+  EXPECT_FALSE(AuditLoggerRegistry::FactoryExists("unknown_logger"));
 }
 
 }  // namespace testing