From 77fd8a47e95e0ae9a16ae025a14378c94fa987aa Mon Sep 17 00:00:00 2001 From: Seufert Date: Thu, 3 Dec 2020 12:28:43 -0700 Subject: [PATCH] Added Log overloads, GetName() method, fixed LogRecord attributes and resources --- api/include/opentelemetry/logs/log_record.h | 24 +- api/include/opentelemetry/logs/logger.h | 350 ++++++++++++++++-- api/include/opentelemetry/logs/noop.h | 3 +- api/test/logs/BUILD | 4 +- api/test/logs/CMakeLists.txt | 11 +- api/test/logs/logger_test.cc | 100 +++-- ...gger_provider_test.cc => provider_test.cc} | 0 sdk/include/opentelemetry/sdk/logs/logger.h | 14 +- sdk/src/logs/logger.cc | 12 +- sdk/src/logs/logger_provider.cc | 2 +- sdk/test/logs/logger_sdk_test.cc | 4 +- 11 files changed, 427 insertions(+), 97 deletions(-) rename api/test/logs/{logger_provider_test.cc => provider_test.cc} (100%) diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h index c663c1c9c7..f8d905ba6e 100644 --- a/api/include/opentelemetry/logs/log_record.h +++ b/api/include/opentelemetry/logs/log_record.h @@ -65,11 +65,6 @@ enum class Severity : uint8_t kDefault = kInfo // default severity is set to kInfo level, similar to what is done in ILogger }; -/* _nullKV is defined as a private variable that allows "resource" and - "attributes" fields to be instantiated using it as the default value */ -static common::KeyValueIterableView> _nullKV = - common::KeyValueIterableView>{{}}; - /** * A default Event object to be passed in log statements, * matching the 10 fields of the Log Data Model. @@ -82,25 +77,28 @@ struct LogRecord core::SystemTimestamp timestamp; // uint64 nanoseconds since Unix epoch trace::TraceId trace_id; // byte sequence trace::SpanId span_id; // byte sequence - trace::TraceFlags trace_flag; // byte - Severity severity; // Severity enum that combines severity_text and severity_number in the - // LogDataModel (can separate in SDK) + trace::TraceFlags trace_flags; // byte + Severity severity; // Severity enum that combines severity_text and severity_number // other fields that will not be set by default nostd::string_view name; // string nostd::string_view body; // currently a simple string, but should be changed "Any" type - common::KeyValueIterable &resource; // key/value pair list - common::KeyValueIterable &attributes; // key/value pair list + nostd::shared_ptr resource; // key/value pair list + nostd::shared_ptr attributes; // key/value pair list /* Default log record if user does not overwrite this. * TODO: find better data type to represent the type for "body" * Future enhancement: Potentially add other constructors to take default arguments * from the user **/ - LogRecord() : resource(_nullKV), attributes(_nullKV) + LogRecord() { - // TODO: in SDK, assign a default timestamp if not specified - name = ""; + // Assign default values + timestamp = core::SystemTimestamp(std::chrono::seconds(0)); + severity = Severity::kDefault; + trace_id = trace::TraceId(); + span_id = trace::SpanId(); + trace_flags = trace::TraceFlags(); } /* for ease of use; user can use this function to convert a map into a KeyValueIterable for the diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index 5f22066460..2f88ee038a 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -39,8 +39,7 @@ class Logger virtual ~Logger() = default; /* Returns the name of the logger */ - // TODO: decide whether this is useful and/or should be kept, as this is not a method required in - // the specification. virtual nostd::string_view getName() = 0; + virtual const nostd::string_view GetName() noexcept = 0; /** * Each of the following overloaded log(...) methods @@ -55,65 +54,338 @@ class Logger * @throws No exceptions under any circumstances. */ - /* The below method is a logging statement that takes in a LogRecord. - * A default LogRecord that will be assigned if no parameters are passed to Logger's .log() method - * which should at minimum assign the trace_id, span_id, and timestamp + /** + * Logs a LogRecord, which contains all the fields of the Log Data Model. Normally called indirectly from other Log() Methods, but can be called directly for high detail. + * @param record A log record filled with information from the user. */ - virtual void log(const LogRecord &record) noexcept = 0; + virtual void Log(const LogRecord &record) noexcept = 0; - /** Overloaded methods for unstructured logging **/ - inline void log(nostd::string_view message) noexcept + ///Overloaded Log methods, which are simpler than calling a LogRecord directly + + /** + * Writes a log. + * @param severity The log's severity + * @param message The message to log + */ + inline void Log(Severity severity, nostd::string_view message) noexcept { - // Set severity to the default then call log(Severity, String message) method - log(Severity::kDefault, message); - } + //Create a LogRecord to hold this information + LogRecord r; + r.severity = severity; + r.body = message; - inline void log(Severity severity, nostd::string_view message) noexcept - { - // TODO: set default timestamp later (not in API) - log(severity, message, core::SystemTimestamp(std::chrono::system_clock::now())); + //Call the main Log(LogRecord) method + Log(r); } - inline void log(Severity severity, - nostd::string_view message, - core::SystemTimestamp time) noexcept + /** + * Writes a log. + * @param severity The log's severity + * @param name The name of the log + * @param message The message to log + */ + inline void Log(Severity severity, nostd::string_view name, nostd::string_view message) noexcept { - // creates a LogRecord object with given parameters, then calls log(LogRecord) + //Create a LogRecord to hold this information LogRecord r; - r.severity = severity; - r.body = message; - r.timestamp = time; + r.severity = severity; + r.name = name; + r.body = message; - log(r); + //Call the main Log(LogRecord) method + Log(r); } - /** Overloaded methods for structured logging**/ - // TODO: separate this method into separate methods since it is not useful for user to create - // empty logs + /** + * Writes a log. + * @param severity The severity of the log, from 1 to 24 + * @param attributes A key/value object + */ template ::value> * = nullptr> - inline void log(Severity severity = Severity::kDefault, - nostd::string_view name = "", - const T &attributes = {}) noexcept + nostd::enable_if_t::value> * = nullptr> + inline void Log(Severity severity, const T &attributes) noexcept { - log(severity, name, common::KeyValueIterableView(attributes)); + //Create a LogRecord to hold this information + LogRecord r; + r.severity = severity; + r.attributes = nostd::shared_ptr(new common::KeyValueIterableView{attributes}); + + //Call the main Log(LogRecord) method + Log(r); } - inline void log(Severity severity, - nostd::string_view name, - const common::KeyValueIterable &attributes) noexcept + /** + * Writes a log. + * @param severity The severity of the log, from 1 to 24 + * @param name The name of the log + * @param attributes A key/value object + */ + template ::value> * = nullptr> + inline void Log(Severity severity, nostd::string_view name, const T &attributes) noexcept { - // creates a LogRecord object with given parameters, then calls log(LogRecord) + //Create a LogRecord to hold this information LogRecord r; r.severity = severity; r.name = name; - r.attributes = attributes; + r.attributes = nostd::shared_ptr(new common::KeyValueIterableView{attributes}); + + //Call the main Log(LogRecord) method + Log(r); + } + + /** + * Writes a log with a severity of trace. + * @param message The message to log + */ + inline void Trace(nostd::string_view message) noexcept + { + Log(Severity::kTrace, message); + } + + /** + * Writes a log with a severity of trace. + * @param name The name of the log + * @param message The message to log + */ + inline void Trace(nostd::string_view name, nostd::string_view message) noexcept + { + Log(Severity::kTrace, name, message); + } + + /** + * Writes a log with a severity of trace. + * @param attributes A key/value object + */ + template ::value> * = nullptr> + inline void Trace(const T &attributes) noexcept + { + Log(Severity::kTrace, attributes); + } + + /** + * Writes a log with a severity of trace. + * @param name The name of the log + * @param attributes A key/value object + */ + template ::value> * = nullptr> + inline void Trace(nostd::string_view name, const T &attributes) noexcept + { + Log(Severity::kTrace, name, attributes); + } + + /** + * Writes a log with a severity of debug. + * @param message The message to log + */ + inline void Debug(nostd::string_view message) noexcept + { + Log(Severity::kDebug, message); + } + + /** + * Writes a log with a severity of debug. + * @param name The name of the log + * @param message The message to log + */ + inline void Debug(nostd::string_view name, nostd::string_view message) noexcept + { + Log(Severity::kDebug, name, message); + } + + /** + * Writes a log with a severity of debug. + * @param attributes A key/value object + */ + template ::value> * = nullptr> + inline void Debug(const T &attributes) noexcept + { + Log(Severity::kDebug, attributes); + } + + /** + * Writes a log with a severity of debug. + * @param name The name of the log + * @param attributes A key/value object + */ + template ::value> * = nullptr> + inline void Debug(nostd::string_view name, const T &attributes) noexcept + { + Log(Severity::kDebug, name, attributes); + } + + /** + * Writes a log with a severity of info. + * @param message The message to log + */ + inline void Info(nostd::string_view message) noexcept + { + Log(Severity::kInfo, message); + } - log(r); + /** + * Writes a log with a severity of info. + * @param name The name of the log + * @param message The message to log + */ + inline void Info(nostd::string_view name, nostd::string_view message) noexcept + { + Log(Severity::kInfo, name, message); + } + + /** + * Writes a log with a severity of info. + * @param attributes A key/value object + */ + template ::value> * = nullptr> + inline void Info(const T &attributes) noexcept + { + Log(Severity::kInfo, attributes); + } + + /** + * Writes a log with a severity of info. + * @param name The name of the log + * @param attributes A key/value object + */ + template ::value> * = nullptr> + inline void Info(nostd::string_view name, const T &attributes) noexcept + { + Log(Severity::kInfo, name, attributes); + } + + /** + * Writes a log with a severity of warn. + * @param message The message to log + */ + inline void Warn(nostd::string_view message) noexcept + { + Log(Severity::kWarn, message); + } + + /** + * Writes a log with a severity of warn. + * @param name The name of the log + * @param message The message to log + */ + inline void Warn(nostd::string_view name, nostd::string_view message) noexcept + { + Log(Severity::kWarn, name, message); + } + + /** + * Writes a log with a severity of warn. + * @param attributes A key/value object + */ + template ::value> * = nullptr> + inline void Warn(const T &attributes) noexcept + { + Log(Severity::kWarn, attributes); + } + + /** + * Writes a log with a severity of warn. + * @param name The name of the log + * @param attributes A key/value object + */ + template ::value> * = nullptr> + inline void Warn(nostd::string_view name, const T &attributes) noexcept + { + Log(Severity::kWarn, name, attributes); + } + + /** + * Writes a log with a severity of error. + * @param message The message to log + */ + inline void Error(nostd::string_view message) noexcept + { + Log(Severity::kError, message); + } + + /** + * Writes a log with a severity of error. + * @param name The name of the log + * @param message The message to log + */ + inline void Error(nostd::string_view name, nostd::string_view message) noexcept + { + Log(Severity::kError, name, message); + } + + /** + * Writes a log with a severity of error. + * @param attributes A key/value object + */ + template ::value> * = nullptr> + inline void Error(const T &attributes) noexcept + { + Log(Severity::kError, attributes); + } + + /** + * Writes a log with a severity of error. + * @param name The name of the log + * @param attributes A key/value object + */ + template ::value> * = nullptr> + inline void Error(nostd::string_view name, const T &attributes) noexcept + { + Log(Severity::kError, name, attributes); + } + + /** + * Writes a log with a severity of fatal. + * @param message The message to log + */ + inline void Fatal(nostd::string_view message) noexcept + { + Log(Severity::kFatal, message); + } + + /** + * Writes a log with a severity of fatal. + * @param name The name of the log + * @param message The message to log + */ + inline void Fatal(nostd::string_view name, nostd::string_view message) noexcept + { + Log(Severity::kFatal, name, message); + } + + /** + * Writes a log with a severity of fatal. + * @param attributes A key/value object + */ + template ::value> * = nullptr> + inline void Fatal(const T &attributes) noexcept + { + Log(Severity::kFatal, attributes); + } + + /** + * Writes a log with a severity of fatal. + * @param name The name of the log + * @param attributes A key/value object + */ + template ::value> * = nullptr> + inline void Fatal(nostd::string_view name, const T &attributes) noexcept + { + Log(Severity::kFatal, name, attributes); } - // TODO: add function aliases such as void debug(), void trace(), void info(), etc. for each - // severity level /** Future enhancement: templated method for objects / custom types (e.g. JSON, XML, custom * classes, etc) **/ diff --git a/api/include/opentelemetry/logs/noop.h b/api/include/opentelemetry/logs/noop.h index 3184baa3c6..43406771d9 100644 --- a/api/include/opentelemetry/logs/noop.h +++ b/api/include/opentelemetry/logs/noop.h @@ -41,7 +41,8 @@ class NoopLogger final : public Logger public: NoopLogger() = default; - void log(const LogRecord &record) noexcept override {} + void Log(const LogRecord& record) noexcept override {} + const nostd::string_view GetName() noexcept override { return "noop logger"; }; }; /** diff --git a/api/test/logs/BUILD b/api/test/logs/BUILD index 899400b2e8..cba6f19220 100644 --- a/api/test/logs/BUILD +++ b/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", diff --git a/api/test/logs/CMakeLists.txt b/api/test/logs/CMakeLists.txt index 9b1b3b0e7d..8f797b97c1 100644 --- a/api/test/logs/CMakeLists.txt +++ b/api/test/logs/CMakeLists.txt @@ -1,9 +1,6 @@ -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} - TEST_PREFIX logs. - TEST_LIST ${testname}) + gtest_add_tests(TARGET logs_api_${testname} TEST_PREFIX logs. TEST_LIST logs_api_${testname}) endforeach() diff --git a/api/test/logs/logger_test.cc b/api/test/logs/logger_test.cc index 937ece860a..b05e333a93 100644 --- a/api/test/logs/logger_test.cc +++ b/api/test/logs/logger_test.cc @@ -15,43 +15,41 @@ using opentelemetry::nostd::shared_ptr; using opentelemetry::nostd::span; using opentelemetry::nostd::string_view; -TEST(Logger, GetLoggerDefault) +TEST(LoggerTest, GetLoggerDefaultNoop) { auto lp = Provider::GetLoggerProvider(); auto logger = lp->GetLogger("TestLogger"); EXPECT_NE(nullptr, logger); + EXPECT_EQ(logger->GetName(), "noop logger"); } -TEST(Logger, GetNoopLoggerName) -{ - auto lp = Provider::GetLoggerProvider(); - auto logger = lp->GetLogger("TestLogger"); -} - -TEST(Logger, GetNoopLoggerNameWithArgs) +TEST(LoggerTest, GetLogger) { auto lp = Provider::GetLoggerProvider(); + // Get a logger with no arguments + auto logger1 = lp->GetLogger("TestLogger1"); + + // Get a logger with options passed + auto logger2 = lp->GetLogger("TestLogger2", "Options"); + + // Get a logger with arguments std::array sv{"string"}; span 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 + auto logger3 = lp->GetLogger("TestLogger3", args); } -TEST(Logger, NoopLog) -{ - auto lp = Provider::GetLoggerProvider(); - auto logger = lp->GetLogger("TestLogger"); - LogRecord r; - r.name = "Noop log name"; - logger->log(r); -} +// Define a global log record that will be modified when the Log() method is called +static opentelemetry::nostd::shared_ptr record_; // Define a basic Logger class class TestLogger : public Logger { - void log(const LogRecord &record) noexcept override {} + void Log(const LogRecord &record) noexcept override + { + record_ = opentelemetry::nostd::shared_ptr(new LogRecord(record)); + } + const opentelemetry::nostd::string_view GetName() noexcept override { return "test logger"; }; }; // Define a basic LoggerProvider class that returns an instance of the logger class defined above @@ -68,19 +66,67 @@ class TestProvider : public LoggerProvider } }; -TEST(Logger, PushLoggerImplementation) +TEST(LoggerTest, PushLoggerImplementation) { - // Push the new loggerprovider class into the global singleton + // Push the new loggerprovider class into the API auto test_provider = shared_ptr(new TestProvider()); Provider::SetLoggerProvider(test_provider); auto lp = Provider::GetLoggerProvider(); - // GetLogger(name, options) function + // Get a logger instance and check whether it's GetName() method returns + // "test logger" as defined in the custom implementation auto logger = lp->GetLogger("TestLogger"); + ASSERT_EQ("test logger", logger->GetName()); +} - // GetLogger(name, args) function - std::array sv{"string"}; - span args{sv}; - auto logger2 = lp->GetLogger("TestLogger2", args); +TEST(Logger, LogMethodOverloads) +{ + // Use the same TestProvider and TestLogger from the previous test + auto test_provider = shared_ptr(new TestProvider()); + Provider::SetLoggerProvider(test_provider); + + auto lp = Provider::GetLoggerProvider(); + auto logger = lp->GetLogger("TestLogger"); + + // Check that calling the Log() overloads correctly constructs a log record which is automatically put into the static logger_ for testing + + // Test Log(severity, name, message) method + logger->Log(Severity::kError, "Log Name", "This is the log message"); + ASSERT_EQ(record_->severity, Severity::kError); + ASSERT_EQ(record_->name, "Log Name"); + ASSERT_EQ(record_->body, "This is the log message"); + + // Test Trace(name, KVIterable) method + std::map m = {{"key1", "val1"}, {"key2", "val2"}}; + logger->Trace("Logging a map", m); + ASSERT_EQ(record_->severity, Severity::kTrace); + ASSERT_EQ(record_->name, "Logging a map"); + ASSERT_EQ(record_->attributes->size(), 2); +} + +TEST(LogRecord, SetDefault) +{ + LogRecord r; + + // Check that the timestamp is set to 0 by default + ASSERT_EQ(r.timestamp, opentelemetry::core::SystemTimestamp(std::chrono::seconds(0))); + + // Check that the severity is set to kDefault by default + ASSERT_EQ(r.severity, Severity::kDefault); + + // Check that trace_id is set to all zeros by default + char trace_buf[32]; + r.trace_id.ToLowerBase16(trace_buf); + ASSERT_EQ(std::string(trace_buf, sizeof(trace_buf)), "00000000000000000000000000000000"); + + // Check that span_id is set to all zeros by default + char span_buf[16]; + r.span_id.ToLowerBase16(span_buf); + ASSERT_EQ(std::string(span_buf, sizeof(span_buf)), "0000000000000000"); + + // Check that trace_flags is set to all zeros by default + char flags_buf[2]; + r.trace_flags.ToLowerBase16(flags_buf); + ASSERT_EQ(std::string(flags_buf, sizeof(flags_buf)), "00"); } diff --git a/api/test/logs/logger_provider_test.cc b/api/test/logs/provider_test.cc similarity index 100% rename from api/test/logs/logger_provider_test.cc rename to api/test/logs/provider_test.cc diff --git a/sdk/include/opentelemetry/sdk/logs/logger.h b/sdk/include/opentelemetry/sdk/logs/logger.h index 4b49351030..eb629bd480 100644 --- a/sdk/include/opentelemetry/sdk/logs/logger.h +++ b/sdk/include/opentelemetry/sdk/logs/logger.h @@ -39,20 +39,30 @@ 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(std::shared_ptr logger_provider) noexcept; + explicit Logger(opentelemetry::nostd::string_view name, + std::shared_ptr 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 record The record to write into the processor. */ - void log(const opentelemetry::logs::LogRecord &record) noexcept override; + void Log(const opentelemetry::logs::LogRecord &record) noexcept override; private: // The logger provider of this Logger. Uses a weak_ptr to avoid cyclic dependancy issues the with // logger provider std::weak_ptr logger_provider_; + + // The name of this logger + opentelemetry::nostd::string_view logger_name_; }; } // namespace logs diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index bc19f5d1c1..4eab6d89f8 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -21,11 +21,17 @@ namespace sdk { namespace logs { -Logger::Logger(std::shared_ptr logger_provider) noexcept - : logger_provider_(logger_provider) +Logger::Logger(opentelemetry::nostd::string_view name, + std::shared_ptr logger_provider) noexcept + : logger_name_(name), logger_provider_(logger_provider) {} -void Logger::log(const opentelemetry::logs::LogRecord &record) noexcept +const opentelemetry::nostd::string_view Logger::GetName() noexcept +{ + return logger_name_; +} + +void Logger::Log(const opentelemetry::logs::LogRecord &record) noexcept { // If this logger does not have a processor, no need to create a log record auto processor = logger_provider_.lock()->GetProcessor(); diff --git a/sdk/src/logs/logger_provider.cc b/sdk/src/logs/logger_provider.cc index b040f9baab..6c978c3472 100644 --- a/sdk/src/logs/logger_provider.cc +++ b/sdk/src/logs/logger_provider.cc @@ -54,7 +54,7 @@ opentelemetry::nostd::shared_ptr LoggerProvider::Ge // If no logger with that name exists yet, create it and add it to the map of loggers opentelemetry::nostd::shared_ptr logger( - new Logger(this->shared_from_this())); + new Logger(name, this->shared_from_this())); loggers_[name.data()] = logger; return logger; } diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index d4825d0032..e278210e01 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -32,7 +32,7 @@ TEST(LoggerSDK, LogToNullProcessor) // Log a sample log record to a nullptr processor opentelemetry::logs::LogRecord r; r.name = "Test log"; - logger->log(r); + logger->Log(r); } class DummyProcessor : public LogProcessor @@ -73,5 +73,5 @@ TEST(LoggerSDK, LogToAProcessor) // Log a sample log record to the processor opentelemetry::logs::LogRecord r; r.name = "Test log"; - logger->log(r); + logger->Log(r); }