From 383c5ca65db7dbd0bfc981fa27fbaa1b0fa90e8e Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Tue, 13 Oct 2020 00:36:16 -0400 Subject: [PATCH 01/17] Cleaup commits --- api/include/opentelemetry/logs/TBD | 0 api/include/opentelemetry/logs/log_record.h | 74 +++++++++++++ api/include/opentelemetry/logs/logger.h | 103 ++++++++++++++++++ .../opentelemetry/logs/logger_provider.h | 36 ++++++ api/include/opentelemetry/logs/noop.h | 74 +++++++++++++ api/include/opentelemetry/logs/provider.h | 55 ++++++++++ api/test/CMakeLists.txt | 1 + api/test/logs/BUILD | 35 ++++++ api/test/logs/CMakeLists.txt | 6 + api/test/logs/log_record_test.cc | 20 ++++ api/test/logs/logger_provider_test.cc | 48 ++++++++ api/test/logs/logger_test.cc | 71 ++++++++++++ 12 files changed, 523 insertions(+) delete mode 100644 api/include/opentelemetry/logs/TBD create mode 100644 api/include/opentelemetry/logs/log_record.h create mode 100644 api/include/opentelemetry/logs/logger.h create mode 100644 api/include/opentelemetry/logs/logger_provider.h create mode 100644 api/include/opentelemetry/logs/noop.h create mode 100644 api/include/opentelemetry/logs/provider.h create mode 100644 api/test/logs/BUILD create mode 100644 api/test/logs/CMakeLists.txt create mode 100644 api/test/logs/log_record_test.cc create mode 100644 api/test/logs/logger_provider_test.cc create mode 100644 api/test/logs/logger_test.cc diff --git a/api/include/opentelemetry/logs/TBD b/api/include/opentelemetry/logs/TBD deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h new file mode 100644 index 0000000000..7795a70f7e --- /dev/null +++ b/api/include/opentelemetry/logs/log_record.h @@ -0,0 +1,74 @@ +#pragma once + +#include +#include +#include "opentelemetry/common/key_value_iterable_view.h" +#include "opentelemetry/core/timestamp.h" +#include "opentelemetry/logs/logger.h" +#include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/trace/span_id.h" +#include "opentelemetry/trace/trace_flags.h" +#include "opentelemetry/trace/trace_id.h" +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace logs +{ + +enum class Severity // using a class enum here won't allow enum values to be compared to integers + // only other Severity enums (need an explicit cast) +{ + NONE = 0, // default Severity; added, not part of Log Data Model + TRACE = 1, + TRACE2 = 2, + TRACE3 = 3, + TRACE4 = 4, + DEBUG = 5, + DEBUG2 = 6, + DEBUG3 = 7, + DEBUG4 = 8, + INFO = 9, + INFO2 = 10, + INFO3 = 11, + INFO4 = 12, + WARN = 13, + WARN2 = 14, + WARN3 = 15, + WARN4 = 16, + ERROR = 17, + ERROR2 = 18, + ERROR3 = 19, + ERROR4 = 20, + FATAL = 21, + FATAL2 = 22, + FATAL3 = 23, + FATAL4 = 24 +}; + +/** + * A default Event object to be passed in log statements, + * matching the 10 fields of the Log Data Model. + * (https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/logs/data-model.md#log-and-event-record-definition) + * + */ +struct LogRecord +{ + + // default fields that will be set if the user doesn't specify them + uint64_t timestamp = 0; // uint64 nanoseconds since Unix epoch + trace::TraceId trace_id; // byte sequence + trace::SpanId span_id; // byte sequence + trace::TraceFlags trace_flag; // byte + nostd::string_view severity_text; // string; TODO: add a severity_num_to_text() method in + // Logger.h's Severity class enum? + // Severity severity_number = Severity::NONE; // integer + + // other fields + nostd::string_view name; // string + common::AttributeValue body; // any type + nostd::shared_ptr resource; // key/value pair list + nostd::shared_ptr attributes; // key/value pair list +}; +} // namespace logs +OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h new file mode 100644 index 0000000000..46a23b0485 --- /dev/null +++ b/api/include/opentelemetry/logs/logger.h @@ -0,0 +1,103 @@ +#pragma once +#include +#include + +#include "opentelemetry/common/key_value_iterable.h" // fix after moving to "common" +#include "opentelemetry/logs/log_record.h" +#include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/nostd/span.h" +#include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace logs +{ +/** + * Handles log record creation. + * +// */ +// static const struct LogRecord defaultRecord = { +// timestamp : std::chrono::steady_clock::now().time_since_epoch(), +// trace_id : 0, +// span_id : 0, +// severity_number : 0 +// }; + +class Logger +{ +private: + std::vector records; + +public: + virtual ~Logger() = default; + + /* Returns the name of the logger */ + virtual nostd::string_view getName() = 0; + + /** + * Creates a log message with the passed characteristics. + * + * @param name the name of the log event. + * @param sev the severity level of the log event. + * @param msg the string message of the log (perhaps support std::fmt or fmt-lib format). + * @param record the log record (object type LogRecord) that is logged. + * @param attributes the attributes, stored as a 2D list of key/value pairs, that are associated + * with this log. + * @param some_obj the log object of user templated type that is logged. + * @throws No exceptions under any circumstances. + */ + + /** A method for an unstructured log **/ + virtual void log(nostd::string_view name, Severity sev, nostd::string_view msg) noexcept = 0; + + /** Methods for structured logs **/ + // (name, sev, KeyValueIterable) overloads + virtual void log(nostd::string_view name = "", + Severity sev = Severity::NONE, + const common::KeyValueIterable &attributes = {}) noexcept = 0; + + /* + void log(Severity sev, const common::KeyValueIterable &attributes) noexcept + { + log("", sev, attributes); + } + void log(nostd::string_view name, const common::KeyValueIterable &attributes) noexcept + { + log(name, Severity::NONE, attributes); + } + void log(const common::KeyValueIterable &attributes) noexcept { log("", attributes); } + */ + + // (name, sev, KeyValueIterable) overloads + template ::value> * = nullptr> + void log(nostd::string_view name = "", + Severity sev = Severity::NONE, + const T &attributes = {}) noexcept + { + log(name, sev, common::KeyValueIterableView(attributes)); + } + + void log(nostd::string_view name, + std::initializer_list> + attributes) noexcept + { + // log(name, nostd::span>{attributes.begin(), attributes.end()}); + } + + /* 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 contain the trace_id, span_id, and current timestamp + * TODO: correlate timestamp, traceid and spanid at the minimum to Log Record + * + */ + virtual void log(const LogRecord &record = {}) noexcept = 0; + + /*** Future additions*/ + /* templated method for objects / custom types (e.g. JSON, XML, custom classes, etc) (for later) + */ + // template void log(T &some_obj) noexcept override; +}; +} // namespace logs +OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/logs/logger_provider.h b/api/include/opentelemetry/logs/logger_provider.h new file mode 100644 index 0000000000..89ce19a8a1 --- /dev/null +++ b/api/include/opentelemetry/logs/logger_provider.h @@ -0,0 +1,36 @@ +#pragma once + +#include "opentelemetry/logs/logger.h" +#include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/nostd/string_view.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace logs +{ +/** + * Creates new Logger instances. + */ +class LoggerProvider +{ +public: + virtual ~LoggerProvider() = default; + + /** + * Gets or creates a named Logger instance. + * + * Optionally a version can be passed to create a named and versioned Logger + * instance. + * + * Optionally a configuration file name can be passed to create a configuration for + * the Logger instance. + * + */ + + virtual nostd::shared_ptr GetLogger(nostd::string_view logger_name, + nostd::string_view options = "") = 0; + + virtual nostd::shared_ptr GetLogger(nostd::string_view logger_name, + nostd::span args) = 0; +}; +} // namespace logs +OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/logs/noop.h b/api/include/opentelemetry/logs/noop.h new file mode 100644 index 0000000000..799b8648ec --- /dev/null +++ b/api/include/opentelemetry/logs/noop.h @@ -0,0 +1,74 @@ +#pragma once +// Please refer to provider.h for documentation on how to obtain a Logger object. +// +// This file is part of the internal implementation of OpenTelemetry. Nothing in this file should be +// used directly. Please refer to logger.h for documentation on these interfaces. + +#include "opentelemetry/context/runtime_context.h" +#include "opentelemetry/logs/logger.h" +#include "opentelemetry/logs/logger_provider.h" +#include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/nostd/unique_ptr.h" +#include "opentelemetry/version.h" + +#include + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace logs +{ +/** + * No-op implementation of Logger. This class should not be used directly. It should only be + * instantiated using a LoggerProvider's GetLogger() call. + */ +class NoopLogger final : public Logger +{ +public: + NoopLogger() = default; + + // returns the name of the logger + nostd::string_view getName() noexcept override { return "NOOP Logger"; } + + // unstructured logging + void log(nostd::string_view name, Severity sev, nostd::string_view msg) noexcept override {} + + // structured logging + void log(nostd::string_view name, + Severity sev, + const common::KeyValueIterable &attributes) noexcept override + {} + // void log(nostd::string_view name, const common::KeyValueIterable &attributes) noexcept override + // {} + void log(const LogRecord &record) noexcept override {} + + // templated method for objects / custom types (e.g. JSON, XML, custom classes, etc); may support + // later template void log(T &some_obj) noexcept override {} +}; + +/** + * No-op implementation of a LoggerProvider. + */ +class NoopLoggerProvider final : public opentelemetry::logs::LoggerProvider +{ +public: + NoopLoggerProvider() + : logger_{ + nostd::shared_ptr(new opentelemetry::logs::NoopLogger)} + {} + + nostd::shared_ptr GetLogger(nostd::string_view logger_name, + nostd::string_view options) override + { + return logger_; + } + + nostd::shared_ptr GetLogger(nostd::string_view logger_name, + nostd::span args) override + { + return logger_; + } + +private: + nostd::shared_ptr logger_; +}; +} // namespace logs +OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/logs/provider.h b/api/include/opentelemetry/logs/provider.h new file mode 100644 index 0000000000..90a00d881a --- /dev/null +++ b/api/include/opentelemetry/logs/provider.h @@ -0,0 +1,55 @@ +#pragma once + +#include + +#include "opentelemetry/common/spin_lock_mutex.h" +#include "opentelemetry/logs/logger_provider.h" +#include "opentelemetry/logs/noop.h" +#include "opentelemetry/nostd/shared_ptr.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace logs +{ +/** + * Stores the singleton global LoggerProvider. + */ +class Provider +{ +public: + /** + * Returns the singleton LoggerProvider. + * + * By default, a no-op LoggerProvider is returned. This will never return a + * nullptr LoggerProvider. + */ + static nostd::shared_ptr GetLoggerProvider() noexcept + { + std::lock_guard guard(GetLock()); + return nostd::shared_ptr(GetProvider()); + } + + /** + * Changes the singleton LoggerProvider. + */ + static void SetLoggerProvider(nostd::shared_ptr tp) noexcept + { + std::lock_guard guard(GetLock()); + GetProvider() = tp; + } + +private: + static nostd::shared_ptr &GetProvider() noexcept + { + static nostd::shared_ptr provider(new NoopLoggerProvider); + return provider; + } + + static common::SpinLockMutex &GetLock() noexcept + { + static common::SpinLockMutex lock; + return lock; + } +}; + +} // namespace logs +OPENTELEMETRY_END_NAMESPACE diff --git a/api/test/CMakeLists.txt b/api/test/CMakeLists.txt index 11cf6d1319..05ed4f5720 100644 --- a/api/test/CMakeLists.txt +++ b/api/test/CMakeLists.txt @@ -4,3 +4,4 @@ add_subdirectory(plugin) add_subdirectory(nostd) add_subdirectory(trace) add_subdirectory(metrics) +add_subdirectory(logs) diff --git a/api/test/logs/BUILD b/api/test/logs/BUILD new file mode 100644 index 0000000000..4fb5c351e7 --- /dev/null +++ b/api/test/logs/BUILD @@ -0,0 +1,35 @@ +load("//bazel:otel_cc_benchmark.bzl", "otel_cc_benchmark") + +cc_test( + name = "logger_provider_test", + srcs = [ + "logger_provider_test.cc", + ], + deps = [ + "//api", + "@com_google_googletest//:gtest_main", + ], +) + +cc_test( + name = "logger_test", + srcs = [ + "logger_test.cc", + ], + deps = [ + "//api", + "@com_google_googletest//:gtest_main", + ], +) + +cc_test( + name = "log_record_test", + srcs = [ + "log_record_test.cc", + ], + linkstatic = 1, + deps = [ + "//api", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/api/test/logs/CMakeLists.txt b/api/test/logs/CMakeLists.txt new file mode 100644 index 0000000000..872daf2d53 --- /dev/null +++ b/api/test/logs/CMakeLists.txt @@ -0,0 +1,6 @@ +foreach(testname logger_provider_test logger_test log_record_test) + add_executable(${testname} "${testname}.cc") + target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} + ${CMAKE_THREAD_LIBS_INIT} opentelemetry_api) + gtest_add_tests(TARGET ${testname} TEST_PREFIX logs. TEST_LIST ${testname}) +endforeach() diff --git a/api/test/logs/log_record_test.cc b/api/test/logs/log_record_test.cc new file mode 100644 index 0000000000..f3774984dc --- /dev/null +++ b/api/test/logs/log_record_test.cc @@ -0,0 +1,20 @@ +#include "opentelemetry/logs/provider.h" +#include "opentelemetry/nostd/shared_ptr.h" + +#include + +using opentelemetry::logs::Logger; +using opentelemetry::logs::LoggerProvider; +using opentelemetry::logs::Provider; + +// TODO +TEST(LogRecord, TimestampStore) {} +TEST(LogRecord, TraceIdStore){} +TEST(LogRecord, SpanIdStore){} +TEST(LogRecord, TraceFlagsStore){} +TEST(LogRecord, SeverityTextStore){} +TEST(LogRecord, SeverityNumberStore){} +TEST(LogRecord, NameStore){} +TEST(LogRecord, BodyStore){} +TEST(LogRecord, ResourceStore){} +TEST(LogRecord, AttributesStore){} diff --git a/api/test/logs/logger_provider_test.cc b/api/test/logs/logger_provider_test.cc new file mode 100644 index 0000000000..fdaadbebe2 --- /dev/null +++ b/api/test/logs/logger_provider_test.cc @@ -0,0 +1,48 @@ +#include "opentelemetry/logs/provider.h" +#include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/nostd/span.h" +#include "opentelemetry/nostd/string_view.h" + +#include + +using opentelemetry::logs::Logger; +using opentelemetry::logs::LoggerProvider; +using opentelemetry::logs::Provider; +using opentelemetry::nostd::shared_ptr; +using opentelemetry::nostd::span; +using opentelemetry::nostd::string_view; + +class TestProvider : public LoggerProvider +{ + shared_ptr GetLogger(string_view library_name, string_view options) override + { + return shared_ptr(nullptr); + } + + shared_ptr GetLogger(string_view library_name, span args) override + { + return shared_ptr(nullptr); + } + + class TestProvider : public LoggerProvider + { + auto tf = Provider::GetLoggerProvider(); + EXPECT_NE(nullptr, tf); + } + + TEST(Provider, SetLoggerProvider) + { + auto tf = shared_ptr(new TestProvider()); + Provider::SetLoggerProvider(tf); + ASSERT_EQ(tf, Provider::GetLoggerProvider()); + } + + TEST(Provider, MultipleLoggerProviders) + { + auto tf = shared_ptr(new TestProvider()); + Provider::SetLoggerProvider(tf); + auto tf2 = shared_ptr(new TestProvider()); + + ASSERT_NE(Provider::GetLoggerProvider(), tf); + } + \ No newline at end of file diff --git a/api/test/logs/logger_test.cc b/api/test/logs/logger_test.cc new file mode 100644 index 0000000000..a3dc1946bc --- /dev/null +++ b/api/test/logs/logger_test.cc @@ -0,0 +1,71 @@ +#include "opentelemetry/logs/logger.h" +#include "opentelemetry/logs/provider.h" +#include "opentelemetry/nostd/shared_ptr.h" + +#include + +using opentelemetry::common::KeyValueIterable; +using opentelemetry::logs::Logger; +using opentelemetry::logs::LoggerProvider; +using opentelemetry::logs::LogRecord; +using opentelemetry::logs::Provider; +using opentelemetry::logs::Severity; +using opentelemetry::nostd::shared_ptr; +using opentelemetry::nostd::span; +using opentelemetry::nostd::string_view; + +TEST(Logger, GetLoggerDefault) +{ + auto lp = Provider::GetLoggerProvider(); + auto logger = lp->GetLogger("TestLogger"); + EXPECT_NE(nullptr, logger); +} + +TEST(Logger, GetNoopLoggerName) +{ + auto lp = Provider::GetLoggerProvider(); + auto logger = lp->GetLogger("TestLogger"); + string_view name = logger->getName(); + EXPECT_EQ("NOOP Logger", name); +} + +// Define a basic Logger class +class TestLogger : public Logger +{ + // returns the name of the logger + string_view getName() noexcept override { return "My custom implementation"; } + + // unstructured logging + void log(string_view name, Severity sev, string_view msg) noexcept override {} + + // structured logging + void log(string_view name, Severity sev, const KeyValueIterable &attributes) noexcept override {} + // void log(string_view name, const KeyValueIterable &attributes) noexcept override {} + void log(const LogRecord &record) noexcept override {} +}; + +// Define a basic LoggerProvider class that returns an instance of the logger class defined above +class TestProvider : public LoggerProvider +{ + shared_ptr GetLogger(string_view library_name, string_view options = "") override + { + return shared_ptr(new TestLogger()); + } + + shared_ptr GetLogger(string_view library_name, span args) override + { + return shared_ptr(new TestLogger()); + } +}; + +TEST(Logger, PushLoggerImplementation) +{ + // Push the new loggerprovider class into the global singleton + auto test_provider = shared_ptr(new TestProvider()); + Provider::SetLoggerProvider(test_provider); + + auto lp = Provider::GetLoggerProvider(); + auto logger = lp->GetLogger("TestLogger"); + string_view name = logger->getName(); + EXPECT_EQ("My custom implementation", name); +} From 042b63f962233b666d3fcc81332d296759d84593 Mon Sep 17 00:00:00 2001 From: Seufert Date: Mon, 19 Oct 2020 15:39:37 -0700 Subject: [PATCH 02/17] Default arguments for log_record, changed attributes to be KeyValueIterable & --- api/include/opentelemetry/logs/log_record.h | 56 +++++++++++++++++---- api/include/opentelemetry/logs/logger.h | 47 ++++++++--------- api/include/opentelemetry/logs/noop.h | 3 -- api/test/logs/logger_provider_test.cc | 39 +++++++------- api/test/logs/logger_test.cc | 3 -- 5 files changed, 89 insertions(+), 59 deletions(-) diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h index 7795a70f7e..01ab512cb4 100644 --- a/api/include/opentelemetry/logs/log_record.h +++ b/api/include/opentelemetry/logs/log_record.h @@ -24,7 +24,7 @@ enum class Severity // using a class enum here won't allow enum values to be co TRACE2 = 2, TRACE3 = 3, TRACE4 = 4, - DEBUG = 5, + DEBUG1 = 5, DEBUG2 = 6, DEBUG3 = 7, DEBUG4 = 8, @@ -54,21 +54,59 @@ enum class Severity // using a class enum here won't allow enum values to be co */ struct LogRecord { - +public: // default fields that will be set if the user doesn't specify them - uint64_t timestamp = 0; // uint64 nanoseconds since Unix epoch + uint64_t timestamp; // uint64 nanoseconds since Unix epoch trace::TraceId trace_id; // byte sequence trace::SpanId span_id; // byte sequence trace::TraceFlags trace_flag; // byte - nostd::string_view severity_text; // string; TODO: add a severity_num_to_text() method in - // Logger.h's Severity class enum? - // Severity severity_number = Severity::NONE; // integer + nostd::string_view severity_text; // string; TODO: add a severity_num_to_text() method + Severity severity_number; // integer // other fields nostd::string_view name; // string - common::AttributeValue body; // any type - nostd::shared_ptr resource; // key/value pair list - nostd::shared_ptr attributes; // key/value pair list + bool body; // currently using bool to represent whether it was defined, change once defined + common::KeyValueIterable &resource; // key/value pair list + common::KeyValueIterable &attributes; // key/value pair list + + //Default constructor where all the values are set to defaults + /* -------------------------------------------------------------------------- + TODO: Potentially add other constructors to take default arguments from the user + ----------------------------------------------------------------------------- */ + LogRecord() : resource(_nullKV), attributes(_nullKV) { + timestamp = 0; + //trace_id is by default 00000000000000000000000000000000 + //span_id is by default 0000000000000000 + //trace_flag is by default 0 + severity_text = "NONE"; + severity_number = Severity::NONE; + name = ""; + body = false; + //resource and attributes are set to _nullKV on line 85. + //_nullKV is defined as a private variable that allows resource and + //attributes to be instantiated using it as the default value + } + + + // convert a map into a KeyValueIterable for the resources + template ::value> * = nullptr> + inline void SetResource(const T &_resource) + { + resource = common::KeyValueIterableView(_resource); + } + + // convert a map into a KeyValueIterable for the attributes + template ::value> * = nullptr> + inline void SetAttributes(const T &_attributes) + { + attributes = common::KeyValueIterableView(_attributes); + } + +private: + //Used as a default for the resource and attributes fields + opentelemetry::common::KeyValueIterableView> _nullKV = opentelemetry::common::KeyValueIterableView>{{}}; }; } // namespace logs OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index 46a23b0485..95b1dd9db1 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -1,6 +1,7 @@ #pragma once #include #include +#include #include "opentelemetry/common/key_value_iterable.h" // fix after moving to "common" #include "opentelemetry/logs/log_record.h" @@ -47,26 +48,30 @@ class Logger * @throws No exceptions under any circumstances. */ - /** A method for an unstructured log **/ - virtual void log(nostd::string_view name, Severity sev, nostd::string_view msg) noexcept = 0; + /** Methods for an unstructured logging **/ + void log(nostd::string_view msg) noexcept { + log(Severity::NONE, msg); //Set severity to NONE as default then call the log method below + } + + void log(Severity sev, nostd::string_view msg) noexcept { + log(sev, msg, 0); //Set timestamp to 0 as default then call the log method below + } + void log(Severity sev, nostd::string_view msg, uint64_t time) noexcept { + LogRecord r; + r.name = msg; + r.severity_number = sev; + r.severity_text = "NONE"; //TODO, create function to map severity_number to text + r.timestamp = time; + log(r); //Call the log(LogRecord) log method + } + /** Methods for structured logs **/ // (name, sev, KeyValueIterable) overloads - virtual void log(nostd::string_view name = "", - Severity sev = Severity::NONE, - const common::KeyValueIterable &attributes = {}) noexcept = 0; + virtual void log(nostd::string_view name, + Severity sev, + const common::KeyValueIterable &attributes) noexcept = 0; - /* - void log(Severity sev, const common::KeyValueIterable &attributes) noexcept - { - log("", sev, attributes); - } - void log(nostd::string_view name, const common::KeyValueIterable &attributes) noexcept - { - log(name, Severity::NONE, attributes); - } - void log(const common::KeyValueIterable &attributes) noexcept { log("", attributes); } - */ // (name, sev, KeyValueIterable) overloads template (attributes)); } - void log(nostd::string_view name, - std::initializer_list> - attributes) noexcept - { - // log(name, nostd::span>{attributes.begin(), attributes.end()}); - } - /* 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 contain the trace_id, span_id, and current timestamp * TODO: correlate timestamp, traceid and spanid at the minimum to Log Record * */ - virtual void log(const LogRecord &record = {}) noexcept = 0; + virtual void log(const LogRecord &record) noexcept = 0; /*** Future additions*/ /* templated method for objects / custom types (e.g. JSON, XML, custom classes, etc) (for later) diff --git a/api/include/opentelemetry/logs/noop.h b/api/include/opentelemetry/logs/noop.h index 799b8648ec..1c9f56d3e4 100644 --- a/api/include/opentelemetry/logs/noop.h +++ b/api/include/opentelemetry/logs/noop.h @@ -28,9 +28,6 @@ class NoopLogger final : public Logger // returns the name of the logger nostd::string_view getName() noexcept override { return "NOOP Logger"; } - // unstructured logging - void log(nostd::string_view name, Severity sev, nostd::string_view msg) noexcept override {} - // structured logging void log(nostd::string_view name, Severity sev, diff --git a/api/test/logs/logger_provider_test.cc b/api/test/logs/logger_provider_test.cc index fdaadbebe2..7f1385424b 100644 --- a/api/test/logs/logger_provider_test.cc +++ b/api/test/logs/logger_provider_test.cc @@ -23,26 +23,27 @@ class TestProvider : public LoggerProvider { return shared_ptr(nullptr); } +}; - class TestProvider : public LoggerProvider - { - auto tf = Provider::GetLoggerProvider(); - EXPECT_NE(nullptr, tf); - } +TEST(Provider, GetLoggerProviderDefault) +{ + auto tf = Provider::GetLoggerProvider(); + EXPECT_NE(nullptr, tf); +} - TEST(Provider, SetLoggerProvider) - { - auto tf = shared_ptr(new TestProvider()); - Provider::SetLoggerProvider(tf); - ASSERT_EQ(tf, Provider::GetLoggerProvider()); - } +TEST(Provider, SetLoggerProvider) +{ + auto tf = shared_ptr(new TestProvider()); + Provider::SetLoggerProvider(tf); + ASSERT_EQ(tf, Provider::GetLoggerProvider()); +} - TEST(Provider, MultipleLoggerProviders) - { - auto tf = shared_ptr(new TestProvider()); - Provider::SetLoggerProvider(tf); - auto tf2 = shared_ptr(new TestProvider()); +TEST(Provider, MultipleLoggerProviders) +{ + auto tf = shared_ptr(new TestProvider()); + Provider::SetLoggerProvider(tf); + auto tf2 = shared_ptr(new TestProvider()); + Provider::SetLoggerProvider(tf2); - ASSERT_NE(Provider::GetLoggerProvider(), tf); - } - \ No newline at end of file + ASSERT_NE(Provider::GetLoggerProvider(), tf); +} diff --git a/api/test/logs/logger_test.cc b/api/test/logs/logger_test.cc index a3dc1946bc..54db01c2e9 100644 --- a/api/test/logs/logger_test.cc +++ b/api/test/logs/logger_test.cc @@ -35,9 +35,6 @@ class TestLogger : public Logger // returns the name of the logger string_view getName() noexcept override { return "My custom implementation"; } - // unstructured logging - void log(string_view name, Severity sev, string_view msg) noexcept override {} - // structured logging void log(string_view name, Severity sev, const KeyValueIterable &attributes) noexcept override {} // void log(string_view name, const KeyValueIterable &attributes) noexcept override {} From dd3b5d5d0ae98011ca35c4b1b81ae7d9b7c9132a Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Mon, 19 Oct 2020 19:07:25 -0400 Subject: [PATCH 03/17] Fix compile, combine severity, overload methods, Cleanup --- api/include/opentelemetry/logs/log_record.h | 120 +++++++++++++------- api/include/opentelemetry/logs/logger.h | 79 ++++++------- api/include/opentelemetry/logs/noop.h | 10 -- api/test/logs/log_record_test.cc | 18 +-- api/test/logs/logger_test.cc | 3 +- 5 files changed, 129 insertions(+), 101 deletions(-) diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h index 01ab512cb4..f30dc67157 100644 --- a/api/include/opentelemetry/logs/log_record.h +++ b/api/include/opentelemetry/logs/log_record.h @@ -16,15 +16,16 @@ OPENTELEMETRY_BEGIN_NAMESPACE namespace logs { -enum class Severity // using a class enum here won't allow enum values to be compared to integers - // only other Severity enums (need an explicit cast) +/* Note: using a class enum here won't allow enum values to be compared to integers, i.e. only other Severity enums (need an explicit cast) +*/ +enum class Severity { - NONE = 0, // default Severity; added, not part of Log Data Model + NONE = 0, // default Severity; added, not part of Log Data Model TRACE = 1, TRACE2 = 2, TRACE3 = 3, TRACE4 = 4, - DEBUG1 = 5, + DEBUG = 5, DEBUG2 = 6, DEBUG3 = 7, DEBUG4 = 8, @@ -54,59 +55,102 @@ enum class Severity // using a class enum here won't allow enum values to be co */ struct LogRecord { -public: // default fields that will be set if the user doesn't specify them - uint64_t timestamp; // uint64 nanoseconds since Unix epoch - trace::TraceId trace_id; // byte sequence - trace::SpanId span_id; // byte sequence - trace::TraceFlags trace_flag; // byte - nostd::string_view severity_text; // string; TODO: add a severity_num_to_text() method - Severity severity_number; // integer + uint64_t 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) // other fields - nostd::string_view name; // string - bool body; // currently using bool to represent whether it was defined, change once defined + 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 - //Default constructor where all the values are set to defaults - /* -------------------------------------------------------------------------- - TODO: Potentially add other constructors to take default arguments from the user - ----------------------------------------------------------------------------- */ - LogRecord() : resource(_nullKV), attributes(_nullKV) { - timestamp = 0; - //trace_id is by default 00000000000000000000000000000000 - //span_id is by default 0000000000000000 - //trace_flag is by default 0 - severity_text = "NONE"; - severity_number = Severity::NONE; - name = ""; - body = false; - //resource and attributes are set to _nullKV on line 85. - //_nullKV is defined as a private variable that allows resource and - //attributes to be instantiated using it as the default value + /* Default log record if user does not overwrite this. + * TODO: Potentially add other constructors to take default arguments from the user + * TODO: find way to correlate actual trace_id and span_id + * TODO: find way to get current timestamp + * TODO: find better data type for "body" field + **/ + LogRecord() : resource(_nullKV), attributes(_nullKV) + { + timestamp = 0; + // trace_id = current_trace_id; // TODO: correlate + // span_id = current_span_id; // TODO: correlate + // trace_flag = current_trace_flag; // TODO: correlate + name = ""; + body = ""; + resource = common::KeyValueIterableView>{{}}; + attributes = common::KeyValueIterableView>{{}}; } - - // convert a map into a KeyValueIterable for the resources - template ::value> * = nullptr> + /* for ease of use, user can use this function to convert a map into a KeyValueIterable for the resources */ + template ::value> * = nullptr> inline void SetResource(const T &_resource) { resource = common::KeyValueIterableView(_resource); } - // convert a map into a KeyValueIterable for the attributes - template ::value> * = nullptr> + /* for ease of use, user to use this function to convert a map into a KeyValueIterable for the attributes */ + template ::value> * = nullptr> inline void SetAttributes(const T &_attributes) { attributes = common::KeyValueIterableView(_attributes); } + private: - //Used as a default for the resource and attributes fields - opentelemetry::common::KeyValueIterableView> _nullKV = opentelemetry::common::KeyValueIterableView>{{}}; + // Move to SDK *(logger.cc): + Severity severity_number = Severity::NONE; + nostd::string_view severity_text = severityToString(severity_number); + inline nostd::string_view severityToString(Severity) + { + switch (this->severity_number) + { + case Severity::NONE: + return "NONE"; + case Severity::TRACE: + case Severity::TRACE2: + case Severity::TRACE3: + case Severity::TRACE4: + return "TRACE"; + case Severity::DEBUG: + case Severity::DEBUG2: + case Severity::DEBUG3: + case Severity::DEBUG4: + return "DEBUG"; + case Severity::INFO: + case Severity::INFO2: + case Severity::INFO3: + case Severity::INFO4: + return "INFO"; + case Severity::WARN: + case Severity::WARN2: + case Severity::WARN3: + case Severity::WARN4: + return "WARN"; + case Severity::ERROR: + case Severity::ERROR2: + case Severity::ERROR3: + case Severity::ERROR4: + return "ERROR"; + case Severity::FATAL: + case Severity::FATAL2: + case Severity::FATAL3: + case Severity::FATAL4: + return "FATAL"; + default: + return "INVALID_SEVERITY"; + } + } + + /* _nullKV is defined as a private variable that allows "resource" and + "attributes" fields to be instantiated using it as the default value */ + common::KeyValueIterableView> _nullKV = + common::KeyValueIterableView>{{}}; + }; } // namespace logs OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index 95b1dd9db1..520ec2f583 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -1,7 +1,8 @@ #pragma once + #include -#include #include +#include #include "opentelemetry/common/key_value_iterable.h" // fix after moving to "common" #include "opentelemetry/logs/log_record.h" @@ -15,15 +16,7 @@ namespace logs { /** * Handles log record creation. - * -// */ -// static const struct LogRecord defaultRecord = { -// timestamp : std::chrono::steady_clock::now().time_since_epoch(), -// trace_id : 0, -// span_id : 0, -// severity_number : 0 -// }; - + **/ class Logger { private: @@ -44,38 +37,39 @@ class Logger * @param record the log record (object type LogRecord) that is logged. * @param attributes the attributes, stored as a 2D list of key/value pairs, that are associated * with this log. - * @param some_obj the log object of user templated type that is logged. * @throws No exceptions under any circumstances. */ - /** Methods for an unstructured logging **/ - void log(nostd::string_view msg) noexcept { - log(Severity::NONE, msg); //Set severity to NONE as default then call the log method below + + /* 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 + */ + virtual void log(const LogRecord &record) noexcept = 0; + + /** Overloaded methods for unstructured logging **/ + void log(nostd::string_view msg) noexcept + { + log(Severity::NONE, msg); // Set severity to NONE as default then call the log method below } - void log(Severity sev, nostd::string_view msg) noexcept { - log(sev, msg, 0); //Set timestamp to 0 as default then call the log method below + void log(Severity sev, nostd::string_view msg) noexcept + { + log(sev, msg, 0); // Set timestamp to 0 as default then call the log method below } - void log(Severity sev, nostd::string_view msg, uint64_t time) noexcept { + void log(Severity sev, nostd::string_view msg, uint64_t time) noexcept + { LogRecord r; - r.name = msg; - r.severity_number = sev; - r.severity_text = "NONE"; //TODO, create function to map severity_number to text - r.timestamp = time; - log(r); //Call the log(LogRecord) log method - } - - /** Methods for structured logs **/ - // (name, sev, KeyValueIterable) overloads - virtual void log(nostd::string_view name, - Severity sev, - const common::KeyValueIterable &attributes) noexcept = 0; + r.body = msg; + r.severity = sev; + r.timestamp = time; + log(r); // converts to a LogRecord object, then calls log(LogRecord) + } - // (name, sev, KeyValueIterable) overloads - template ::value> * = nullptr> + /** Overloaded methods for structured logging**/ + template ::value> * = nullptr> void log(nostd::string_view name = "", Severity sev = Severity::NONE, const T &attributes = {}) noexcept @@ -83,17 +77,18 @@ class Logger log(name, sev, common::KeyValueIterableView(attributes)); } - /* 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 contain the trace_id, span_id, and current timestamp - * TODO: correlate timestamp, traceid and spanid at the minimum to Log Record - * - */ - virtual void log(const LogRecord &record) noexcept = 0; + void log(nostd::string_view name, + Severity sev, + const common::KeyValueIterable &attributes) noexcept { + LogRecord r; + r.name = name; + r.severity = sev; + r.attributes = attributes; - /*** Future additions*/ - /* templated method for objects / custom types (e.g. JSON, XML, custom classes, etc) (for later) - */ + log(r); // converts to a LogRecord object, then calls log(LogRecord) + } + + /** Future enhancement: templated method for objects / custom types (e.g. JSON, XML, custom classes, etc) **/ // template void log(T &some_obj) noexcept override; }; } // namespace logs diff --git a/api/include/opentelemetry/logs/noop.h b/api/include/opentelemetry/logs/noop.h index 1c9f56d3e4..5f35d8bbc4 100644 --- a/api/include/opentelemetry/logs/noop.h +++ b/api/include/opentelemetry/logs/noop.h @@ -28,17 +28,7 @@ class NoopLogger final : public Logger // returns the name of the logger nostd::string_view getName() noexcept override { return "NOOP Logger"; } - // structured logging - void log(nostd::string_view name, - Severity sev, - const common::KeyValueIterable &attributes) noexcept override - {} - // void log(nostd::string_view name, const common::KeyValueIterable &attributes) noexcept override - // {} void log(const LogRecord &record) noexcept override {} - - // templated method for objects / custom types (e.g. JSON, XML, custom classes, etc); may support - // later template void log(T &some_obj) noexcept override {} }; /** diff --git a/api/test/logs/log_record_test.cc b/api/test/logs/log_record_test.cc index f3774984dc..ed418dee1b 100644 --- a/api/test/logs/log_record_test.cc +++ b/api/test/logs/log_record_test.cc @@ -9,12 +9,12 @@ using opentelemetry::logs::Provider; // TODO TEST(LogRecord, TimestampStore) {} -TEST(LogRecord, TraceIdStore){} -TEST(LogRecord, SpanIdStore){} -TEST(LogRecord, TraceFlagsStore){} -TEST(LogRecord, SeverityTextStore){} -TEST(LogRecord, SeverityNumberStore){} -TEST(LogRecord, NameStore){} -TEST(LogRecord, BodyStore){} -TEST(LogRecord, ResourceStore){} -TEST(LogRecord, AttributesStore){} +TEST(LogRecord, TraceIdStore) {} +TEST(LogRecord, SpanIdStore) {} +TEST(LogRecord, TraceFlagsStore) {} +TEST(LogRecord, SeverityTextStore) {} +TEST(LogRecord, SeverityNumberStore) {} +TEST(LogRecord, NameStore) {} +TEST(LogRecord, BodyStore) {} +TEST(LogRecord, ResourceStore) {} +TEST(LogRecord, AttributesStore) {} diff --git a/api/test/logs/logger_test.cc b/api/test/logs/logger_test.cc index 54db01c2e9..885274a68b 100644 --- a/api/test/logs/logger_test.cc +++ b/api/test/logs/logger_test.cc @@ -28,6 +28,7 @@ TEST(Logger, GetNoopLoggerName) string_view name = logger->getName(); EXPECT_EQ("NOOP Logger", name); } +/* TODO: add more tests */ // Define a basic Logger class class TestLogger : public Logger @@ -36,8 +37,6 @@ class TestLogger : public Logger string_view getName() noexcept override { return "My custom implementation"; } // structured logging - void log(string_view name, Severity sev, const KeyValueIterable &attributes) noexcept override {} - // void log(string_view name, const KeyValueIterable &attributes) noexcept override {} void log(const LogRecord &record) noexcept override {} }; From 28821e6879c7cc45dbffa9de682b001f9ea8b602 Mon Sep 17 00:00:00 2001 From: Seufert Date: Tue, 20 Oct 2020 08:20:02 -0700 Subject: [PATCH 04/17] Changed timestamp to SystemTimestamp from int --- api/include/opentelemetry/logs/log_record.h | 8 +- api/include/opentelemetry/logs/logger.h | 5 +- api/test/logs/log_record_test.cc | 115 +++++++++++++++++--- 3 files changed, 107 insertions(+), 21 deletions(-) diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h index f30dc67157..a2968cf715 100644 --- a/api/include/opentelemetry/logs/log_record.h +++ b/api/include/opentelemetry/logs/log_record.h @@ -25,7 +25,7 @@ enum class Severity TRACE2 = 2, TRACE3 = 3, TRACE4 = 4, - DEBUG = 5, + DEBUG1 = 5, DEBUG2 = 6, DEBUG3 = 7, DEBUG4 = 8, @@ -56,7 +56,7 @@ enum class Severity struct LogRecord { // default fields that will be set if the user doesn't specify them - uint64_t timestamp; // uint64 nanoseconds since Unix epoch + 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 @@ -76,7 +76,7 @@ struct LogRecord **/ LogRecord() : resource(_nullKV), attributes(_nullKV) { - timestamp = 0; + timestamp = core::SystemTimestamp(std::chrono::system_clock::now()); // trace_id = current_trace_id; // TODO: correlate // span_id = current_span_id; // TODO: correlate // trace_flag = current_trace_flag; // TODO: correlate @@ -116,7 +116,7 @@ struct LogRecord case Severity::TRACE3: case Severity::TRACE4: return "TRACE"; - case Severity::DEBUG: + case Severity::DEBUG1: case Severity::DEBUG2: case Severity::DEBUG3: case Severity::DEBUG4: diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index 520ec2f583..59a9e5ea2d 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -55,10 +55,11 @@ class Logger void log(Severity sev, nostd::string_view msg) noexcept { - log(sev, msg, 0); // Set timestamp to 0 as default then call the log method below + // Set timestamp to now as default then call the log method below + log(sev, msg, core::SystemTimestamp(std::chrono::system_clock::now())); } - void log(Severity sev, nostd::string_view msg, uint64_t time) noexcept + void log(Severity sev, nostd::string_view msg, core::SystemTimestamp time) noexcept { LogRecord r; r.body = msg; diff --git a/api/test/logs/log_record_test.cc b/api/test/logs/log_record_test.cc index ed418dee1b..8c7bffd8ce 100644 --- a/api/test/logs/log_record_test.cc +++ b/api/test/logs/log_record_test.cc @@ -1,20 +1,105 @@ #include "opentelemetry/logs/provider.h" #include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/logs/log_record.h" +#include "opentelemetry/trace/trace_id.h" +#include "opentelemetry/trace/span_id.h" +#include "opentelemetry/common/key_value_iterable_view.h" + +#include #include -using opentelemetry::logs::Logger; -using opentelemetry::logs::LoggerProvider; -using opentelemetry::logs::Provider; - -// TODO -TEST(LogRecord, TimestampStore) {} -TEST(LogRecord, TraceIdStore) {} -TEST(LogRecord, SpanIdStore) {} -TEST(LogRecord, TraceFlagsStore) {} -TEST(LogRecord, SeverityTextStore) {} -TEST(LogRecord, SeverityNumberStore) {} -TEST(LogRecord, NameStore) {} -TEST(LogRecord, BodyStore) {} -TEST(LogRecord, ResourceStore) {} -TEST(LogRecord, AttributesStore) {} +using opentelemetry::logs::LogRecord; + +TEST(LogRecord, TimestampStore) { + //Create a record at the current time + LogRecord r; + + //Do a large computation + for(int i = 0; i < 1000; i++){} + + //Create a second record at the current time + LogRecord r2; + + //By default, the timestamp should be set to now() + EXPECT_NE(r.timestamp, r2.timestamp); +} + +TEST(LogRecord, TraceIdStore) +{ + LogRecord r; + constexpr uint8_t input[] = {1, 2, 3, 4, 5, 6, 7, 8, 8, 7, 6, 5, 4, 3, 2, 1}; + opentelemetry::trace::TraceId traceId(input); + + //Make sure the traceId was written correctly + EXPECT_TRUE(traceId.IsValid()); + + //Convert the traceId back to hex + char output[32]; + traceId.ToLowerBase16(output); + EXPECT_EQ("01020304050607080807060504030201", std::string(output, sizeof(output))); +} + +TEST(LogRecord, SpanIdStore) +{ + LogRecord r; + constexpr uint8_t input[] = {1, 2, 3, 4, 5, 6, 7, 8}; + opentelemetry::trace::SpanId spanId(input); + + //Make sure the spanId was written correctly + EXPECT_TRUE(spanId.IsValid()); + + //Convert the spanId back to hex + char output[16]; + spanId.ToLowerBase16(output); + EXPECT_EQ("0102030405060708", std::string(output, sizeof(output))); +} + +TEST(LogRecord, TraceFlagsStore) +{ + LogRecord r; + + //By default, flag should be 0 + EXPECT_EQ(0, r.trace_flag.flags()); + + //Set the traceflag to 1 + opentelemetry::trace::TraceFlags flags{opentelemetry::trace::TraceFlags::kIsSampled}; + r.trace_flag = flags; + EXPECT_EQ(1, r.trace_flag.flags()); +} + +TEST(LogRecord, SeverityStore) +{ + LogRecord r; + r.severity = opentelemetry::logs::Severity::ERROR; + EXPECT_EQ(r.severity, opentelemetry::logs::Severity::ERROR); +} + +TEST(LogRecord, NameStore) +{ + LogRecord r; + r.name = "My Name"; + EXPECT_EQ(r.name, "My Name"); +} + +//Implement once Body field is established +//TEST(LogRecord, BodyStore) +//{ +// +//} + +/*TEST(LogRecord, ResourceStore) +{ + using M = std::map; + M m1 = {{"abc", "123"}, {"xyz", "456"}}; + + LogRecord r; + r.SetResource(m1); + + //EXPECT_EQ(common::KeyValueIterableView(m1), r.resource); +} + +TEST(LogRecord, AttributesStore) +{ + +}*/ From 5b8debbbfe0e73596c9a04f27511a75a32bbdb19 Mon Sep 17 00:00:00 2001 From: Seufert Date: Tue, 20 Oct 2020 11:14:30 -0700 Subject: [PATCH 05/17] Renamed enum to follow google conventions --- api/include/opentelemetry/logs/log_record.h | 98 ++++++--------------- api/include/opentelemetry/logs/logger.h | 11 ++- api/test/logs/log_record_test.cc | 4 +- 3 files changed, 37 insertions(+), 76 deletions(-) diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h index a2968cf715..aee0bbec99 100644 --- a/api/include/opentelemetry/logs/log_record.h +++ b/api/include/opentelemetry/logs/log_record.h @@ -18,33 +18,33 @@ namespace logs /* Note: using a class enum here won't allow enum values to be compared to integers, i.e. only other Severity enums (need an explicit cast) */ -enum class Severity -{ - NONE = 0, // default Severity; added, not part of Log Data Model - TRACE = 1, - TRACE2 = 2, - TRACE3 = 3, - TRACE4 = 4, - DEBUG1 = 5, - DEBUG2 = 6, - DEBUG3 = 7, - DEBUG4 = 8, - INFO = 9, - INFO2 = 10, - INFO3 = 11, - INFO4 = 12, - WARN = 13, - WARN2 = 14, - WARN3 = 15, - WARN4 = 16, - ERROR = 17, - ERROR2 = 18, - ERROR3 = 19, - ERROR4 = 20, - FATAL = 21, - FATAL2 = 22, - FATAL3 = 23, - FATAL4 = 24 +enum class Severity : uint8_t +{ //Follows this standard: https://google.github.io/styleguide/cppguide.html#Enumerator_Names + kNone = 0, // default Severity; added, not part of Log Data Model + kTrace = 1, + KTrace2 = 2, + kTrace3 = 3, + kTrace4 = 4, + kDebug = 5, + kDebug2 = 6, + kDebug3 = 7, + kDebug4 = 8, + kInfo = 9, + kInfo2 = 10, + kInfo3 = 11, + kInfo4 = 12, + kWarn = 13, + kWarn2 = 14, + kWarn3 = 15, + kWarn4 = 16, + kError = 17, + kError2 = 18, + kError3 = 19, + kError4 = 20, + kFatal = 21, + kFatal2 = 22, + kFatal3 = 23, + kFatal4 = 24 }; /** @@ -102,50 +102,6 @@ struct LogRecord private: - // Move to SDK *(logger.cc): - Severity severity_number = Severity::NONE; - nostd::string_view severity_text = severityToString(severity_number); - inline nostd::string_view severityToString(Severity) - { - switch (this->severity_number) - { - case Severity::NONE: - return "NONE"; - case Severity::TRACE: - case Severity::TRACE2: - case Severity::TRACE3: - case Severity::TRACE4: - return "TRACE"; - case Severity::DEBUG1: - case Severity::DEBUG2: - case Severity::DEBUG3: - case Severity::DEBUG4: - return "DEBUG"; - case Severity::INFO: - case Severity::INFO2: - case Severity::INFO3: - case Severity::INFO4: - return "INFO"; - case Severity::WARN: - case Severity::WARN2: - case Severity::WARN3: - case Severity::WARN4: - return "WARN"; - case Severity::ERROR: - case Severity::ERROR2: - case Severity::ERROR3: - case Severity::ERROR4: - return "ERROR"; - case Severity::FATAL: - case Severity::FATAL2: - case Severity::FATAL3: - case Severity::FATAL4: - return "FATAL"; - default: - return "INVALID_SEVERITY"; - } - } - /* _nullKV is defined as a private variable that allows "resource" and "attributes" fields to be instantiated using it as the default value */ common::KeyValueIterableView> _nullKV = diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index 59a9e5ea2d..4b1c3bf475 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -39,7 +39,9 @@ class Logger * with this log. * @throws No exceptions under any circumstances. */ - + + //Potential bool isenabled(severity): Break into seperate future PRs + //Log issue about macros (disabling log events below certain level) /* 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 @@ -47,10 +49,13 @@ class Logger */ virtual void log(const LogRecord &record) noexcept = 0; +//First, finalize core, +//Afterwards, come back and add overloads + /** Overloaded methods for unstructured logging **/ void log(nostd::string_view msg) noexcept { - log(Severity::NONE, msg); // Set severity to NONE as default then call the log method below + log(Severity::kNone, msg); // Set severity to NONE as default then call the log method below } void log(Severity sev, nostd::string_view msg) noexcept @@ -72,7 +77,7 @@ class Logger /** Overloaded methods for structured logging**/ template ::value> * = nullptr> void log(nostd::string_view name = "", - Severity sev = Severity::NONE, + Severity sev = Severity::kNone, const T &attributes = {}) noexcept { log(name, sev, common::KeyValueIterableView(attributes)); diff --git a/api/test/logs/log_record_test.cc b/api/test/logs/log_record_test.cc index 8c7bffd8ce..cbcc858afb 100644 --- a/api/test/logs/log_record_test.cc +++ b/api/test/logs/log_record_test.cc @@ -71,8 +71,8 @@ TEST(LogRecord, TraceFlagsStore) TEST(LogRecord, SeverityStore) { LogRecord r; - r.severity = opentelemetry::logs::Severity::ERROR; - EXPECT_EQ(r.severity, opentelemetry::logs::Severity::ERROR); + r.severity = opentelemetry::logs::Severity::kError2; + EXPECT_EQ(r.severity, opentelemetry::logs::Severity::kError2); } TEST(LogRecord, NameStore) From b1169821579269354f69c4e954c69663a7319650 Mon Sep 17 00:00:00 2001 From: Seufert Date: Tue, 20 Oct 2020 15:00:55 -0700 Subject: [PATCH 06/17] Made nullKV static and fixed formatting --- api/include/opentelemetry/logs/log_record.h | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h index aee0bbec99..accb1127fc 100644 --- a/api/include/opentelemetry/logs/log_record.h +++ b/api/include/opentelemetry/logs/log_record.h @@ -22,7 +22,7 @@ enum class Severity : uint8_t { //Follows this standard: https://google.github.io/styleguide/cppguide.html#Enumerator_Names kNone = 0, // default Severity; added, not part of Log Data Model kTrace = 1, - KTrace2 = 2, + kTrace2 = 2, kTrace3 = 3, kTrace4 = 4, kDebug = 5, @@ -47,6 +47,12 @@ enum class Severity : uint8_t kFatal4 = 24 }; +/* _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. @@ -81,10 +87,7 @@ struct LogRecord // span_id = current_span_id; // TODO: correlate // trace_flag = current_trace_flag; // TODO: correlate name = ""; - body = ""; - resource = common::KeyValueIterableView>{{}}; - attributes = common::KeyValueIterableView>{{}}; - } + } /* for ease of use, user can use this function to convert a map into a KeyValueIterable for the resources */ template ::value> * = nullptr> @@ -99,14 +102,6 @@ struct LogRecord { attributes = common::KeyValueIterableView(_attributes); } - - -private: - /* _nullKV is defined as a private variable that allows "resource" and - "attributes" fields to be instantiated using it as the default value */ - common::KeyValueIterableView> _nullKV = - common::KeyValueIterableView>{{}}; - }; } // namespace logs OPENTELEMETRY_END_NAMESPACE From 758752cecf24d6c15fc1672d43a7cc82b12833c1 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Wed, 21 Oct 2020 16:55:23 -0400 Subject: [PATCH 07/17] CI format --- api/include/opentelemetry/logs/log_record.h | 61 +++++++------- api/include/opentelemetry/logs/logger.h | 37 ++++---- api/test/logs/log_record_test.cc | 93 +++++++++++---------- 3 files changed, 101 insertions(+), 90 deletions(-) diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h index accb1127fc..8fbaa0cb16 100644 --- a/api/include/opentelemetry/logs/log_record.h +++ b/api/include/opentelemetry/logs/log_record.h @@ -16,11 +16,12 @@ OPENTELEMETRY_BEGIN_NAMESPACE namespace logs { -/* Note: using a class enum here won't allow enum values to be compared to integers, i.e. only other Severity enums (need an explicit cast) -*/ +/* Note: using a class enum here won't allow enum values to be compared to integers, i.e. only other + * Severity enums (need an explicit cast) + */ enum class Severity : uint8_t -{ //Follows this standard: https://google.github.io/styleguide/cppguide.html#Enumerator_Names - kNone = 0, // default Severity; added, not part of Log Data Model +{ // Follows this standard: https://google.github.io/styleguide/cppguide.html#Enumerator_Names + kNone = 0, // default Severity; added, not part of Log Data Model kTrace = 1, kTrace2 = 2, kTrace3 = 3, @@ -50,8 +51,7 @@ enum class Severity : uint8_t /* _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>{{}}; - + common::KeyValueIterableView>{{}}; /** * A default Event object to be passed in log statements, @@ -62,42 +62,47 @@ static common::KeyValueIterableView::value> * = nullptr> + /* for ease of use, user can use this function to convert a map into a KeyValueIterable for the + * resources */ + template ::value> * = nullptr> inline void SetResource(const T &_resource) { resource = common::KeyValueIterableView(_resource); } - /* for ease of use, user to use this function to convert a map into a KeyValueIterable for the attributes */ - template ::value> * = nullptr> + /* for ease of use, user to use this function to convert a map into a KeyValueIterable for the + * attributes */ + template ::value> * = nullptr> inline void SetAttributes(const T &_attributes) { attributes = common::KeyValueIterableView(_attributes); diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index 4b1c3bf475..77f6e416e8 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -39,9 +39,9 @@ class Logger * with this log. * @throws No exceptions under any circumstances. */ - - //Potential bool isenabled(severity): Break into seperate future PRs - //Log issue about macros (disabling log events below certain level) + + // Potential bool isenabled(severity): Break into seperate future PRs + // Log issue about macros (disabling log events below certain level) /* 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 @@ -49,8 +49,8 @@ class Logger */ virtual void log(const LogRecord &record) noexcept = 0; -//First, finalize core, -//Afterwards, come back and add overloads + // First, finalize core, + // Afterwards, come back and add overloads /** Overloaded methods for unstructured logging **/ void log(nostd::string_view msg) noexcept @@ -67,15 +67,16 @@ class Logger void log(Severity sev, nostd::string_view msg, core::SystemTimestamp time) noexcept { LogRecord r; - r.body = msg; - r.severity = sev; - r.timestamp = time; + r.body = msg; + r.severity = sev; + r.timestamp = time; - log(r); // converts to a LogRecord object, then calls log(LogRecord) + log(r); // converts to a LogRecord object, then calls log(LogRecord) } /** Overloaded methods for structured logging**/ - template ::value> * = nullptr> + template ::value> * = nullptr> void log(nostd::string_view name = "", Severity sev = Severity::kNone, const T &attributes = {}) noexcept @@ -84,17 +85,19 @@ class Logger } void log(nostd::string_view name, - Severity sev, - const common::KeyValueIterable &attributes) noexcept { + Severity sev, + const common::KeyValueIterable &attributes) noexcept + { LogRecord r; - r.name = name; - r.severity = sev; - r.attributes = attributes; + r.name = name; + r.severity = sev; + r.attributes = attributes; - log(r); // converts to a LogRecord object, then calls log(LogRecord) + log(r); // converts to a LogRecord object, then calls log(LogRecord) } - /** Future enhancement: templated method for objects / custom types (e.g. JSON, XML, custom classes, etc) **/ + /** Future enhancement: templated method for objects / custom types (e.g. JSON, XML, custom + * classes, etc) **/ // template void log(T &some_obj) noexcept override; }; } // namespace logs diff --git a/api/test/logs/log_record_test.cc b/api/test/logs/log_record_test.cc index cbcc858afb..070dd2da33 100644 --- a/api/test/logs/log_record_test.cc +++ b/api/test/logs/log_record_test.cc @@ -1,9 +1,9 @@ +#include "opentelemetry/logs/log_record.h" +#include "opentelemetry/common/key_value_iterable_view.h" #include "opentelemetry/logs/provider.h" #include "opentelemetry/nostd/shared_ptr.h" -#include "opentelemetry/logs/log_record.h" -#include "opentelemetry/trace/trace_id.h" #include "opentelemetry/trace/span_id.h" -#include "opentelemetry/common/key_value_iterable_view.h" +#include "opentelemetry/trace/trace_id.h" #include @@ -11,79 +11,82 @@ using opentelemetry::logs::LogRecord; -TEST(LogRecord, TimestampStore) { - //Create a record at the current time - LogRecord r; +TEST(LogRecord, TimestampStore) +{ + // Create a record at the current time + LogRecord r; - //Do a large computation - for(int i = 0; i < 1000; i++){} + // Do a large computation + for (int i = 0; i < 1000; i++) + { + } - //Create a second record at the current time - LogRecord r2; + // Create a second record at the current time + LogRecord r2; - //By default, the timestamp should be set to now() - EXPECT_NE(r.timestamp, r2.timestamp); + // By default, the timestamp should be set to now() + EXPECT_NE(r.timestamp, r2.timestamp); } TEST(LogRecord, TraceIdStore) { - LogRecord r; - constexpr uint8_t input[] = {1, 2, 3, 4, 5, 6, 7, 8, 8, 7, 6, 5, 4, 3, 2, 1}; - opentelemetry::trace::TraceId traceId(input); + LogRecord r; + constexpr uint8_t input[] = {1, 2, 3, 4, 5, 6, 7, 8, 8, 7, 6, 5, 4, 3, 2, 1}; + opentelemetry::trace::TraceId traceId(input); - //Make sure the traceId was written correctly - EXPECT_TRUE(traceId.IsValid()); + // Make sure the traceId was written correctly + EXPECT_TRUE(traceId.IsValid()); - //Convert the traceId back to hex - char output[32]; - traceId.ToLowerBase16(output); - EXPECT_EQ("01020304050607080807060504030201", std::string(output, sizeof(output))); + // Convert the traceId back to hex + char output[32]; + traceId.ToLowerBase16(output); + EXPECT_EQ("01020304050607080807060504030201", std::string(output, sizeof(output))); } TEST(LogRecord, SpanIdStore) { - LogRecord r; - constexpr uint8_t input[] = {1, 2, 3, 4, 5, 6, 7, 8}; - opentelemetry::trace::SpanId spanId(input); + LogRecord r; + constexpr uint8_t input[] = {1, 2, 3, 4, 5, 6, 7, 8}; + opentelemetry::trace::SpanId spanId(input); - //Make sure the spanId was written correctly - EXPECT_TRUE(spanId.IsValid()); + // Make sure the spanId was written correctly + EXPECT_TRUE(spanId.IsValid()); - //Convert the spanId back to hex - char output[16]; - spanId.ToLowerBase16(output); - EXPECT_EQ("0102030405060708", std::string(output, sizeof(output))); + // Convert the spanId back to hex + char output[16]; + spanId.ToLowerBase16(output); + EXPECT_EQ("0102030405060708", std::string(output, sizeof(output))); } TEST(LogRecord, TraceFlagsStore) { - LogRecord r; + LogRecord r; - //By default, flag should be 0 - EXPECT_EQ(0, r.trace_flag.flags()); + // By default, flag should be 0 + EXPECT_EQ(0, r.trace_flag.flags()); - //Set the traceflag to 1 - opentelemetry::trace::TraceFlags flags{opentelemetry::trace::TraceFlags::kIsSampled}; - r.trace_flag = flags; - EXPECT_EQ(1, r.trace_flag.flags()); + // Set the traceflag to 1 + opentelemetry::trace::TraceFlags flags{opentelemetry::trace::TraceFlags::kIsSampled}; + r.trace_flag = flags; + EXPECT_EQ(1, r.trace_flag.flags()); } TEST(LogRecord, SeverityStore) { - LogRecord r; - r.severity = opentelemetry::logs::Severity::kError2; - EXPECT_EQ(r.severity, opentelemetry::logs::Severity::kError2); + LogRecord r; + r.severity = opentelemetry::logs::Severity::kError2; + EXPECT_EQ(r.severity, opentelemetry::logs::Severity::kError2); } TEST(LogRecord, NameStore) { - LogRecord r; - r.name = "My Name"; - EXPECT_EQ(r.name, "My Name"); + LogRecord r; + r.name = "My Name"; + EXPECT_EQ(r.name, "My Name"); } -//Implement once Body field is established -//TEST(LogRecord, BodyStore) +// Implement once Body field is established +// TEST(LogRecord, BodyStore) //{ // //} From c144de78d8701f09c3989e9e2a51329571911394 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Mon, 26 Oct 2020 13:36:41 -0400 Subject: [PATCH 08/17] Add license and remove comments --- api/include/opentelemetry/logs/log_record.h | 23 +- sdk/test/logs/batch_log_processor_test.cc | 287 ++++++++++++++++++++ 2 files changed, 303 insertions(+), 7 deletions(-) create mode 100644 sdk/test/logs/batch_log_processor_test.cc diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h index 8fbaa0cb16..7cd8d7490e 100644 --- a/api/include/opentelemetry/logs/log_record.h +++ b/api/include/opentelemetry/logs/log_record.h @@ -1,3 +1,17 @@ +# Copyright 2019, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + #pragma once #include @@ -76,17 +90,12 @@ struct LogRecord common::KeyValueIterable &attributes; // key/value pair list /* Default log record if user does not overwrite this. - * TODO: Potentially add other constructors to take default arguments from the user - * TODO: find way to correlate actual trace_id and span_id - * TODO: find way to get current timestamp - * TODO: find better data type for "body" field + * TODO: find better data type for "body" field + * Future enhancement: Potentially add other constructors to take default arguments from the user **/ LogRecord() : resource(_nullKV), attributes(_nullKV) { timestamp = core::SystemTimestamp(std::chrono::system_clock::now()); - // trace_id = current_trace_id; // TODO: correlate - // span_id = current_span_id; // TODO: correlate - // trace_flag = current_trace_flag; // TODO: correlate name = ""; } diff --git a/sdk/test/logs/batch_log_processor_test.cc b/sdk/test/logs/batch_log_processor_test.cc new file mode 100644 index 0000000000..18525fe16b --- /dev/null +++ b/sdk/test/logs/batch_log_processor_test.cc @@ -0,0 +1,287 @@ +#include "opentelemetry/sdk/logs/batch_log_processor.h" + +#include +#include +#include + +OPENTELEMETRY_BEGIN_NAMESPACE + +/** + * Returns a mock log exporter meant exclusively for testing only + */ +class MockLogExporter final : public sdk::logs::LogExporter +{ +public: + MockLogExporter( + std::shared_ptr>> logs_received, + std::shared_ptr> is_shutdown, + std::shared_ptr> is_export_completed, + const std::chrono::milliseconds export_delay = std::chrono::milliseconds(0)) noexcept + : logs_received_(logs_received), + is_shutdown_(is_shutdown), + is_export_completed_(is_export_completed), + export_delay_(export_delay) + {} + + sdk::logs::ExportResult Export( + const nostd::log> &recordables) noexcept override + { + *is_export_completed_ = false; + + std::this_thread::sleep_for(export_delay_); + + for (auto &recordable : recordables) + { + auto log = std::unique_ptr( + static_cast(recordable.release())); + + if (log != nullptr) + { + logs_received_->push_back(std::move(log)); + } + } + + *is_export_completed_ = true; + return sdk::logs::ExportResult::kSuccess; + } + + void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override + { + *is_shutdown_ = true; + } + + bool IsExportCompleted() { return is_export_completed_->load(); } + +private: + std::shared_ptr>> logs_received_; + std::shared_ptr> is_shutdown_; + std::shared_ptr> is_export_completed_; + // Meant exclusively to test force flush timeout + const std::chrono::milliseconds export_delay_; +}; + +/** + * Fixture Class + */ +class BatchLogProcessorTestPeer : public testing::Test +{ +public: + std::shared_ptr GetMockProcessor( + std::shared_ptr>> logs_received, + std::shared_ptr> is_shutdown, + std::shared_ptr> is_export_completed = + std::shared_ptr>(new std::atomic(false)), + const std::chrono::milliseconds export_delay = std::chrono::milliseconds(0), + const std::chrono::milliseconds schedule_delay_millis = std::chrono::milliseconds(5000), + const size_t max_queue_size = 2048, + const size_t max_export_batch_size = 512) + { + return std::shared_ptr(new sdk::logs::BatchLogProcessor( + GetMockExporter(logs_received, is_shutdown, is_export_completed, export_delay), + max_queue_size, schedule_delay_millis, max_export_batch_size)); + } + + std::unique_ptr>> GetTestSpans( + std::shared_ptr processor, + const int num_logs) + { + std::unique_ptr>> test_logs( + new std::vector>); + + for (int i = 0; i < num_logs; ++i) + { + test_logs->push_back(processor->MakeRecordable()); + static_cast(test_logs->at(i).get()) + ->SetName("Span " + std::to_string(i)); + } + + return test_logs; + } + +private: + std::unique_ptr GetMockExporter( + std::shared_ptr>> logs_received, + std::shared_ptr> is_shutdown, + std::shared_ptr> is_export_completed, + const std::chrono::milliseconds export_delay = std::chrono::milliseconds(0)) + { + return std::unique_ptr( + new MockLogExporter(logs_received, is_shutdown, is_export_completed, export_delay)); + } +}; + +/* ################################## TESTS ############################################ */ + +TEST_F(BatchLogProcessorTestPeer, TestShutdown) +{ + std::shared_ptr> is_shutdown(new std::atomic(false)); + std::shared_ptr>> logs_received( + new std::vector>); + + auto batch_processor = GetMockProcessor(logs_received, is_shutdown); + const int num_logs = 3; + + auto test_logs = GetTestSpans(batch_processor, num_logs); + + for (int i = 0; i < num_logs; ++i) + { + batch_processor->OnEnd(std::move(test_logs->at(i))); + } + + batch_processor->Shutdown(); + + EXPECT_EQ(num_logs, logs_received->size()); + for (int i = 0; i < num_logs; ++i) + { + EXPECT_EQ("Span " + std::to_string(i), logs_received->at(i)->GetName()); + } + + EXPECT_TRUE(is_shutdown->load()); +} + +TEST_F(BatchLogProcessorTestPeer, TestForceFlush) +{ + std::shared_ptr> is_shutdown(new std::atomic(false)); + std::shared_ptr>> logs_received( + new std::vector>); + + auto batch_processor = GetMockProcessor(logs_received, is_shutdown); + const int num_logs = 2048; + + auto test_logs = GetTestSpans(batch_processor, num_logs); + + for (int i = 0; i < num_logs; ++i) + { + batch_processor->OnEnd(std::move(test_logs->at(i))); + } + + // Give some time to export + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + + batch_processor->ForceFlush(); + + EXPECT_EQ(num_logs, logs_received->size()); + for (int i = 0; i < num_logs; ++i) + { + EXPECT_EQ("Span " + std::to_string(i), logs_received->at(i)->GetName()); + } + + // Create some more logs to make sure that the processor still works + auto more_test_logs = GetTestSpans(batch_processor, num_logs); + for (int i = 0; i < num_logs; ++i) + { + batch_processor->OnEnd(std::move(more_test_logs->at(i))); + } + + // Give some time to export the logs + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + + batch_processor->ForceFlush(); + + EXPECT_EQ(num_logs * 2, logs_received->size()); + for (int i = 0; i < num_logs; ++i) + { + EXPECT_EQ("Span " + std::to_string(i % num_logs), + logs_received->at(num_logs + i)->GetName()); + } +} + +TEST_F(BatchLogProcessorTestPeer, TestManySpansLoss) +{ + /* Test that when exporting more than max_queue_size logs, some are most likely lost*/ + + std::shared_ptr> is_shutdown(new std::atomic(false)); + std::shared_ptr>> logs_received( + new std::vector>); + + const int max_queue_size = 4096; + + auto batch_processor = GetMockProcessor(logs_received, is_shutdown); + + auto test_logs = GetTestSpans(batch_processor, max_queue_size); + + for (int i = 0; i < max_queue_size; ++i) + { + batch_processor->OnEnd(std::move(test_logs->at(i))); + } + + // Give some time to export the logs + std::this_thread::sleep_for(std::chrono::milliseconds(700)); + + batch_processor->ForceFlush(); + + // Span should be exported by now + EXPECT_GE(max_queue_size, logs_received->size()); +} + +TEST_F(BatchLogProcessorTestPeer, TestManySpansLossLess) +{ + /* Test that no logs are lost when sending max_queue_size logs */ + + std::shared_ptr> is_shutdown(new std::atomic(false)); + std::shared_ptr>> logs_received( + new std::vector>); + + const int num_logs = 2048; + + auto batch_processor = GetMockProcessor(logs_received, is_shutdown); + + auto test_logs = GetTestSpans(batch_processor, num_logs); + + for (int i = 0; i < num_logs; ++i) + { + batch_processor->OnEnd(std::move(test_logs->at(i))); + } + + // Give some time to export the logs + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + + batch_processor->ForceFlush(); + + EXPECT_EQ(num_logs, logs_received->size()); + for (int i = 0; i < num_logs; ++i) + { + EXPECT_EQ("Span " + std::to_string(i), logs_received->at(i)->GetName()); + } +} + +TEST_F(BatchLogProcessorTestPeer, TestScheduleDelayMillis) +{ + /* Test that max_export_batch_size logs are exported every schedule_delay_millis + seconds */ + + std::shared_ptr> is_shutdown(new std::atomic(false)); + std::shared_ptr> is_export_completed(new std::atomic(false)); + std::shared_ptr>> logs_received( + new std::vector>); + + const std::chrono::milliseconds export_delay(0); + const std::chrono::milliseconds schedule_delay_millis(2000); + const size_t max_export_batch_size = 512; + + auto batch_processor = GetMockProcessor(logs_received, is_shutdown, is_export_completed, + export_delay, schedule_delay_millis); + + auto test_logs = GetTestSpans(batch_processor, max_export_batch_size); + + for (size_t i = 0; i < max_export_batch_size; ++i) + { + batch_processor->OnEnd(std::move(test_logs->at(i))); + } + + // Sleep for schedule_delay_millis milliseconds + std::this_thread::sleep_for(schedule_delay_millis); + + // small delay to give time to export + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + + // Spans should be exported by now + EXPECT_TRUE(is_export_completed->load()); + EXPECT_EQ(max_export_batch_size, logs_received->size()); + for (size_t i = 0; i < max_export_batch_size; ++i) + { + EXPECT_EQ("Span " + std::to_string(i), logs_received->at(i)->GetName()); + } +} + +OPENTELEMETRY_END_NAMESPACE From e0296b92f321c654ac487652baaa94e9ca0d81de Mon Sep 17 00:00:00 2001 From: Seufert Date: Thu, 22 Oct 2020 12:10:48 -0700 Subject: [PATCH 09/17] Removed unit test to fix compilation issue --- api/include/opentelemetry/logs/logger.h | 3 - api/test/logs/BUILD | 12 --- api/test/logs/CMakeLists.txt | 2 +- api/test/logs/log_record_test.cc | 108 ------------------------ 4 files changed, 1 insertion(+), 124 deletions(-) delete mode 100644 api/test/logs/log_record_test.cc diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index 77f6e416e8..b68f4e63fe 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -19,9 +19,6 @@ namespace logs **/ class Logger { -private: - std::vector records; - public: virtual ~Logger() = default; diff --git a/api/test/logs/BUILD b/api/test/logs/BUILD index 4fb5c351e7..899400b2e8 100644 --- a/api/test/logs/BUILD +++ b/api/test/logs/BUILD @@ -21,15 +21,3 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) - -cc_test( - name = "log_record_test", - srcs = [ - "log_record_test.cc", - ], - linkstatic = 1, - deps = [ - "//api", - "@com_google_googletest//:gtest_main", - ], -) diff --git a/api/test/logs/CMakeLists.txt b/api/test/logs/CMakeLists.txt index 872daf2d53..737edb893c 100644 --- a/api/test/logs/CMakeLists.txt +++ b/api/test/logs/CMakeLists.txt @@ -1,4 +1,4 @@ -foreach(testname logger_provider_test logger_test log_record_test) +foreach(testname logger_provider_test logger_test) add_executable(${testname} "${testname}.cc") target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} opentelemetry_api) diff --git a/api/test/logs/log_record_test.cc b/api/test/logs/log_record_test.cc deleted file mode 100644 index 070dd2da33..0000000000 --- a/api/test/logs/log_record_test.cc +++ /dev/null @@ -1,108 +0,0 @@ -#include "opentelemetry/logs/log_record.h" -#include "opentelemetry/common/key_value_iterable_view.h" -#include "opentelemetry/logs/provider.h" -#include "opentelemetry/nostd/shared_ptr.h" -#include "opentelemetry/trace/span_id.h" -#include "opentelemetry/trace/trace_id.h" - -#include - -#include - -using opentelemetry::logs::LogRecord; - -TEST(LogRecord, TimestampStore) -{ - // Create a record at the current time - LogRecord r; - - // Do a large computation - for (int i = 0; i < 1000; i++) - { - } - - // Create a second record at the current time - LogRecord r2; - - // By default, the timestamp should be set to now() - EXPECT_NE(r.timestamp, r2.timestamp); -} - -TEST(LogRecord, TraceIdStore) -{ - LogRecord r; - constexpr uint8_t input[] = {1, 2, 3, 4, 5, 6, 7, 8, 8, 7, 6, 5, 4, 3, 2, 1}; - opentelemetry::trace::TraceId traceId(input); - - // Make sure the traceId was written correctly - EXPECT_TRUE(traceId.IsValid()); - - // Convert the traceId back to hex - char output[32]; - traceId.ToLowerBase16(output); - EXPECT_EQ("01020304050607080807060504030201", std::string(output, sizeof(output))); -} - -TEST(LogRecord, SpanIdStore) -{ - LogRecord r; - constexpr uint8_t input[] = {1, 2, 3, 4, 5, 6, 7, 8}; - opentelemetry::trace::SpanId spanId(input); - - // Make sure the spanId was written correctly - EXPECT_TRUE(spanId.IsValid()); - - // Convert the spanId back to hex - char output[16]; - spanId.ToLowerBase16(output); - EXPECT_EQ("0102030405060708", std::string(output, sizeof(output))); -} - -TEST(LogRecord, TraceFlagsStore) -{ - LogRecord r; - - // By default, flag should be 0 - EXPECT_EQ(0, r.trace_flag.flags()); - - // Set the traceflag to 1 - opentelemetry::trace::TraceFlags flags{opentelemetry::trace::TraceFlags::kIsSampled}; - r.trace_flag = flags; - EXPECT_EQ(1, r.trace_flag.flags()); -} - -TEST(LogRecord, SeverityStore) -{ - LogRecord r; - r.severity = opentelemetry::logs::Severity::kError2; - EXPECT_EQ(r.severity, opentelemetry::logs::Severity::kError2); -} - -TEST(LogRecord, NameStore) -{ - LogRecord r; - r.name = "My Name"; - EXPECT_EQ(r.name, "My Name"); -} - -// Implement once Body field is established -// TEST(LogRecord, BodyStore) -//{ -// -//} - -/*TEST(LogRecord, ResourceStore) -{ - using M = std::map; - M m1 = {{"abc", "123"}, {"xyz", "456"}}; - - LogRecord r; - r.SetResource(m1); - - //EXPECT_EQ(common::KeyValueIterableView(m1), r.resource); -} - -TEST(LogRecord, AttributesStore) -{ - -}*/ From dce2d6f4e015b716920dd9468571406a2ddff068 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Mon, 26 Oct 2020 14:18:11 -0400 Subject: [PATCH 10/17] Format --- api/include/opentelemetry/logs/log_record.h | 38 ++++++++++--------- api/include/opentelemetry/logs/logger.h | 17 ++++++++- .../opentelemetry/logs/logger_provider.h | 16 ++++++++ api/include/opentelemetry/logs/noop.h | 16 ++++++++ api/include/opentelemetry/logs/provider.h | 16 ++++++++ sdk/test/logs/batch_log_processor_test.cc | 7 ++-- 6 files changed, 88 insertions(+), 22 deletions(-) diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h index 7cd8d7490e..1fba6ed931 100644 --- a/api/include/opentelemetry/logs/log_record.h +++ b/api/include/opentelemetry/logs/log_record.h @@ -1,16 +1,18 @@ -# Copyright 2019, OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ #pragma once @@ -32,9 +34,11 @@ namespace logs /* Note: using a class enum here won't allow enum values to be compared to integers, i.e. only other * Severity enums (need an explicit cast) + * Follows the Google standard for naming: + * https://google.github.io/styleguide/cppguide.html#Enumerator_Names */ enum class Severity : uint8_t -{ // Follows this standard: https://google.github.io/styleguide/cppguide.html#Enumerator_Names +{ kNone = 0, // default Severity; added, not part of Log Data Model kTrace = 1, kTrace2 = 2, @@ -90,13 +94,13 @@ struct LogRecord common::KeyValueIterable &attributes; // key/value pair list /* Default log record if user does not overwrite this. - * TODO: find better data type for "body" field - * Future enhancement: Potentially add other constructors to take default arguments from the user + * TODO: find better data type for "body" field + * Future enhancement: Potentially add other constructors to take default arguments from the user **/ LogRecord() : resource(_nullKV), attributes(_nullKV) { timestamp = core::SystemTimestamp(std::chrono::system_clock::now()); - name = ""; + name = ""; } /* 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 b68f4e63fe..c42212e75b 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -1,3 +1,19 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + #pragma once #include @@ -37,7 +53,6 @@ class Logger * @throws No exceptions under any circumstances. */ - // Potential bool isenabled(severity): Break into seperate future PRs // Log issue about macros (disabling log events below certain level) /* The below method is a logging statement that takes in a LogRecord. diff --git a/api/include/opentelemetry/logs/logger_provider.h b/api/include/opentelemetry/logs/logger_provider.h index 89ce19a8a1..b0ca7303dc 100644 --- a/api/include/opentelemetry/logs/logger_provider.h +++ b/api/include/opentelemetry/logs/logger_provider.h @@ -1,3 +1,19 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + #pragma once #include "opentelemetry/logs/logger.h" diff --git a/api/include/opentelemetry/logs/noop.h b/api/include/opentelemetry/logs/noop.h index 5f35d8bbc4..91d7e6c0c7 100644 --- a/api/include/opentelemetry/logs/noop.h +++ b/api/include/opentelemetry/logs/noop.h @@ -1,3 +1,19 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + #pragma once // Please refer to provider.h for documentation on how to obtain a Logger object. // diff --git a/api/include/opentelemetry/logs/provider.h b/api/include/opentelemetry/logs/provider.h index 90a00d881a..ab8e779b20 100644 --- a/api/include/opentelemetry/logs/provider.h +++ b/api/include/opentelemetry/logs/provider.h @@ -1,3 +1,19 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + #pragma once #include diff --git a/sdk/test/logs/batch_log_processor_test.cc b/sdk/test/logs/batch_log_processor_test.cc index 18525fe16b..be36f7f1b2 100644 --- a/sdk/test/logs/batch_log_processor_test.cc +++ b/sdk/test/logs/batch_log_processor_test.cc @@ -119,7 +119,7 @@ TEST_F(BatchLogProcessorTestPeer, TestShutdown) new std::vector>); auto batch_processor = GetMockProcessor(logs_received, is_shutdown); - const int num_logs = 3; + const int num_logs = 3; auto test_logs = GetTestSpans(batch_processor, num_logs); @@ -146,7 +146,7 @@ TEST_F(BatchLogProcessorTestPeer, TestForceFlush) new std::vector>); auto batch_processor = GetMockProcessor(logs_received, is_shutdown); - const int num_logs = 2048; + const int num_logs = 2048; auto test_logs = GetTestSpans(batch_processor, num_logs); @@ -181,8 +181,7 @@ TEST_F(BatchLogProcessorTestPeer, TestForceFlush) EXPECT_EQ(num_logs * 2, logs_received->size()); for (int i = 0; i < num_logs; ++i) { - EXPECT_EQ("Span " + std::to_string(i % num_logs), - logs_received->at(num_logs + i)->GetName()); + EXPECT_EQ("Span " + std::to_string(i % num_logs), logs_received->at(num_logs + i)->GetName()); } } From 38a4b7fa88a7bc463f36a2af0464dc19ea59aa07 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Mon, 26 Oct 2020 22:38:09 -0400 Subject: [PATCH 11/17] remove batch processor --- sdk/test/logs/batch_log_processor_test.cc | 286 ---------------------- 1 file changed, 286 deletions(-) delete mode 100644 sdk/test/logs/batch_log_processor_test.cc diff --git a/sdk/test/logs/batch_log_processor_test.cc b/sdk/test/logs/batch_log_processor_test.cc deleted file mode 100644 index be36f7f1b2..0000000000 --- a/sdk/test/logs/batch_log_processor_test.cc +++ /dev/null @@ -1,286 +0,0 @@ -#include "opentelemetry/sdk/logs/batch_log_processor.h" - -#include -#include -#include - -OPENTELEMETRY_BEGIN_NAMESPACE - -/** - * Returns a mock log exporter meant exclusively for testing only - */ -class MockLogExporter final : public sdk::logs::LogExporter -{ -public: - MockLogExporter( - std::shared_ptr>> logs_received, - std::shared_ptr> is_shutdown, - std::shared_ptr> is_export_completed, - const std::chrono::milliseconds export_delay = std::chrono::milliseconds(0)) noexcept - : logs_received_(logs_received), - is_shutdown_(is_shutdown), - is_export_completed_(is_export_completed), - export_delay_(export_delay) - {} - - sdk::logs::ExportResult Export( - const nostd::log> &recordables) noexcept override - { - *is_export_completed_ = false; - - std::this_thread::sleep_for(export_delay_); - - for (auto &recordable : recordables) - { - auto log = std::unique_ptr( - static_cast(recordable.release())); - - if (log != nullptr) - { - logs_received_->push_back(std::move(log)); - } - } - - *is_export_completed_ = true; - return sdk::logs::ExportResult::kSuccess; - } - - void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override - { - *is_shutdown_ = true; - } - - bool IsExportCompleted() { return is_export_completed_->load(); } - -private: - std::shared_ptr>> logs_received_; - std::shared_ptr> is_shutdown_; - std::shared_ptr> is_export_completed_; - // Meant exclusively to test force flush timeout - const std::chrono::milliseconds export_delay_; -}; - -/** - * Fixture Class - */ -class BatchLogProcessorTestPeer : public testing::Test -{ -public: - std::shared_ptr GetMockProcessor( - std::shared_ptr>> logs_received, - std::shared_ptr> is_shutdown, - std::shared_ptr> is_export_completed = - std::shared_ptr>(new std::atomic(false)), - const std::chrono::milliseconds export_delay = std::chrono::milliseconds(0), - const std::chrono::milliseconds schedule_delay_millis = std::chrono::milliseconds(5000), - const size_t max_queue_size = 2048, - const size_t max_export_batch_size = 512) - { - return std::shared_ptr(new sdk::logs::BatchLogProcessor( - GetMockExporter(logs_received, is_shutdown, is_export_completed, export_delay), - max_queue_size, schedule_delay_millis, max_export_batch_size)); - } - - std::unique_ptr>> GetTestSpans( - std::shared_ptr processor, - const int num_logs) - { - std::unique_ptr>> test_logs( - new std::vector>); - - for (int i = 0; i < num_logs; ++i) - { - test_logs->push_back(processor->MakeRecordable()); - static_cast(test_logs->at(i).get()) - ->SetName("Span " + std::to_string(i)); - } - - return test_logs; - } - -private: - std::unique_ptr GetMockExporter( - std::shared_ptr>> logs_received, - std::shared_ptr> is_shutdown, - std::shared_ptr> is_export_completed, - const std::chrono::milliseconds export_delay = std::chrono::milliseconds(0)) - { - return std::unique_ptr( - new MockLogExporter(logs_received, is_shutdown, is_export_completed, export_delay)); - } -}; - -/* ################################## TESTS ############################################ */ - -TEST_F(BatchLogProcessorTestPeer, TestShutdown) -{ - std::shared_ptr> is_shutdown(new std::atomic(false)); - std::shared_ptr>> logs_received( - new std::vector>); - - auto batch_processor = GetMockProcessor(logs_received, is_shutdown); - const int num_logs = 3; - - auto test_logs = GetTestSpans(batch_processor, num_logs); - - for (int i = 0; i < num_logs; ++i) - { - batch_processor->OnEnd(std::move(test_logs->at(i))); - } - - batch_processor->Shutdown(); - - EXPECT_EQ(num_logs, logs_received->size()); - for (int i = 0; i < num_logs; ++i) - { - EXPECT_EQ("Span " + std::to_string(i), logs_received->at(i)->GetName()); - } - - EXPECT_TRUE(is_shutdown->load()); -} - -TEST_F(BatchLogProcessorTestPeer, TestForceFlush) -{ - std::shared_ptr> is_shutdown(new std::atomic(false)); - std::shared_ptr>> logs_received( - new std::vector>); - - auto batch_processor = GetMockProcessor(logs_received, is_shutdown); - const int num_logs = 2048; - - auto test_logs = GetTestSpans(batch_processor, num_logs); - - for (int i = 0; i < num_logs; ++i) - { - batch_processor->OnEnd(std::move(test_logs->at(i))); - } - - // Give some time to export - std::this_thread::sleep_for(std::chrono::milliseconds(50)); - - batch_processor->ForceFlush(); - - EXPECT_EQ(num_logs, logs_received->size()); - for (int i = 0; i < num_logs; ++i) - { - EXPECT_EQ("Span " + std::to_string(i), logs_received->at(i)->GetName()); - } - - // Create some more logs to make sure that the processor still works - auto more_test_logs = GetTestSpans(batch_processor, num_logs); - for (int i = 0; i < num_logs; ++i) - { - batch_processor->OnEnd(std::move(more_test_logs->at(i))); - } - - // Give some time to export the logs - std::this_thread::sleep_for(std::chrono::milliseconds(50)); - - batch_processor->ForceFlush(); - - EXPECT_EQ(num_logs * 2, logs_received->size()); - for (int i = 0; i < num_logs; ++i) - { - EXPECT_EQ("Span " + std::to_string(i % num_logs), logs_received->at(num_logs + i)->GetName()); - } -} - -TEST_F(BatchLogProcessorTestPeer, TestManySpansLoss) -{ - /* Test that when exporting more than max_queue_size logs, some are most likely lost*/ - - std::shared_ptr> is_shutdown(new std::atomic(false)); - std::shared_ptr>> logs_received( - new std::vector>); - - const int max_queue_size = 4096; - - auto batch_processor = GetMockProcessor(logs_received, is_shutdown); - - auto test_logs = GetTestSpans(batch_processor, max_queue_size); - - for (int i = 0; i < max_queue_size; ++i) - { - batch_processor->OnEnd(std::move(test_logs->at(i))); - } - - // Give some time to export the logs - std::this_thread::sleep_for(std::chrono::milliseconds(700)); - - batch_processor->ForceFlush(); - - // Span should be exported by now - EXPECT_GE(max_queue_size, logs_received->size()); -} - -TEST_F(BatchLogProcessorTestPeer, TestManySpansLossLess) -{ - /* Test that no logs are lost when sending max_queue_size logs */ - - std::shared_ptr> is_shutdown(new std::atomic(false)); - std::shared_ptr>> logs_received( - new std::vector>); - - const int num_logs = 2048; - - auto batch_processor = GetMockProcessor(logs_received, is_shutdown); - - auto test_logs = GetTestSpans(batch_processor, num_logs); - - for (int i = 0; i < num_logs; ++i) - { - batch_processor->OnEnd(std::move(test_logs->at(i))); - } - - // Give some time to export the logs - std::this_thread::sleep_for(std::chrono::milliseconds(50)); - - batch_processor->ForceFlush(); - - EXPECT_EQ(num_logs, logs_received->size()); - for (int i = 0; i < num_logs; ++i) - { - EXPECT_EQ("Span " + std::to_string(i), logs_received->at(i)->GetName()); - } -} - -TEST_F(BatchLogProcessorTestPeer, TestScheduleDelayMillis) -{ - /* Test that max_export_batch_size logs are exported every schedule_delay_millis - seconds */ - - std::shared_ptr> is_shutdown(new std::atomic(false)); - std::shared_ptr> is_export_completed(new std::atomic(false)); - std::shared_ptr>> logs_received( - new std::vector>); - - const std::chrono::milliseconds export_delay(0); - const std::chrono::milliseconds schedule_delay_millis(2000); - const size_t max_export_batch_size = 512; - - auto batch_processor = GetMockProcessor(logs_received, is_shutdown, is_export_completed, - export_delay, schedule_delay_millis); - - auto test_logs = GetTestSpans(batch_processor, max_export_batch_size); - - for (size_t i = 0; i < max_export_batch_size; ++i) - { - batch_processor->OnEnd(std::move(test_logs->at(i))); - } - - // Sleep for schedule_delay_millis milliseconds - std::this_thread::sleep_for(schedule_delay_millis); - - // small delay to give time to export - std::this_thread::sleep_for(std::chrono::milliseconds(50)); - - // Spans should be exported by now - EXPECT_TRUE(is_export_completed->load()); - EXPECT_EQ(max_export_batch_size, logs_received->size()); - for (size_t i = 0; i < max_export_batch_size; ++i) - { - EXPECT_EQ("Span " + std::to_string(i), logs_received->at(i)->GetName()); - } -} - -OPENTELEMETRY_END_NAMESPACE From d47292811f2395ea06b63762a73c3068b4f865f4 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Tue, 27 Oct 2020 01:05:37 -0400 Subject: [PATCH 12/17] Remove unecessary header, comments --- api/include/opentelemetry/logs/log_record.h | 11 +++++------ api/include/opentelemetry/logs/logger.h | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h index 1fba6ed931..5adfb2e8f4 100644 --- a/api/include/opentelemetry/logs/log_record.h +++ b/api/include/opentelemetry/logs/log_record.h @@ -20,7 +20,6 @@ #include #include "opentelemetry/common/key_value_iterable_view.h" #include "opentelemetry/core/timestamp.h" -#include "opentelemetry/logs/logger.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/trace/span_id.h" @@ -87,7 +86,7 @@ struct LogRecord Severity severity; // Severity enum that combines severity_text and severity_number in the // LogDataModel (can separate in SDK) - // other fields + // 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 @@ -103,8 +102,8 @@ struct LogRecord name = ""; } - /* for ease of use, user can use this function to convert a map into a KeyValueIterable for the - * resources */ + /* for ease of use; user can use this function to convert a map into a KeyValueIterable for the + * resources field */ template ::value> * = nullptr> inline void SetResource(const T &_resource) @@ -112,8 +111,8 @@ struct LogRecord resource = common::KeyValueIterableView(_resource); } - /* for ease of use, user to use this function to convert a map into a KeyValueIterable for the - * attributes */ + /* for ease of use; user can use this function to convert a map into a KeyValueIterable for the + * attributes field */ template ::value> * = nullptr> inline void SetAttributes(const T &_attributes) diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index c42212e75b..57b597c64b 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -20,7 +20,7 @@ #include #include -#include "opentelemetry/common/key_value_iterable.h" // fix after moving to "common" +#include "opentelemetry/common/key_value_iterable.h" #include "opentelemetry/logs/log_record.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/span.h" From 4c397a580b2f0917a683683ca251eefcf36f5b39 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Tue, 27 Oct 2020 02:21:23 -0400 Subject: [PATCH 13/17] Rebase and format --- api/include/opentelemetry/logs/logger.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index 57b597c64b..cd148f1864 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -20,7 +20,7 @@ #include #include -#include "opentelemetry/common/key_value_iterable.h" +#include "opentelemetry/common/key_value_iterable.h" #include "opentelemetry/logs/log_record.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/span.h" From 7b2881ed284ce041c27ece763ced4adc9de826a2 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Tue, 27 Oct 2020 06:29:30 -0400 Subject: [PATCH 14/17] Add tests to increase code coverage --- api/test/logs/logger_provider_test.cc | 16 +++++++++ api/test/logs/logger_test.cc | 47 +++++++++++++++++++++++---- api/test/logs/noop_test.cc | 22 +++++++++++++ 3 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 api/test/logs/noop_test.cc diff --git a/api/test/logs/logger_provider_test.cc b/api/test/logs/logger_provider_test.cc index 7f1385424b..5b478c7fb0 100644 --- a/api/test/logs/logger_provider_test.cc +++ b/api/test/logs/logger_provider_test.cc @@ -4,6 +4,7 @@ #include "opentelemetry/nostd/string_view.h" #include +#include using opentelemetry::logs::Logger; using opentelemetry::logs::LoggerProvider; @@ -47,3 +48,18 @@ TEST(Provider, MultipleLoggerProviders) ASSERT_NE(Provider::GetLoggerProvider(), tf); } + +TEST(Provider, GetLogger) +{ + auto tf = shared_ptr(new TestProvider()); + // tests GetLogger(name, options) + auto logger = tf->GetLogger("logger1"); + EXPECT_EQ(nullptr, logger); + + // tests GetLogger(name, arguments) + + std::array sv{"string"}; + span args{sv}; + auto logger2 = tf->GetLogger("logger2", args); + EXPECT_EQ(nullptr, logger2); +} diff --git a/api/test/logs/logger_test.cc b/api/test/logs/logger_test.cc index 885274a68b..a2ecd96ab7 100644 --- a/api/test/logs/logger_test.cc +++ b/api/test/logs/logger_test.cc @@ -3,6 +3,7 @@ #include "opentelemetry/nostd/shared_ptr.h" #include +#include using opentelemetry::common::KeyValueIterable; using opentelemetry::logs::Logger; @@ -28,16 +29,41 @@ TEST(Logger, GetNoopLoggerName) string_view name = logger->getName(); EXPECT_EQ("NOOP Logger", name); } -/* TODO: add more tests */ + +TEST(Logger, GetNoopLoggerNameWithArgs) +{ + auto lp = Provider::GetLoggerProvider(); + + 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 + string_view name = logger->getName(); + EXPECT_EQ("NOOP Logger", name); +} + +TEST(Logger, NoopLog) +{ + auto lp = Provider::GetLoggerProvider(); + auto logger = lp->GetLogger("TestLogger"); + LogRecord r; + r.name = "Noop log name"; + logger->log(r); +} // Define a basic Logger class class TestLogger : public Logger { - // returns the name of the logger - string_view getName() noexcept override { return "My custom implementation"; } - - // structured logging void log(const LogRecord &record) noexcept override {} + + // returns the name of the logger + string_view getName() noexcept override + { + log(LogRecord{}); // ensure code coverage for the above method. the log() method is already + // tested in NoopLogger tests. + return "My custom implementation"; + } }; // Define a basic LoggerProvider class that returns an instance of the logger class defined above @@ -60,8 +86,17 @@ TEST(Logger, PushLoggerImplementation) auto test_provider = shared_ptr(new TestProvider()); Provider::SetLoggerProvider(test_provider); - auto lp = Provider::GetLoggerProvider(); + auto lp = Provider::GetLoggerProvider(); + + // GetLogger(name, options) function auto logger = lp->GetLogger("TestLogger"); string_view name = logger->getName(); EXPECT_EQ("My custom implementation", name); + + // GetLogger(name, args) function + std::array sv{"string"}; + span args{sv}; + auto logger2 = lp->GetLogger("TestLogger2", args); + string_view name2 = logger2->getName(); + EXPECT_EQ("My custom implementation", name); } diff --git a/api/test/logs/noop_test.cc b/api/test/logs/noop_test.cc new file mode 100644 index 0000000000..b47e670ea8 --- /dev/null +++ b/api/test/logs/noop_test.cc @@ -0,0 +1,22 @@ +#include "opentelemetry/logs/noop.h" +#include "opentelemetry/core/timestamp.h" + +#include +#include +#include + +#include + +using opentelemetry::core::SystemTimestamp; +using opentelemetry::logs::Logger; +using opentelemetry::logs::NoopLogger; + +TEST(NoopTest, UseNoopLoggers) +{ + std::shared_ptr logger{new NoopLogger{}}; + SystemTimestamp t1; + auto nlog = logger->log(LogRecord{.timestamp = t1, .name = "noop log event"}); + nlog->EXPECT_EQ(s1->GetLogger(), "Noop Logger"); + + s1->GetContext(); +} From f844441b47c46f3ed8583dd32d8e0ac47a79f24c Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Tue, 27 Oct 2020 15:23:45 -0400 Subject: [PATCH 15/17] Address comments --- api/include/opentelemetry/logs/log_record.h | 8 +-- api/include/opentelemetry/logs/logger.h | 70 +++++++++++++-------- api/include/opentelemetry/logs/noop.h | 7 +-- api/test/logs/logger_test.cc | 24 +++---- 4 files changed, 59 insertions(+), 50 deletions(-) diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h index 5adfb2e8f4..5f09eed214 100644 --- a/api/include/opentelemetry/logs/log_record.h +++ b/api/include/opentelemetry/logs/log_record.h @@ -38,7 +38,6 @@ namespace logs */ enum class Severity : uint8_t { - kNone = 0, // default Severity; added, not part of Log Data Model kTrace = 1, kTrace2 = 2, kTrace3 = 3, @@ -62,7 +61,8 @@ enum class Severity : uint8_t kFatal = 21, kFatal2 = 22, kFatal3 = 23, - kFatal4 = 24 + kFatal4 = 24, + 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 @@ -93,12 +93,12 @@ struct LogRecord common::KeyValueIterable &attributes; // key/value pair list /* Default log record if user does not overwrite this. - * TODO: find better data type for "body" field + * TODO: find better data type for "body" field to represent the type (specified by Log Data Model ) * Future enhancement: Potentially add other constructors to take default arguments from the user **/ LogRecord() : resource(_nullKV), attributes(_nullKV) { - timestamp = core::SystemTimestamp(std::chrono::system_clock::now()); + // TODO : in SDK, assign a default timestamp if not specified name = ""; } diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index cd148f1864..f5c190f001 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -39,75 +39,95 @@ class Logger virtual ~Logger() = default; /* Returns the name of the logger */ - virtual nostd::string_view getName() = 0; + // 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; /** - * Creates a log message with the passed characteristics. + * Each of the following overloaded log(...) methods + * creates a log message with the specific parameters passed. * * @param name the name of the log event. - * @param sev the severity level of the log event. - * @param msg the string message of the log (perhaps support std::fmt or fmt-lib format). + * @param severity the severity level of the log event. + * @param message the string message of the log (perhaps support std::fmt or fmt-lib format). * @param record the log record (object type LogRecord) that is logged. * @param attributes the attributes, stored as a 2D list of key/value pairs, that are associated * with this log. * @throws No exceptions under any circumstances. */ - // Log issue about macros (disabling log events below certain level) - /* 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 */ virtual void log(const LogRecord &record) noexcept = 0; - // First, finalize core, - // Afterwards, come back and add overloads /** Overloaded methods for unstructured logging **/ - void log(nostd::string_view msg) noexcept + void log(nostd::string_view message) noexcept { - log(Severity::kNone, msg); // Set severity to NONE as default then call the log method below + // TODO: move this log method to the SDK? since it needs to call an SDK method "isEnabled(Severity)" + + // if(isEnabled(Severity::kDefault)){ + // return; + // } + + // Set severity to the default then call log(Severity, String message) method + log(Severity::kDefault, message); } - void log(Severity sev, nostd::string_view msg) noexcept - { - // Set timestamp to now as default then call the log method below - log(sev, msg, core::SystemTimestamp(std::chrono::system_clock::now())); + void log(Severity severity, nostd::string_view message) noexcept + { + // TODO: move this log method to the SDK? since it needs to call an SDK method "isEnabled(Severity)" + // if(isEnabled(Severity::kDefault)){ + // return; + // } + + // TODO: set default timestamp later (not in API) + log(severity, message, core::SystemTimestamp(std::chrono::system_clock::now())); } - void log(Severity sev, nostd::string_view msg, core::SystemTimestamp time) noexcept + void log(Severity severity, nostd::string_view message, core::SystemTimestamp time) noexcept { + // TODO: move this log method to the SDK? since it needs to call an SDK method "isEnabled(Severity)" + // if(isEnabled(Severity::kDefault)){ + // return; + // } + + // creates a LogRecord object with given parameters, then calls log(LogRecord) LogRecord r; - r.body = msg; - r.severity = sev; + r.severity = severity; + r.body = message; r.timestamp = time; - log(r); // converts to a LogRecord object, then calls log(LogRecord) + 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 template ::value> * = nullptr> - void log(nostd::string_view name = "", - Severity sev = Severity::kNone, + void log(Severity severity = Severity::kDefault, + nostd::string_view name = "", const T &attributes = {}) noexcept { - log(name, sev, common::KeyValueIterableView(attributes)); + log(severity, name, common::KeyValueIterableView(attributes)); } - void log(nostd::string_view name, - Severity sev, + void log(Severity severity, + nostd::string_view name, const common::KeyValueIterable &attributes) noexcept { + // creates a LogRecord object with given parameters, then calls log(LogRecord) LogRecord r; + r.severity = severity; r.name = name; - r.severity = sev; r.attributes = attributes; - log(r); // converts to a LogRecord object, then calls log(LogRecord) + log(r); } + // 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) **/ // template void log(T &some_obj) noexcept override; diff --git a/api/include/opentelemetry/logs/noop.h b/api/include/opentelemetry/logs/noop.h index 91d7e6c0c7..3184baa3c6 100644 --- a/api/include/opentelemetry/logs/noop.h +++ b/api/include/opentelemetry/logs/noop.h @@ -20,6 +20,8 @@ // This file is part of the internal implementation of OpenTelemetry. Nothing in this file should be // used directly. Please refer to logger.h for documentation on these interfaces. +#include + #include "opentelemetry/context/runtime_context.h" #include "opentelemetry/logs/logger.h" #include "opentelemetry/logs/logger_provider.h" @@ -27,8 +29,6 @@ #include "opentelemetry/nostd/unique_ptr.h" #include "opentelemetry/version.h" -#include - OPENTELEMETRY_BEGIN_NAMESPACE namespace logs { @@ -41,9 +41,6 @@ class NoopLogger final : public Logger public: NoopLogger() = default; - // returns the name of the logger - nostd::string_view getName() noexcept override { return "NOOP Logger"; } - void log(const LogRecord &record) noexcept override {} }; diff --git a/api/test/logs/logger_test.cc b/api/test/logs/logger_test.cc index a2ecd96ab7..a110554e97 100644 --- a/api/test/logs/logger_test.cc +++ b/api/test/logs/logger_test.cc @@ -26,8 +26,8 @@ TEST(Logger, GetNoopLoggerName) { auto lp = Provider::GetLoggerProvider(); auto logger = lp->GetLogger("TestLogger"); - string_view name = logger->getName(); - EXPECT_EQ("NOOP Logger", name); + // string_view name = logger->getName(); + // EXPECT_EQ("NOOP Logger", name); } TEST(Logger, GetNoopLoggerNameWithArgs) @@ -39,8 +39,8 @@ TEST(Logger, GetNoopLoggerNameWithArgs) auto logger = lp->GetLogger("NoopLoggerWithArgs", args); // should probably also test that arguments were set properly too // by adding a getArgs() method in NoopLogger - string_view name = logger->getName(); - EXPECT_EQ("NOOP Logger", name); + // string_view name = logger->getName(); + // EXPECT_EQ("NOOP Logger", name); } TEST(Logger, NoopLog) @@ -56,14 +56,6 @@ TEST(Logger, NoopLog) class TestLogger : public Logger { void log(const LogRecord &record) noexcept override {} - - // returns the name of the logger - string_view getName() noexcept override - { - log(LogRecord{}); // ensure code coverage for the above method. the log() method is already - // tested in NoopLogger tests. - return "My custom implementation"; - } }; // Define a basic LoggerProvider class that returns an instance of the logger class defined above @@ -90,13 +82,13 @@ TEST(Logger, PushLoggerImplementation) // GetLogger(name, options) function auto logger = lp->GetLogger("TestLogger"); - string_view name = logger->getName(); - EXPECT_EQ("My custom implementation", name); + // string_view name = logger->getName(); + // EXPECT_EQ("My custom implementation", name); // GetLogger(name, args) function std::array sv{"string"}; span args{sv}; auto logger2 = lp->GetLogger("TestLogger2", args); - string_view name2 = logger2->getName(); - EXPECT_EQ("My custom implementation", name); + // string_view name2 = logger2->getName(); + // EXPECT_EQ("My custom implementation", name); } From fe0109ac960388ec5d28e4897d5e9744e185dbcf Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Tue, 27 Oct 2020 16:23:40 -0400 Subject: [PATCH 16/17] Reformat --- api/include/opentelemetry/logs/log_record.h | 59 +++++++++++---------- api/include/opentelemetry/logs/logger.h | 46 ++++++++-------- api/test/logs/logger_provider_test.cc | 6 +-- api/test/logs/logger_test.cc | 22 +++----- api/test/logs/noop_test.cc | 7 ++- 5 files changed, 67 insertions(+), 73 deletions(-) diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h index 5f09eed214..c663c1c9c7 100644 --- a/api/include/opentelemetry/logs/log_record.h +++ b/api/include/opentelemetry/logs/log_record.h @@ -38,31 +38,31 @@ namespace logs */ enum class Severity : uint8_t { - kTrace = 1, - kTrace2 = 2, - kTrace3 = 3, - kTrace4 = 4, - kDebug = 5, - kDebug2 = 6, - kDebug3 = 7, - kDebug4 = 8, - kInfo = 9, - kInfo2 = 10, - kInfo3 = 11, - kInfo4 = 12, - kWarn = 13, - kWarn2 = 14, - kWarn3 = 15, - kWarn4 = 16, - kError = 17, - kError2 = 18, - kError3 = 19, - kError4 = 20, - kFatal = 21, - kFatal2 = 22, - kFatal3 = 23, - kFatal4 = 24, - kDefault = kInfo // default severity is set to kInfo level, similar to what is done in ILogger + kTrace = 1, + kTrace2 = 2, + kTrace3 = 3, + kTrace4 = 4, + kDebug = 5, + kDebug2 = 6, + kDebug3 = 7, + kDebug4 = 8, + kInfo = 9, + kInfo2 = 10, + kInfo3 = 11, + kInfo4 = 12, + kWarn = 13, + kWarn2 = 14, + kWarn3 = 15, + kWarn4 = 16, + kError = 17, + kError2 = 18, + kError3 = 19, + kError4 = 20, + kFatal = 21, + kFatal2 = 22, + kFatal3 = 23, + kFatal4 = 24, + 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 @@ -93,13 +93,14 @@ struct LogRecord common::KeyValueIterable &attributes; // key/value pair list /* Default log record if user does not overwrite this. - * TODO: find better data type for "body" field to represent the type (specified by Log Data Model ) - * Future enhancement: Potentially add other constructors to take default arguments from the user + * 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) { - // TODO : in SDK, assign a default timestamp if not specified - name = ""; + // TODO: in SDK, assign a default timestamp if not specified + name = ""; } /* 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 f5c190f001..e048acaa86 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -39,12 +39,12 @@ 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; + // 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; /** - * Each of the following overloaded log(...) methods - * creates a log message with the specific parameters passed. + * Each of the following overloaded log(...) methods + * creates a log message with the specific parameters passed. * * @param name the name of the log event. * @param severity the severity level of the log event. @@ -61,52 +61,53 @@ class Logger */ virtual void log(const LogRecord &record) noexcept = 0; - /** Overloaded methods for unstructured logging **/ void log(nostd::string_view message) noexcept { - // TODO: move this log method to the SDK? since it needs to call an SDK method "isEnabled(Severity)" + // TODO: move this log method to the SDK? since it needs to call an SDK method + // "isEnabled(Severity)" // if(isEnabled(Severity::kDefault)){ // return; // } - // Set severity to the default then call log(Severity, String message) method - log(Severity::kDefault, message); + // Set severity to the default then call log(Severity, String message) method + log(Severity::kDefault, message); } void log(Severity severity, nostd::string_view message) noexcept - { - // TODO: move this log method to the SDK? since it needs to call an SDK method "isEnabled(Severity)" - // if(isEnabled(Severity::kDefault)){ + { + // TODO: move this log method to the SDK? since it needs to call an SDK method + // "isEnabled(Severity)" if(isEnabled(Severity::kDefault)){ // return; - // } - + // } + // TODO: set default timestamp later (not in API) log(severity, message, core::SystemTimestamp(std::chrono::system_clock::now())); } void log(Severity severity, nostd::string_view message, core::SystemTimestamp time) noexcept { - // TODO: move this log method to the SDK? since it needs to call an SDK method "isEnabled(Severity)" - // if(isEnabled(Severity::kDefault)){ + // TODO: move this log method to the SDK? since it needs to call an SDK method + // "isEnabled(Severity)" if(isEnabled(Severity::kDefault)){ // return; - // } + // } - // creates a LogRecord object with given parameters, then calls log(LogRecord) + // creates a LogRecord object with given parameters, then calls log(LogRecord) LogRecord r; r.severity = severity; r.body = message; r.timestamp = time; - log(r); + 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 + // TODO: separate this method into separate methods since it is not useful for user to create + // empty logs template ::value> * = nullptr> - void log(Severity severity = Severity::kDefault, + void log(Severity severity = Severity::kDefault, nostd::string_view name = "", const T &attributes = {}) noexcept { @@ -123,10 +124,11 @@ class Logger r.name = name; r.attributes = attributes; - log(r); + log(r); } - // TODO: add function aliases such as void debug(), void trace(), void info(), etc. for each severity level + // 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/test/logs/logger_provider_test.cc b/api/test/logs/logger_provider_test.cc index 5b478c7fb0..91fd8f8c15 100644 --- a/api/test/logs/logger_provider_test.cc +++ b/api/test/logs/logger_provider_test.cc @@ -1,11 +1,11 @@ +#include +#include + #include "opentelemetry/logs/provider.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/span.h" #include "opentelemetry/nostd/string_view.h" -#include -#include - using opentelemetry::logs::Logger; using opentelemetry::logs::LoggerProvider; using opentelemetry::logs::Provider; diff --git a/api/test/logs/logger_test.cc b/api/test/logs/logger_test.cc index a110554e97..937ece860a 100644 --- a/api/test/logs/logger_test.cc +++ b/api/test/logs/logger_test.cc @@ -1,10 +1,10 @@ +#include +#include + #include "opentelemetry/logs/logger.h" #include "opentelemetry/logs/provider.h" #include "opentelemetry/nostd/shared_ptr.h" -#include -#include - using opentelemetry::common::KeyValueIterable; using opentelemetry::logs::Logger; using opentelemetry::logs::LoggerProvider; @@ -24,10 +24,8 @@ TEST(Logger, GetLoggerDefault) TEST(Logger, GetNoopLoggerName) { - auto lp = Provider::GetLoggerProvider(); - auto logger = lp->GetLogger("TestLogger"); - // string_view name = logger->getName(); - // EXPECT_EQ("NOOP Logger", name); + auto lp = Provider::GetLoggerProvider(); + auto logger = lp->GetLogger("TestLogger"); } TEST(Logger, GetNoopLoggerNameWithArgs) @@ -39,8 +37,6 @@ TEST(Logger, GetNoopLoggerNameWithArgs) auto logger = lp->GetLogger("NoopLoggerWithArgs", args); // should probably also test that arguments were set properly too // by adding a getArgs() method in NoopLogger - // string_view name = logger->getName(); - // EXPECT_EQ("NOOP Logger", name); } TEST(Logger, NoopLog) @@ -81,14 +77,10 @@ TEST(Logger, PushLoggerImplementation) auto lp = Provider::GetLoggerProvider(); // GetLogger(name, options) function - auto logger = lp->GetLogger("TestLogger"); - // string_view name = logger->getName(); - // EXPECT_EQ("My custom implementation", name); + auto logger = lp->GetLogger("TestLogger"); // GetLogger(name, args) function std::array sv{"string"}; span args{sv}; - auto logger2 = lp->GetLogger("TestLogger2", args); - // string_view name2 = logger2->getName(); - // EXPECT_EQ("My custom implementation", name); + auto logger2 = lp->GetLogger("TestLogger2", args); } diff --git a/api/test/logs/noop_test.cc b/api/test/logs/noop_test.cc index b47e670ea8..d16375b246 100644 --- a/api/test/logs/noop_test.cc +++ b/api/test/logs/noop_test.cc @@ -1,11 +1,10 @@ -#include "opentelemetry/logs/noop.h" -#include "opentelemetry/core/timestamp.h" - +#include #include #include #include -#include +#include "opentelemetry/core/timestamp.h" +#include "opentelemetry/logs/noop.h" using opentelemetry::core::SystemTimestamp; using opentelemetry::logs::Logger; From a2d883471ba422d7a3ddfcb435e794eb7958a5f3 Mon Sep 17 00:00:00 2001 From: Seufert Date: Tue, 27 Oct 2020 15:59:51 -0700 Subject: [PATCH 17/17] Made log methods inline --- api/include/opentelemetry/logs/logger.h | 41 ++++++++----------------- api/test/logs/noop_test.cc | 21 ------------- 2 files changed, 13 insertions(+), 49 deletions(-) delete mode 100644 api/test/logs/noop_test.cc diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index e048acaa86..5f22066460 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -62,37 +62,22 @@ class Logger virtual void log(const LogRecord &record) noexcept = 0; /** Overloaded methods for unstructured logging **/ - void log(nostd::string_view message) noexcept + inline void log(nostd::string_view message) noexcept { - // TODO: move this log method to the SDK? since it needs to call an SDK method - // "isEnabled(Severity)" - - // if(isEnabled(Severity::kDefault)){ - // return; - // } - // Set severity to the default then call log(Severity, String message) method log(Severity::kDefault, message); } - void log(Severity severity, nostd::string_view message) noexcept + inline void log(Severity severity, nostd::string_view message) noexcept { - // TODO: move this log method to the SDK? since it needs to call an SDK method - // "isEnabled(Severity)" if(isEnabled(Severity::kDefault)){ - // return; - // } - // TODO: set default timestamp later (not in API) log(severity, message, core::SystemTimestamp(std::chrono::system_clock::now())); } - void log(Severity severity, nostd::string_view message, core::SystemTimestamp time) noexcept + inline void log(Severity severity, + nostd::string_view message, + core::SystemTimestamp time) noexcept { - // TODO: move this log method to the SDK? since it needs to call an SDK method - // "isEnabled(Severity)" if(isEnabled(Severity::kDefault)){ - // return; - // } - // creates a LogRecord object with given parameters, then calls log(LogRecord) LogRecord r; r.severity = severity; @@ -107,16 +92,16 @@ class Logger // empty logs template ::value> * = nullptr> - void log(Severity severity = Severity::kDefault, - nostd::string_view name = "", - const T &attributes = {}) noexcept + inline void log(Severity severity = Severity::kDefault, + nostd::string_view name = "", + const T &attributes = {}) noexcept { log(severity, name, common::KeyValueIterableView(attributes)); } - void log(Severity severity, - nostd::string_view name, - const common::KeyValueIterable &attributes) noexcept + inline void log(Severity severity, + nostd::string_view name, + const common::KeyValueIterable &attributes) noexcept { // creates a LogRecord object with given parameters, then calls log(LogRecord) LogRecord r; @@ -132,7 +117,7 @@ class Logger /** Future enhancement: templated method for objects / custom types (e.g. JSON, XML, custom * classes, etc) **/ - // template void log(T &some_obj) noexcept override; + // template virtual void log(T &some_obj) noexcept; }; } // namespace logs -OPENTELEMETRY_END_NAMESPACE +OPENTELEMETRY_END_NAMESPACE \ No newline at end of file diff --git a/api/test/logs/noop_test.cc b/api/test/logs/noop_test.cc deleted file mode 100644 index d16375b246..0000000000 --- a/api/test/logs/noop_test.cc +++ /dev/null @@ -1,21 +0,0 @@ -#include -#include -#include -#include - -#include "opentelemetry/core/timestamp.h" -#include "opentelemetry/logs/noop.h" - -using opentelemetry::core::SystemTimestamp; -using opentelemetry::logs::Logger; -using opentelemetry::logs::NoopLogger; - -TEST(NoopTest, UseNoopLoggers) -{ - std::shared_ptr logger{new NoopLogger{}}; - SystemTimestamp t1; - auto nlog = logger->log(LogRecord{.timestamp = t1, .name = "noop log event"}); - nlog->EXPECT_EQ(s1->GetLogger(), "Noop Logger"); - - s1->GetContext(); -}