Supporting Logging API .Log() Overloads and GetName() method (open-te…

…lemetry#422)

api/include/opentelemetry/logs/noop.h

@@ -50,6 +50,8 @@ namespace logs
 class NoopLogger final : public Logger
 {
 public:
+  const nostd::string_view GetName() noexcept override { return "noop logger"; }
+
   void Log(Severity severity,
            nostd::string_view name,
            nostd::string_view body,

api/test/logs/BUILD

@@ -1,9 +1,9 @@
 load("//bazel:otel_cc_benchmark.bzl", "otel_cc_benchmark")
 
 cc_test(
-    name = "logger_provider_test",
+    name = "provider_test",
     srcs = [
-        "logger_provider_test.cc",
+        "provider_test.cc",
     ],
     deps = [
         "//api",

api/test/logs/CMakeLists.txt

@@ -1,9 +1,9 @@
-foreach(testname logger_provider_test logger_test)
-  add_executable(${testname} "${testname}.cc")
-  target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES}
+foreach(testname provider_test logger_test)
+  add_executable(logs_api_${testname} "${testname}.cc")
+  target_link_libraries(logs_api_${testname} ${GTEST_BOTH_LIBRARIES}
                         ${CMAKE_THREAD_LIBS_INIT} opentelemetry_api)
   gtest_add_tests(
-    TARGET ${testname}
+    TARGET logs_api_${testname}
     TEST_PREFIX logs.
-    TEST_LIST ${testname})
+    TEST_LIST logs_api_${testname})
 endforeach()

api/test/logs/logger_test.cc

@@ -14,40 +14,61 @@ using opentelemetry::nostd::shared_ptr;
 using opentelemetry::nostd::span;
 using opentelemetry::nostd::string_view;
 
+// Check that the default logger is a noop logger instance
 TEST(Logger, GetLoggerDefault)
 {
   auto lp     = Provider::GetLoggerProvider();
   auto logger = lp->GetLogger("TestLogger");
+  auto name   = logger->GetName();
   EXPECT_NE(nullptr, logger);
+  EXPECT_EQ(name, "noop logger");
 }
 
-TEST(Logger, GetNoopLoggerName)
-{
-  auto lp     = Provider::GetLoggerProvider();
-  auto logger = lp->GetLogger("TestLogger");
-}
-
+// Test the two additional overloads for GetLogger()
 TEST(Logger, GetNoopLoggerNameWithArgs)
 {
   auto lp = Provider::GetLoggerProvider();
 
+  // GetLogger(name, list(args))
   std::array<string_view, 1> sv{"string"};
   span<string_view> args{sv};
-  auto logger = lp->GetLogger("NoopLoggerWithArgs", args);
-  // should probably also test that arguments were set properly too
-  // by adding a getArgs() method in NoopLogger
+  lp->GetLogger("NoopLoggerWithArgs", args);
+
+  // GetLogger(name, string options)
+  lp->GetLogger("NoopLoggerWithOptions", "options");
 }
 
-TEST(Logger, NoopLog)
+// Test the Log() overloads
+TEST(Logger, LogMethodOverloads)
 {
   auto lp     = Provider::GetLoggerProvider();
   auto logger = lp->GetLogger("TestLogger");
-  logger->Log("Noop log name");
+
+  // Create a map to test the logs with
+  std::map<std::string, std::string> m = {{"key1", "value1"}};
+
+  // Log overloads
+  logger->Log(Severity::kTrace, "Test log message");
+  logger->Log(Severity::kInfo, "Logging a message", "Test log message");
+  logger->Log(Severity::kDebug, m);
+  logger->Log(Severity::kWarn, "Logging a map", m);
+  logger->Log(Severity::kError, {{"key1", "value 1"}, {"key2", 2}});
+  logger->Log(Severity::kFatal, "Logging an initializer list", {{"key1", "value 1"}, {"key2", 2}});
+
+  // Severity methods
+  logger->Trace("Test log message");
+  logger->Debug("Logging a message", "Test log message");
+  logger->Info(m);
+  logger->Warn("Logging a map", m);
+  logger->Error({{"key1", "value 1"}, {"key2", 2}});
+  logger->Fatal("Logging an initializer list", {{"key1", "value 1"}, {"key2", 2}});
 }
 
 // Define a basic Logger class
 class TestLogger : public Logger
 {
+  const opentelemetry::nostd::string_view GetName() noexcept override { return "test logger"; }
+
   void Log(Severity severity,
            string_view name,
            string_view body,
@@ -82,11 +103,7 @@ TEST(Logger, PushLoggerImplementation)
 
   auto lp = Provider::GetLoggerProvider();
 
-  // GetLogger(name, options) function
+  // Check that the implementation was pushed by calling TestLogger's GetName()
   auto logger = lp->GetLogger("TestLogger");
-
-  // GetLogger(name, args) function
-  std::array<string_view, 1> sv{"string"};
-  span<string_view> args{sv};
-  auto logger2 = lp->GetLogger("TestLogger2", args);
-}
+  ASSERT_EQ("test logger", logger->GetName());
+}

exporters/ostream/test/ostream_log_test.cc

@@ -253,7 +253,7 @@ TEST(OStreamLogExporter, IntegrationTest)
 
   // Write a log to ostream exporter
   opentelemetry::core::SystemTimestamp now(std::chrono::system_clock::now());
-  logger->Log(opentelemetry::logs::Severity::kDebug, "Hello", now);
+  logger->Log(opentelemetry::logs::Severity::kDebug, "", "Hello", {}, {}, {}, {}, {}, now);
 
   // Restore cout's original streambuf
   std::cout.rdbuf(original);

sdk/include/opentelemetry/sdk/logs/logger.h

@@ -34,11 +34,17 @@ class Logger final : public opentelemetry::logs::Logger
 public:
   /**
    * Initialize a new logger.
+   * @param name The name of this logger instance
    * @param logger_provider The logger provider that owns this logger.
    */
   explicit Logger(opentelemetry::nostd::string_view name,
                   std::shared_ptr<LoggerProvider> logger_provider) noexcept;
 
+  /**
+   * Returns the name of this logger.
+   */
+  const opentelemetry::nostd::string_view GetName() noexcept override;
+
   /**
    * Writes a log record into the processor.
    * @param severity the severity level of the log event.

sdk/src/logs/logger.cc

@@ -27,6 +27,11 @@ Logger::Logger(opentelemetry::nostd::string_view name,
     : logger_name_(std::string(name)), logger_provider_(logger_provider)
 {}
 
+const opentelemetry::nostd::string_view Logger::GetName() noexcept
+{
+  return logger_name_;
+}
+
 /**
  * Create and populate recordable with the log event's fields passed in.
  * The timestamp, severity, traceid, spanid, and traceflags, are injected

sdk/test/logs/logger_sdk_test.cc

@@ -31,7 +31,7 @@ TEST(LoggerSDK, LogToNullProcessor)
   auto logger = lp->GetLogger("logger");
 
   // Log a sample log record to a nullptr processor
-  logger->Log("Test log");
+  logger->Debug("Test log");
 }
 
 class MockProcessor final : public LogProcessor
@@ -56,11 +56,11 @@ class MockProcessor final : public LogProcessor
     // Cast the recordable received into a concrete LogRecord type
     auto copy = std::shared_ptr<LogRecord>(static_cast<LogRecord *>(record.release()));
 
-    // Copy over the received log record's name, body, timestamp fields over to the recordable
+    // Copy over the received log record's severity, name, and body fields over to the recordable
     // passed in the constructor
     record_received_->SetSeverity(copy->GetSeverity());
+    record_received_->SetName(copy->GetName());
     record_received_->SetBody(copy->GetBody());
-    record_received_->SetTimestamp(copy->GetTimestamp());
   }
   bool ForceFlush(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept
   {
@@ -92,10 +92,9 @@ TEST(LoggerSDK, LogToAProcessor)
   ASSERT_EQ(processor, lp->GetProcessor());
 
   // Check that the recordable created by the Log() statement is set properly
-  opentelemetry::core::SystemTimestamp now(std::chrono::system_clock::now());
-  logger->Log(opentelemetry::logs::Severity::kWarn, "Message", now);
+  logger->Log(opentelemetry::logs::Severity::kWarn, "Log Name", "Log Message");
 
   ASSERT_EQ(shared_recordable->GetSeverity(), opentelemetry::logs::Severity::kWarn);
-  ASSERT_EQ(shared_recordable->GetBody(), "Message");
-  ASSERT_EQ(shared_recordable->GetTimestamp().time_since_epoch(), now.time_since_epoch());
+  ASSERT_EQ(shared_recordable->GetName(), "Log Name");
+  ASSERT_EQ(shared_recordable->GetBody(), "Log Message");
 }