From c61877281f70e414ec0972e2e32ccc8a18e2077c Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Thu, 10 Dec 2020 02:13:07 -0500 Subject: [PATCH 01/21] Add Recordable interface --- .../opentelemetry/sdk/logs/recordable.h | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 sdk/include/opentelemetry/sdk/logs/recordable.h diff --git a/sdk/include/opentelemetry/sdk/logs/recordable.h b/sdk/include/opentelemetry/sdk/logs/recordable.h new file mode 100644 index 0000000000..3deadb3272 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/logs/recordable.h @@ -0,0 +1,94 @@ +#pragma once + +#include "opentelemetry/common/attribute_value.h" +#include "opentelemetry/common/key_value_iterable.h" +#include "opentelemetry/core/timestamp.h" +#include "opentelemetry/logs/severity.h" +#include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/sdk/common/empty_attributes.h" +#include "opentelemetry/trace/span.h" +#include "opentelemetry/trace/span_context.h" +#include "opentelemetry/trace/span_id.h" +#include "opentelemetry/trace/trace_flags.h" +#include "opentelemetry/trace/trace_id.h" +#include "opentelemetry/version.h" + +#include + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace logs +{ +/** + * Maintains a representation of a log in a format that can be processed by a recorder. + * + * This class is thread-compatible. + */ +class Recordable +{ +public: + virtual ~Recordable() = default; + + /** + * Set the timestamp for this log. + * @param timestamp the timestamp to set + */ + virtual void SetTimestamp(core::SystemTimestamp timestamp) noexcept = 0; + + /** + * Set the severity for this log. + * @param severity the severity of the event + */ + virtual void SetSeverity(opentelemetry::logs::Severity severity) noexcept = 0; + + /** + * Set name for this log + * @param name the name to set + */ + virtual void SetName(nostd::string_view name) noexcept = 0; + + /** + * Set body field for this log. + * @param message the body to set + */ + virtual void SetBody(nostd::string_view message) noexcept = 0; + + /** + * Set a single resource of a log record. + * @param key the name of the resource to set + * @param value the resource value to set + */ + virtual void SetResource(nostd::string_view key, + const opentelemetry::common::AttributeValue &value) noexcept = 0; + + /** + * Set an attribute of a log. + * @param key the name of the attribute + * @param value the attribute value + */ + virtual void SetAttribute(nostd::string_view key, + const opentelemetry::common::AttributeValue &value) noexcept = 0; + + /** + * Set the trace id for this log. + * @param trace_id the trace id to set + */ + virtual void SetTraceId(opentelemetry::trace::TraceId trace_id) noexcept = 0; + + /** + * Set the span id for this log. + * @param span_id the span id to set + */ + virtual void SetSpanId(opentelemetry::trace::SpanId span_id) noexcept = 0; + + /** + * Inject trace_flags for this log. + * @param trace_flags the trace flags to set + */ + virtual void SetTraceFlags(opentelemetry::trace::TraceFlags trace_flags) noexcept = 0; + +}; +} // namespace logs +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE From 96f1620ad6a87e2a0f7b9a5c8d6a07abf9e237f2 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Thu, 10 Dec 2020 06:08:17 -0500 Subject: [PATCH 02/21] Add copy of AttributeMap to sdk/logs --- .../opentelemetry/sdk/logs/attribute_utils.h | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 sdk/include/opentelemetry/sdk/logs/attribute_utils.h diff --git a/sdk/include/opentelemetry/sdk/logs/attribute_utils.h b/sdk/include/opentelemetry/sdk/logs/attribute_utils.h new file mode 100644 index 0000000000..e74d09a5a5 --- /dev/null +++ b/sdk/include/opentelemetry/sdk/logs/attribute_utils.h @@ -0,0 +1,123 @@ +#pragma once + +#include +#include +#include "opentelemetry/common/attribute_value.h" +#include "opentelemetry/common/key_value_iterable_view.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace logs +{ +/** + * A counterpart to AttributeValue that makes sure a value is owned. This + * replaces all non-owning references with owned copies. + */ +using LogRecordAttributeValue = nostd::variant, + std::vector, + std::vector, + std::vector, + std::vector, + std::vector, + std::vector>; + +/** + * Creates an owned copy (LogRecordAttributeValue) of a non-owning AttributeValue. + */ +struct AttributeConverter +{ + LogRecordAttributeValue operator()(bool v) { return LogRecordAttributeValue(v); } + LogRecordAttributeValue operator()(int32_t v) { return LogRecordAttributeValue(v); } + LogRecordAttributeValue operator()(uint32_t v) { return LogRecordAttributeValue(v); } + /*LogRecordAttributeValue operator()(int v) + { + return LogRecordAttributeValue(static_cast(v)); + }*/ + LogRecordAttributeValue operator()(int64_t v) { return LogRecordAttributeValue(v); } + LogRecordAttributeValue operator()(uint64_t v) { return LogRecordAttributeValue(v); } + LogRecordAttributeValue operator()(double v) { return LogRecordAttributeValue(v); } + LogRecordAttributeValue operator()(nostd::string_view v) + { + return LogRecordAttributeValue(std::string(v)); + } + LogRecordAttributeValue operator()(nostd::span v) { return convertSpan(v); } + LogRecordAttributeValue operator()(nostd::span v) + { + return convertSpan(v); + } + LogRecordAttributeValue operator()(nostd::span v) + { + return convertSpan(v); + } + LogRecordAttributeValue operator()(nostd::span v) + { + return convertSpan(v); + } + LogRecordAttributeValue operator()(nostd::span v) + { + return convertSpan(v); + } + LogRecordAttributeValue operator()(nostd::span v) { return convertSpan(v); } + LogRecordAttributeValue operator()(nostd::span v) + { + return convertSpan(v); + } + + template + LogRecordAttributeValue convertSpan(nostd::span vals) + { + std::vector copy; + for (auto &val : vals) + { + copy.push_back(T(val)); + } + + return LogRecordAttributeValue(std::move(copy)); + } +}; + +/** + * Class for storing attributes. + */ +class AttributeMap +{ +public: + // Contruct empty attribute map + AttributeMap(){}; + + // Contruct attribute map and populate with attributes + AttributeMap(const opentelemetry::common::KeyValueIterable &attributes) + { + attributes.ForEachKeyValue([&](nostd::string_view key, + opentelemetry::common::AttributeValue value) noexcept { + SetAttribute(key, value); + return true; + }); + } + + const std::unordered_map &GetAttributes() const noexcept + { + return attributes_; + } + + void SetAttribute(nostd::string_view key, + const opentelemetry::common::AttributeValue &value) noexcept + { + attributes_[std::string(key)] = nostd::visit(converter_, value); + } + +private: + std::unordered_map attributes_; + AttributeConverter converter_; +}; +} // namespace logs +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE From d3b27dd7d3142ea9cd0653b5f106c6355ca74f5e Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Thu, 10 Dec 2020 02:38:47 -0500 Subject: [PATCH 03/21] Add default Recordable implementation (LogRecord class) --- .../opentelemetry/sdk/logs/log_record.h | 207 ++++++++++++++++++ 1 file changed, 207 insertions(+) create mode 100644 sdk/include/opentelemetry/sdk/logs/log_record.h diff --git a/sdk/include/opentelemetry/sdk/logs/log_record.h b/sdk/include/opentelemetry/sdk/logs/log_record.h new file mode 100644 index 0000000000..806048f47b --- /dev/null +++ b/sdk/include/opentelemetry/sdk/logs/log_record.h @@ -0,0 +1,207 @@ +/* + * 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 +#include +#include "opentelemetry/sdk/logs/attribute_utils.h" // same as traces/attribute_utils +#include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/sdk/logs/recordable.h" +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace logs +{ + +/** + * 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) + * + */ +class LogRecord final : public Recordable +{ +private: + // Log Data Model fields + core::SystemTimestamp timestamp_; // uint64 nanoseconds since Unix epoch + opentelemetry::trace::TraceId trace_id_; // byte sequence + opentelemetry::trace::SpanId span_id_; // byte sequence + opentelemetry::trace::TraceFlags trace_flags_; // byte + opentelemetry::logs::Severity severity_; // Severity enum + nostd::string_view name_; // string + nostd::string_view body_; // currently a simple string, but should be changed "Any" type + AttributeMap resource_map_; + AttributeMap attributes_map_; + +public: + + /********** Setters for each field (overrides methods from the Recordable interface) ************/ + /** + * Set the timestamp for this log. + * @param timestamp the timestamp of the event + */ + void SetTimestamp(core::SystemTimestamp timestamp) noexcept override + { + timestamp_ = timestamp; + } + + /** + * Set the severity for this log. + * @param severity the severity of the event + */ + void SetSeverity(opentelemetry::logs::Severity severity) noexcept override + { + severity_ = severity; + } + + /** + * Set name for this log + * @param name the name to set + */ + void SetName(nostd::string_view name) noexcept override + { + name_ = name; + } + + /** + * Set body field for this log. + * @param message the body to set + */ + void SetBody(nostd::string_view message) noexcept override + { + body_ = message; + } + + /** + * Set a resource for this log. + * @param name the name of the resource + * @param value the resource value + */ + void SetResource(nostd::string_view key, + const opentelemetry::common::AttributeValue &value) noexcept override + { + resource_map_.SetAttribute(key, value); + } + + /** + * Set an attribute of a log. + * @param name the name of the attribute + * @param value the attribute value + */ + + void SetAttribute(nostd::string_view key, + const opentelemetry::common::AttributeValue &value) noexcept override + { + attributes_map_.SetAttribute(key, value); + } + + /** + * Set trace id for this log. + * @param trace_id the trace id to set + */ + void SetTraceId(opentelemetry::trace::TraceId trace_id) noexcept override + { + trace_id_ = trace_id; + } + + /** + * Set span id for this log. + * @param span_id the span id to set + */ + virtual void SetSpanId(opentelemetry::trace::SpanId span_id) noexcept override + { + span_id_ = span_id; + } + + /** + * Inject a trace_flags for this log. + * @param trace_flags the span id to set + */ + void SetTraceFlags(opentelemetry::trace::TraceFlags trace_flags) noexcept override + { + trace_flags_ = trace_flags; + } + + /************************** Getters for each field ****************************/ + + /** + * Get the timestamp for this log + * @return the timestamp for this log + */ + core::SystemTimestamp GetTimestamp() const noexcept { return timestamp_; } + + /** + * Get the severity for this log + * @return the severity for this log + */ + opentelemetry::logs::Severity GetSeverity() const noexcept { return severity_; } + + /** + * Get the name of this log + * @return the name of this log + */ + nostd::string_view GetName() const noexcept { return name_; } + + /** + * Get the body of this log + * @return the body of this log + */ + nostd::string_view GetBody() const noexcept { return body_; } + + /** + * Get the resource field for this log + * @return the resource field for this log + */ + const std::unordered_map &GetResource() const noexcept + { + return resource_map_.GetAttributes(); + } + + /** + * Get the attributes for this log + * @return the attributes for this log + */ + const std::unordered_map &GetAttributes() const noexcept + { + return attributes_map_.GetAttributes(); + } + + /** + * Get the trace id for this log + * @return the trace id for this log + */ + opentelemetry::trace::TraceId GetTraceId() const noexcept { return trace_id_; } + + /** + * Get the span id for this log + * @return the span id for this log + */ + opentelemetry::trace::SpanId GetSpanId() const noexcept { return span_id_; } + + /** + * Get the trace flags for this log + * @return the trace flags for this log + */ + opentelemetry::trace::TraceFlags GetTraceFlags() const noexcept { return trace_flags_; } + + +}; +} // namespace logs +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE From 59dba66b7e05f5ce7c895270681935130bf55190 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Thu, 10 Dec 2020 02:52:47 -0500 Subject: [PATCH 04/21] Update processor and exporter --- sdk/include/opentelemetry/sdk/logs/exporter.h | 15 +++++++++++++-- sdk/include/opentelemetry/sdk/logs/processor.h | 14 ++++++++++++-- .../opentelemetry/sdk/logs/simple_log_processor.h | 4 +++- sdk/src/logs/simple_log_processor.cc | 9 +++++++-- sdk/test/logs/simple_log_processor_test.cc | 4 ++-- 5 files changed, 37 insertions(+), 9 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/logs/exporter.h b/sdk/include/opentelemetry/sdk/logs/exporter.h index 6f44d32a28..8009e373c4 100644 --- a/sdk/include/opentelemetry/sdk/logs/exporter.h +++ b/sdk/include/opentelemetry/sdk/logs/exporter.h @@ -18,9 +18,9 @@ #include #include -#include "opentelemetry/logs/log_record.h" #include "opentelemetry/nostd/span.h" #include "opentelemetry/sdk/logs/processor.h" +#include "opentelemetry/sdk/logs/recordable.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk @@ -46,6 +46,17 @@ class LogExporter public: virtual ~LogExporter() = default; + /** + * Create a log recordable. This object will be used to record log data and + * will subsequently be passed to LogExporter::Export. Vendors can implement + * custom recordables or use the default LogRecord recordable provided by the + * SDK. + * @return a newly initialized Recordable object + * + * Note: This method must be callable from multiple threads. + */ + virtual std::unique_ptr MakeRecordable() noexcept = 0; + /** * Exports the batch of log records to their export destination. * This method must not be called concurrently for the same exporter instance. @@ -55,7 +66,7 @@ class LogExporter * @returns an ExportResult code (whether export was success or failure) */ virtual ExportResult Export( - const nostd::span> &records) noexcept = 0; + const nostd::span> &records) noexcept = 0; /** * Marks the exporter as ShutDown and cleans up any resources as required. diff --git a/sdk/include/opentelemetry/sdk/logs/processor.h b/sdk/include/opentelemetry/sdk/logs/processor.h index 7468548dcf..6cb5696ce8 100644 --- a/sdk/include/opentelemetry/sdk/logs/processor.h +++ b/sdk/include/opentelemetry/sdk/logs/processor.h @@ -18,7 +18,7 @@ #include #include -#include "opentelemetry/logs/log_record.h" +#include "opentelemetry/sdk/logs/recordable.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk @@ -34,11 +34,21 @@ class LogProcessor public: virtual ~LogProcessor() = default; + +/** + * Create a log recordable. This requests a new log recordable from the + * associated exporter. + * @return a newly initialized recordable + * + * Note: This method must be callable from multiple threads. + */ + virtual std::unique_ptr MakeRecordable() noexcept = 0; + /** * OnReceive is called by the SDK once a log record has been successfully created. * @param record the log record */ - virtual void OnReceive(std::unique_ptr &&record) noexcept = 0; + virtual void OnReceive(std::unique_ptr &&record) noexcept = 0; /** * Exports all log records that have not yet been exported to the configured Exporter. diff --git a/sdk/include/opentelemetry/sdk/logs/simple_log_processor.h b/sdk/include/opentelemetry/sdk/logs/simple_log_processor.h index 18ed28b2cf..38a4194c16 100644 --- a/sdk/include/opentelemetry/sdk/logs/simple_log_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/simple_log_processor.h @@ -43,7 +43,9 @@ class SimpleLogProcessor : public LogProcessor explicit SimpleLogProcessor(std::unique_ptr &&exporter); virtual ~SimpleLogProcessor() = default; - void OnReceive(std::unique_ptr &&record) noexcept override; + std::unique_ptr MakeRecordable() noexcept override; + + void OnReceive(std::unique_ptr &&record) noexcept override; bool ForceFlush( std::chrono::microseconds timeout = std::chrono::microseconds::max()) noexcept override; diff --git a/sdk/src/logs/simple_log_processor.cc b/sdk/src/logs/simple_log_processor.cc index d88fc9eb92..29ca2ccfc0 100644 --- a/sdk/src/logs/simple_log_processor.cc +++ b/sdk/src/logs/simple_log_processor.cc @@ -32,14 +32,19 @@ SimpleLogProcessor::SimpleLogProcessor(std::unique_ptr &&exporter) : exporter_(std::move(exporter)) {} +std::unique_ptr SimpleLogProcessor::MakeRecordable() noexcept +{ + return exporter_->MakeRecordable(); +} + /** * Batches the log record it receives in a batch of 1 and immediately sends it * to the configured exporter */ void SimpleLogProcessor::OnReceive( - std::unique_ptr &&record) noexcept + std::unique_ptr &&record) noexcept { - nostd::span> batch(&record, 1); + nostd::span> batch(&record, 1); // Get lock to ensure Export() is never called concurrently const std::lock_guard locked(lock_); diff --git a/sdk/test/logs/simple_log_processor_test.cc b/sdk/test/logs/simple_log_processor_test.cc index 80c83183d3..ad2d33c6d5 100644 --- a/sdk/test/logs/simple_log_processor_test.cc +++ b/sdk/test/logs/simple_log_processor_test.cc @@ -26,7 +26,7 @@ class TestExporter final : public LogExporter // Stores the names of the log records this exporter receives to an internal list ExportResult Export( - const opentelemetry::nostd::span> &records) noexcept override + const opentelemetry::nostd::span> &records) noexcept override { *batch_size_received = records.size(); for (auto &record : records) @@ -116,7 +116,7 @@ class FailShutDownExporter final : public LogExporter FailShutDownExporter() {} ExportResult Export( - const opentelemetry::nostd::span> &records) noexcept override + const opentelemetry::nostd::span> &records) noexcept override { return ExportResult::kSuccess; } From 029cc40fceea67176eefd761a9afd5ed0aef897e Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Thu, 10 Dec 2020 01:55:20 -0500 Subject: [PATCH 05/21] Update API (Add severity.h file, remove api::LogRecord, update api::Logger) --- api/include/opentelemetry/logs/log_record.h | 125 ----------------- api/include/opentelemetry/logs/logger.h | 148 +++++++++++++------- api/include/opentelemetry/logs/noop.h | 16 ++- api/include/opentelemetry/logs/severity.h | 57 ++++++++ sdk/include/opentelemetry/sdk/logs/logger.h | 1 - 5 files changed, 170 insertions(+), 177 deletions(-) delete mode 100644 api/include/opentelemetry/logs/log_record.h create mode 100644 api/include/opentelemetry/logs/severity.h diff --git a/api/include/opentelemetry/logs/log_record.h b/api/include/opentelemetry/logs/log_record.h deleted file mode 100644 index c663c1c9c7..0000000000 --- a/api/include/opentelemetry/logs/log_record.h +++ /dev/null @@ -1,125 +0,0 @@ -/* - * 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 -#include -#include "opentelemetry/common/key_value_iterable_view.h" -#include "opentelemetry/core/timestamp.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 -{ - -/* 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 -{ - 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 - "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. - * (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 - core::SystemTimestamp timestamp; // uint64 nanoseconds since Unix epoch - trace::TraceId trace_id; // byte sequence - trace::SpanId span_id; // byte sequence - trace::TraceFlags trace_flag; // byte - Severity severity; // Severity enum that combines severity_text and severity_number in the - // LogDataModel (can separate in SDK) - - // other fields that will not be set by default - nostd::string_view name; // string - nostd::string_view body; // currently a simple string, but should be changed "Any" type - common::KeyValueIterable &resource; // key/value pair list - common::KeyValueIterable &attributes; // key/value pair list - - /* Default log record if user does not overwrite this. - * TODO: find better data type to represent the type for "body" - * Future enhancement: Potentially add other constructors to take default arguments - * from the user - **/ - LogRecord() : resource(_nullKV), attributes(_nullKV) - { - // 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 - * resources field */ - template ::value> * = nullptr> - inline void SetResource(const T &_resource) - { - resource = common::KeyValueIterableView(_resource); - } - - /* 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) - { - attributes = common::KeyValueIterableView(_attributes); - } -}; -} // namespace logs -OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index 5f22066460..c90ca32fcb 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -20,11 +20,17 @@ #include #include +#include "opentelemetry/common/attribute_value.h" #include "opentelemetry/common/key_value_iterable.h" -#include "opentelemetry/logs/log_record.h" +#include "opentelemetry/common/key_value_iterable_view.h" +#include "opentelemetry/core/timestamp.h" +#include "opentelemetry/logs/severity.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/span.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 @@ -35,6 +41,17 @@ namespace logs **/ class Logger { +protected: + // Default values to set for fields that the user doesn't specify + core::SystemTimestamp default_timestamp_; // uint64 nanoseconds since Unix epoch, default 0 + Severity default_severity_; // default 0 (kInvalid) + nostd::string_view default_name_; // default 0 length, nullptr data + nostd::string_view default_body_; // default 0 length, nullptr data. TODO: currently a simple + // string, but should be changed "Any" type + trace::TraceId default_trace_id_; // default 00000000000000000000000000000000 + trace::SpanId default_span_id_; // default 0000000000000000 + trace::TraceFlags default_trace_flags_; // default 00 + public: virtual ~Logger() = default; @@ -43,81 +60,116 @@ class Logger // the specification. virtual nostd::string_view getName() = 0; /** - * Each of the following overloaded log(...) methods + * Each of the following overloaded Log(...) methods * creates a log message with the specific parameters passed. * + * @param timestamp the timestamp the log record was created. * @param name the name of the log event. * @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. + * @param attributes the attributes, stored as a 2D list of key/value pairs, that are associated wtih the log event. + * @param trace_id the trace id associated with the log event. + * @param span_id the span id associate with the log event. + * @param trace_flags the trace flags associated with the log event. * @throws No exceptions under any circumstances. */ - /* The below method is a logging statement that takes in a LogRecord. - * A default LogRecord that will be assigned if no parameters are passed to Logger's .log() method - * which should at minimum assign the trace_id, span_id, and timestamp + /** + * The base Log(...) method that all other Log(...) overloaded methods will eventually call, + * in order to create a log record. + * + * Note this takes in a KeyValueIterable for the resource and attributes fields. */ - virtual void log(const LogRecord &record) noexcept = 0; - - /** Overloaded methods for unstructured logging **/ - inline void log(nostd::string_view message) noexcept + virtual void Log(core::SystemTimestamp timestamp, + Severity severity, + nostd::string_view name, + nostd::string_view body, + const common::KeyValueIterable & resource, + const common::KeyValueIterable & attributes, + trace::TraceId trace_id, + trace::SpanId span_id, + trace::TraceFlags trace_flags) noexcept = 0; + + /*** Overloaded methods for KeyValueIterables ***/ + /** + * The secondary base Log(...) method that all other Log(...) overloaded methods except the one above + * will eventually call, in order to create a log record. + * + * Note this takes in template types for the resource and attributes fields. + */ + template ::value> * = nullptr, + nostd::enable_if_t::value> * = nullptr> + void Log(core::SystemTimestamp timestamp, + Severity severity, + nostd::string_view name, + nostd::string_view body, + const T &resources, + const U &attributes, + trace::TraceId trace_id, + trace::SpanId span_id, + trace::TraceFlags trace_flags) noexcept { - // Set severity to the default then call log(Severity, String message) method - log(Severity::kDefault, message); + Log(timestamp, severity, name, body, common::KeyValueIterableView(resources), + common::KeyValueIterableView(attributes), trace_id, span_id, trace_flags); } - inline void log(Severity severity, nostd::string_view message) noexcept + void Log(core::SystemTimestamp timestamp, + Severity severity, + nostd::string_view name, + nostd::string_view body, + std::initializer_list> resource, + std::initializer_list> attributes, + trace::TraceId trace_id, + trace::SpanId span_id, + trace::TraceFlags trace_flags) noexcept { - // TODO: set default timestamp later (not in API) - log(severity, message, core::SystemTimestamp(std::chrono::system_clock::now())); + return this->Log( + timestamp, severity, name, body, + nostd::span>{resource.begin(), + resource.end()}, + nostd::span>{attributes.begin(), + attributes.end()}, trace_id, span_id, trace_flags); } - inline void log(Severity severity, + /** Wrapper methods that the user could call for convenience when logging **/ + + // Set default values for unspecified fields, then call the base Log() method + inline void Log(Severity severity, nostd::string_view message, core::SystemTimestamp time) noexcept { - // creates a LogRecord object with given parameters, then calls log(LogRecord) - LogRecord r; - r.severity = severity; - r.body = message; - r.timestamp = time; - - log(r); + this->Log(time, severity, default_name_, message, {}, {}, default_trace_id_, default_span_id_, default_trace_flags_); } - /** 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> - inline void log(Severity severity = Severity::kDefault, - nostd::string_view name = "", - const T &attributes = {}) noexcept + // Set default time, and call base Log(severity, message, time) method + inline void Log(Severity severity, nostd::string_view message) noexcept { - log(severity, name, common::KeyValueIterableView(attributes)); + this->Log(severity, message, default_timestamp_); } - inline void log(Severity severity, - nostd::string_view name, - const common::KeyValueIterable &attributes) noexcept + // Set default severity then call Log(Severity, String message) method + inline void Log(nostd::string_view message) noexcept { - // creates a LogRecord object with given parameters, then calls log(LogRecord) - LogRecord r; - r.severity = severity; - r.name = name; - r.attributes = attributes; - - log(r); + this->Log(default_severity_, message); } - // TODO: add function aliases such as void debug(), void trace(), void info(), etc. for each + // Set default time, body, resources, traceid, spanid, and traceflags, + // and then call Log(Severity, String message) method + // inline void Log(Severity severity, + // nostd::string_view name, + // const common::KeyValueIterable &attributes) noexcept + // { + // this->Log(default_timestamp_, severity, name, default_body_, default_resource_, attributes, default_trace_id_, default_span_id_, default_trace_flags_); + // } + + // TODO: Add more overloaded Log(...) methods with different combiantions of parameters. + + // 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 virtual void log(T &some_obj) noexcept; }; } // namespace logs -OPENTELEMETRY_END_NAMESPACE \ No newline at end of file +OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/logs/noop.h b/api/include/opentelemetry/logs/noop.h index 3184baa3c6..c005fb6262 100644 --- a/api/include/opentelemetry/logs/noop.h +++ b/api/include/opentelemetry/logs/noop.h @@ -25,8 +25,19 @@ #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/common/attribute_value.h" +#include "opentelemetry/common/key_value_iterable.h" +#include "opentelemetry/core/timestamp.h" +#include "opentelemetry/logs/severity.h" +#include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/nostd/span.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" + #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -39,9 +50,8 @@ namespace logs class NoopLogger final : public Logger { public: - NoopLogger() = default; - void log(const LogRecord &record) noexcept override {} + void Log(core::SystemTimestamp timestamp, Severity severity, nostd::string_view name, nostd::string_view body, const common::KeyValueIterable &resource, const common::KeyValueIterable &attributes, trace::TraceId trace_id, trace::SpanId span_id, trace::TraceFlags trace_flags) noexcept override{} }; /** diff --git a/api/include/opentelemetry/logs/severity.h b/api/include/opentelemetry/logs/severity.h new file mode 100644 index 0000000000..c66b77a190 --- /dev/null +++ b/api/include/opentelemetry/logs/severity.h @@ -0,0 +1,57 @@ +/* + * 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/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace logs +{ + +/** + * Severity Levels assigned to log events, based on Log Data Model + */ +enum class Severity : uint8_t +{ + kInvalid = 0, + 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 +}; + +} // namespace logs +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/logs/logger.h b/sdk/include/opentelemetry/sdk/logs/logger.h index 4b49351030..9494dbf152 100644 --- a/sdk/include/opentelemetry/sdk/logs/logger.h +++ b/sdk/include/opentelemetry/sdk/logs/logger.h @@ -16,7 +16,6 @@ #pragma once -#include "opentelemetry/logs/log_record.h" #include "opentelemetry/logs/logger.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/string_view.h" From e3dbf3ada252d56351e4f496bb5e202d419cff98 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Thu, 10 Dec 2020 06:05:50 -0500 Subject: [PATCH 06/21] Update SDK (Logger and LoggerProvider) --- sdk/include/opentelemetry/sdk/logs/logger.h | 16 +++- .../opentelemetry/sdk/logs/logger_provider.h | 1 + sdk/src/logs/BUILD | 1 + sdk/src/logs/CMakeLists.txt | 2 +- sdk/src/logs/logger.cc | 88 ++++++++++++++++--- 5 files changed, 89 insertions(+), 19 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/logs/logger.h b/sdk/include/opentelemetry/sdk/logs/logger.h index 9494dbf152..1724e9e92c 100644 --- a/sdk/include/opentelemetry/sdk/logs/logger.h +++ b/sdk/include/opentelemetry/sdk/logs/logger.h @@ -44,10 +44,18 @@ class Logger final : public opentelemetry::logs::Logger /** * Writes a log record into the processor. - * @param record The record to write into the processor. - */ - void log(const opentelemetry::logs::LogRecord &record) noexcept override; - + * @param timestamp the timestamp the log record was created. + * @param name the name of the log event. + * @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 wtih the log event. + * @param trace_id the trace id associated with the log event. + * @param span_id the span id associate with the log event. + * @param trace_flags the trace flags associated with the log event. + * @throws No exceptions under any circumstances. */ + void Log(core::SystemTimestamp timestamp, opentelemetry::logs::Severity severity, nostd::string_view name, nostd::string_view body, const common::KeyValueIterable &resource, const common::KeyValueIterable &attributes, trace::TraceId trace_id, trace::SpanId span_id, trace::TraceFlags trace_flags) noexcept override; + private: // The logger provider of this Logger. Uses a weak_ptr to avoid cyclic dependancy issues the with // logger provider diff --git a/sdk/include/opentelemetry/sdk/logs/logger_provider.h b/sdk/include/opentelemetry/sdk/logs/logger_provider.h index fdba956624..a79f486713 100644 --- a/sdk/include/opentelemetry/sdk/logs/logger_provider.h +++ b/sdk/include/opentelemetry/sdk/logs/logger_provider.h @@ -20,6 +20,7 @@ #include #include #include +#include #include "opentelemetry/logs/logger_provider.h" #include "opentelemetry/logs/noop.h" diff --git a/sdk/src/logs/BUILD b/sdk/src/logs/BUILD index c1a0542fa1..4829b3838f 100644 --- a/sdk/src/logs/BUILD +++ b/sdk/src/logs/BUILD @@ -22,5 +22,6 @@ cc_library( deps = [ "//api", "//sdk:headers", + "//sdk/src/trace", ], ) diff --git a/sdk/src/logs/CMakeLists.txt b/sdk/src/logs/CMakeLists.txt index e2e7c2c915..e0e4d6f000 100644 --- a/sdk/src/logs/CMakeLists.txt +++ b/sdk/src/logs/CMakeLists.txt @@ -1,4 +1,4 @@ add_library(opentelemetry_logs logger_provider.cc logger.cc simple_log_processor.cc) -target_link_libraries(opentelemetry_logs opentelemetry_common) +target_link_libraries(opentelemetry_logs opentelemetry_common opentelemetry_trace) diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index bc19f5d1c1..cf1ee9b91d 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -15,6 +15,8 @@ */ #include "opentelemetry/sdk/logs/logger.h" + #include "opentelemetry/sdk/logs/log_record.h" + #include "opentelemetry/trace/provider.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk @@ -25,7 +27,12 @@ Logger::Logger(std::shared_ptr logger_provider) noexcept : logger_provider_(logger_provider) {} -void Logger::log(const opentelemetry::logs::LogRecord &record) noexcept +/** + * Create and popualte recordable with the log event's fields passed in. + * The timestamp, severity, traceid, spanid, and traceflags, are injected + * if the user does not specify them. + */ +void Logger::Log(core::SystemTimestamp timestamp, opentelemetry::logs::Severity severity, nostd::string_view name, nostd::string_view body, const common::KeyValueIterable &resource, const common::KeyValueIterable &attributes, opentelemetry::trace::TraceId trace_id, opentelemetry::trace::SpanId span_id, opentelemetry::trace::TraceFlags trace_flags) noexcept { // If this logger does not have a processor, no need to create a log record auto processor = logger_provider_.lock()->GetProcessor(); @@ -36,23 +43,76 @@ void Logger::log(const opentelemetry::logs::LogRecord &record) noexcept // TODO: Sampler logic (should include check for minSeverity) - /** - * Convert the LogRecord to the heap first before sending to processor. - * TODO: Change the API log(LogRecord) function to log(*LogRecord) so the following line - * converting record a heap variable can be removed - */ - auto record_pointer = - std::unique_ptr(new opentelemetry::logs::LogRecord(record)); + auto recordable = processor->MakeRecordable(); + if (recordable == nullptr) + { + return; + } + + // Populate recordable fields + if (timestamp == this->default_timestamp_){ + // Inject current timestamp if none is specified by user + recordable->SetTimestamp(core::SystemTimestamp(std::chrono::system_clock::now())); + } + else + { + recordable->SetTimestamp(timestamp); + } - // TODO: Do not want to overwrite user-set timestamp if there already is one - - // add a flag in the API to check if timestamp is set by user already before setting timestamp + recordable->SetSeverity(severity); + recordable->SetName(name); + recordable->SetBody(body); - // Inject timestamp if none is set - record_pointer->timestamp = core::SystemTimestamp(std::chrono::system_clock::now()); - // TODO: inject traceid/spanid later + resource.ForEachKeyValue([&](nostd::string_view key, + opentelemetry::common::AttributeValue value) noexcept { + recordable->SetResource(key, value); + return true; + }); + + attributes.ForEachKeyValue([&](nostd::string_view key, + opentelemetry::common::AttributeValue value) noexcept { + recordable->SetAttribute(key, value); + return true; + }); + + + // Inject trace_id/span_id/trace_flags if none is set by user + auto provider = opentelemetry::trace::Provider::GetTracerProvider(); + auto tracer = provider->GetTracer("foo_library"); + auto span_context = tracer->GetCurrentSpan()->GetContext(); + + // Traceid + if (!trace_id.IsValid()) + { + recordable->SetTraceId(span_context.trace_id()); + } + else + { + recordable->SetTraceId(trace_id); + } + + // Spanid + if (!span_id.IsValid()) + { + recordable->SetSpanId(span_context.span_id()); + } + else + { + recordable->SetSpanId(span_id); + } + + // Traceflag + if (!trace_flags.IsSampled()) + { + recordable->SetTraceFlags(span_context.trace_flags()); + } + else + { + recordable->SetTraceFlags(trace_flags); + } // Send the log record to the processor - processor->OnReceive(std::move(record_pointer)); + processor->OnReceive(std::move(recordable)); } } // namespace logs From e198ae47a4ea695b25255fb3740e6747a8f5979d Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Thu, 10 Dec 2020 06:44:35 -0500 Subject: [PATCH 07/21] Refactor tests --- api/test/logs/logger_test.cc | 9 ++--- sdk/test/logs/logger_provider_sdk_test.cc | 7 +++- sdk/test/logs/logger_sdk_test.cc | 18 +++++----- sdk/test/logs/simple_log_processor_test.cc | 42 ++++++++++++++-------- 4 files changed, 44 insertions(+), 32 deletions(-) diff --git a/api/test/logs/logger_test.cc b/api/test/logs/logger_test.cc index 937ece860a..40682871ee 100644 --- a/api/test/logs/logger_test.cc +++ b/api/test/logs/logger_test.cc @@ -1,14 +1,13 @@ #include #include +#include "opentelemetry/core/timestamp.h" #include "opentelemetry/logs/logger.h" #include "opentelemetry/logs/provider.h" #include "opentelemetry/nostd/shared_ptr.h" -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; @@ -43,15 +42,13 @@ TEST(Logger, NoopLog) { auto lp = Provider::GetLoggerProvider(); auto logger = lp->GetLogger("TestLogger"); - LogRecord r; - r.name = "Noop log name"; - logger->log(r); + logger->Log("Noop log name"); } // Define a basic Logger class class TestLogger : public Logger { - void log(const LogRecord &record) noexcept override {} + void Log(opentelemetry::core::SystemTimestamp timestamp, Severity severity, string_view name, string_view body, const opentelemetry::common::KeyValueIterable &resource, const opentelemetry::common::KeyValueIterable &attributes, opentelemetry::trace::TraceId trace_id, opentelemetry::trace::SpanId span_id, opentelemetry::trace::TraceFlags trace_flags) noexcept override{} }; // Define a basic LoggerProvider class that returns an instance of the logger class defined above diff --git a/sdk/test/logs/logger_provider_sdk_test.cc b/sdk/test/logs/logger_provider_sdk_test.cc index 02e7c47275..6c7c825795 100644 --- a/sdk/test/logs/logger_provider_sdk_test.cc +++ b/sdk/test/logs/logger_provider_sdk_test.cc @@ -18,6 +18,7 @@ #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/sdk/logs/logger.h" #include "opentelemetry/sdk/logs/logger_provider.h" +#include "opentelemetry/sdk/logs/log_record.h" #include @@ -69,7 +70,11 @@ TEST(LoggerProviderSDK, LoggerProviderLoggerArguments) class DummyProcessor : public LogProcessor { - void OnReceive(std::unique_ptr &&record) noexcept {} + std::unique_ptr MakeRecordable() noexcept { + return std::unique_ptr(new LogRecord); + } + + void OnReceive(std::unique_ptr &&record) noexcept {} bool ForceFlush(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept { return true; diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index d4825d0032..bb98dae84d 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -15,6 +15,7 @@ */ #include "opentelemetry/sdk/logs/logger.h" +#include "opentelemetry/sdk/logs/log_record.h" #include @@ -22,7 +23,7 @@ using namespace opentelemetry::sdk::logs; TEST(LoggerSDK, LogToNullProcessor) { - // Confirm Logger::log() does not have undefined behavior + // Confirm Logger::Log() does not have undefined behavior // even when there is no processor set // since it calls Processor::OnReceive() @@ -30,14 +31,16 @@ TEST(LoggerSDK, LogToNullProcessor) auto logger = lp->GetLogger("logger"); // Log a sample log record to a nullptr processor - opentelemetry::logs::LogRecord r; - r.name = "Test log"; - logger->log(r); + logger->Log("Test log"); } class DummyProcessor : public LogProcessor { - void OnReceive(std::unique_ptr &&record) noexcept {} + std::unique_ptr MakeRecordable() noexcept + { + return std::unique_ptr(new LogRecord); + } + void OnReceive(std::unique_ptr &&record) noexcept {} bool ForceFlush(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept { return true; @@ -69,9 +72,4 @@ TEST(LoggerSDK, LogToAProcessor) // Should later introduce a way to assert that // the logger's processor is the same as "proc" // and that the logger's processor is the same as lp's processor - - // Log a sample log record to the processor - opentelemetry::logs::LogRecord r; - r.name = "Test log"; - logger->log(r); } diff --git a/sdk/test/logs/simple_log_processor_test.cc b/sdk/test/logs/simple_log_processor_test.cc index ad2d33c6d5..3ee78c97d8 100644 --- a/sdk/test/logs/simple_log_processor_test.cc +++ b/sdk/test/logs/simple_log_processor_test.cc @@ -1,5 +1,7 @@ #include "opentelemetry/sdk/logs/simple_log_processor.h" #include "opentelemetry/sdk/logs/exporter.h" +#include "opentelemetry/sdk/logs/log_record.h" +#include "opentelemetry/nostd/span.h" #include @@ -7,7 +9,6 @@ #include using namespace opentelemetry::sdk::logs; -using opentelemetry::logs::LogRecord; /* * A test exporter that can return a vector of all the records it has received, @@ -17,13 +18,18 @@ class TestExporter final : public LogExporter { public: TestExporter(int *shutdown_counter, - std::shared_ptr> logs_received, + std::shared_ptr>> logs_received, size_t *batch_size_received) : shutdown_counter_(shutdown_counter), logs_received_(logs_received), batch_size_received(batch_size_received) {} + std::unique_ptr MakeRecordable() noexcept override + { + return std::unique_ptr(new LogRecord()); + } + // Stores the names of the log records this exporter receives to an internal list ExportResult Export( const opentelemetry::nostd::span> &records) noexcept override @@ -31,7 +37,13 @@ class TestExporter final : public LogExporter *batch_size_received = records.size(); for (auto &record : records) { - logs_received_->push_back(record->name.data()); + auto log_record = std::unique_ptr( + static_cast(record.release())); + + if (log_record != nullptr) + { + logs_received_->push_back(std::move(log_record)); + } } return ExportResult::kSuccess; } @@ -45,7 +57,7 @@ class TestExporter final : public LogExporter private: int *shutdown_counter_; - std::shared_ptr> logs_received_; + std::shared_ptr>> logs_received_; size_t *batch_size_received; }; @@ -54,7 +66,7 @@ class TestExporter final : public LogExporter TEST(SimpleLogProcessorTest, SendReceivedLogsToExporter) { // Create a simple processor with a TestExporter attached - std::shared_ptr> logs_received(new std::vector); + std::shared_ptr>> logs_received(new std::vector>); size_t batch_size_received = 0; std::unique_ptr exporter( @@ -66,24 +78,19 @@ TEST(SimpleLogProcessorTest, SendReceivedLogsToExporter) const int num_logs = 5; for (int i = 0; i < num_logs; i++) { - auto record = std::unique_ptr(new LogRecord()); - std::string s("Log name"); - s += std::to_string(i); - record->name = s; - - processor.OnReceive(std::move(record)); + auto recordable = processor.MakeRecordable(); + recordable->SetName("Log"); + processor.OnReceive(std::move(recordable)); // Verify that the batch of 1 log record sent by processor matches what exporter received EXPECT_EQ(1, batch_size_received); } // Test whether the processor's log sent matches the log record received by the exporter - EXPECT_EQ(logs_received->size(), num_logs); + EXPECT_EQ(logs_received->size(), num_logs); for (int i = 0; i < num_logs; i++) { - std::string s("Log name"); - s += std::to_string(i); - EXPECT_EQ(s, logs_received->at(i)); + EXPECT_EQ("Log", logs_received->at(i)->GetName()); } } @@ -115,6 +122,11 @@ class FailShutDownExporter final : public LogExporter public: FailShutDownExporter() {} + std::unique_ptr MakeRecordable() noexcept override + { + return std::unique_ptr(new LogRecord()); + } + ExportResult Export( const opentelemetry::nostd::span> &records) noexcept override { From e7a78c2a7eee10a09ed3c7df3836ef8695617c6a Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Thu, 10 Dec 2020 07:44:13 -0500 Subject: [PATCH 08/21] Format --- api/include/opentelemetry/logs/logger.h | 105 +++++++++--------- api/include/opentelemetry/logs/noop.h | 20 +++- api/test/logs/logger_test.cc | 11 +- .../opentelemetry/sdk/logs/attribute_utils.h | 26 ++--- sdk/include/opentelemetry/sdk/logs/exporter.h | 3 +- .../opentelemetry/sdk/logs/log_record.h | 58 ++++------ sdk/include/opentelemetry/sdk/logs/logger.h | 15 ++- .../opentelemetry/sdk/logs/logger_provider.h | 2 +- .../opentelemetry/sdk/logs/processor.h | 3 +- .../opentelemetry/sdk/logs/recordable.h | 15 ++- sdk/src/logs/CMakeLists.txt | 3 +- sdk/src/logs/logger.cc | 40 ++++--- sdk/src/logs/simple_log_processor.cc | 5 +- sdk/test/logs/logger_provider_sdk_test.cc | 5 +- sdk/test/logs/logger_sdk_test.cc | 4 +- sdk/test/logs/simple_log_processor_test.cc | 10 +- 16 files changed, 172 insertions(+), 153 deletions(-) diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index c90ca32fcb..bc437432b2 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -42,15 +42,15 @@ namespace logs class Logger { protected: - // Default values to set for fields that the user doesn't specify + // Default values to set for fields that the user doesn't specify core::SystemTimestamp default_timestamp_; // uint64 nanoseconds since Unix epoch, default 0 Severity default_severity_; // default 0 (kInvalid) nostd::string_view default_name_; // default 0 length, nullptr data - nostd::string_view default_body_; // default 0 length, nullptr data. TODO: currently a simple - // string, but should be changed "Any" type - trace::TraceId default_trace_id_; // default 00000000000000000000000000000000 - trace::SpanId default_span_id_; // default 0000000000000000 - trace::TraceFlags default_trace_flags_; // default 00 + nostd::string_view default_body_; // default 0 length, nullptr data. TODO: currently a simple + // string, but should be changed "Any" type + trace::TraceId default_trace_id_; // default 00000000000000000000000000000000 + trace::SpanId default_span_id_; // default 0000000000000000 + trace::TraceFlags default_trace_flags_; // default 00 public: virtual ~Logger() = default; @@ -68,7 +68,8 @@ class Logger * @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 wtih the log event. + * @param attributes the attributes, stored as a 2D list of key/value pairs, that are associated + * wtih the log event. * @param trace_id the trace id associated with the log event. * @param span_id the span id associate with the log event. * @param trace_flags the trace flags associated with the log event. @@ -78,83 +79,81 @@ class Logger /** * The base Log(...) method that all other Log(...) overloaded methods will eventually call, * in order to create a log record. - * + * * Note this takes in a KeyValueIterable for the resource and attributes fields. */ - virtual void Log(core::SystemTimestamp timestamp, - Severity severity, - nostd::string_view name, - nostd::string_view body, - const common::KeyValueIterable & resource, - const common::KeyValueIterable & attributes, - trace::TraceId trace_id, - trace::SpanId span_id, + virtual void Log(core::SystemTimestamp timestamp, + Severity severity, + nostd::string_view name, + nostd::string_view body, + const common::KeyValueIterable &resource, + const common::KeyValueIterable &attributes, + trace::TraceId trace_id, + trace::SpanId span_id, trace::TraceFlags trace_flags) noexcept = 0; - /*** Overloaded methods for KeyValueIterables ***/ + /*** Overloaded methods for KeyValueIterables ***/ /** - * The secondary base Log(...) method that all other Log(...) overloaded methods except the one above - * will eventually call, in order to create a log record. - * + * The secondary base Log(...) method that all other Log(...) overloaded methods except the one + * above will eventually call, in order to create a log record. + * * Note this takes in template types for the resource and attributes fields. */ template ::value> * = nullptr, nostd::enable_if_t::value> * = nullptr> - void Log(core::SystemTimestamp timestamp, - Severity severity, - nostd::string_view name, - nostd::string_view body, - const T &resources, - const U &attributes, - trace::TraceId trace_id, - trace::SpanId span_id, - trace::TraceFlags trace_flags) noexcept + void Log(core::SystemTimestamp timestamp, + Severity severity, + nostd::string_view name, + nostd::string_view body, + const T &resources, + const U &attributes, + trace::TraceId trace_id, + trace::SpanId span_id, + trace::TraceFlags trace_flags) noexcept { Log(timestamp, severity, name, body, common::KeyValueIterableView(resources), - common::KeyValueIterableView(attributes), trace_id, span_id, trace_flags); + common::KeyValueIterableView(attributes), trace_id, span_id, trace_flags); } void Log(core::SystemTimestamp timestamp, - Severity severity, - nostd::string_view name, - nostd::string_view body, - std::initializer_list> resource, - std::initializer_list> attributes, - trace::TraceId trace_id, - trace::SpanId span_id, - trace::TraceFlags trace_flags) noexcept + Severity severity, + nostd::string_view name, + nostd::string_view body, + std::initializer_list> resource, + std::initializer_list> attributes, + trace::TraceId trace_id, + trace::SpanId span_id, + trace::TraceFlags trace_flags) noexcept { - return this->Log( - timestamp, severity, name, body, - nostd::span>{resource.begin(), - resource.end()}, - nostd::span>{attributes.begin(), - attributes.end()}, trace_id, span_id, trace_flags); + return this->Log(timestamp, severity, name, body, + nostd::span>{ + resource.begin(), resource.end()}, + nostd::span>{ + attributes.begin(), attributes.end()}, + trace_id, span_id, trace_flags); } /** Wrapper methods that the user could call for convenience when logging **/ - + // Set default values for unspecified fields, then call the base Log() method inline void Log(Severity severity, nostd::string_view message, core::SystemTimestamp time) noexcept { - this->Log(time, severity, default_name_, message, {}, {}, default_trace_id_, default_span_id_, default_trace_flags_); + this->Log(time, severity, default_name_, message, {}, {}, default_trace_id_, default_span_id_, + default_trace_flags_); } // Set default time, and call base Log(severity, message, time) method inline void Log(Severity severity, nostd::string_view message) noexcept { - this->Log(severity, message, default_timestamp_); + this->Log(severity, message, default_timestamp_); } // Set default severity then call Log(Severity, String message) method - inline void Log(nostd::string_view message) noexcept - { - this->Log(default_severity_, message); - } + inline void Log(nostd::string_view message) noexcept { this->Log(default_severity_, message); } // Set default time, body, resources, traceid, spanid, and traceflags, // and then call Log(Severity, String message) method @@ -162,14 +161,14 @@ class Logger // nostd::string_view name, // const common::KeyValueIterable &attributes) noexcept // { - // this->Log(default_timestamp_, severity, name, default_body_, default_resource_, attributes, default_trace_id_, default_span_id_, default_trace_flags_); + // this->Log(default_timestamp_, severity, name, default_body_, default_resource_, attributes, + // default_trace_id_, default_span_id_, default_trace_flags_); // } // TODO: Add more overloaded Log(...) methods with different combiantions of parameters. // TODO: Add function aliases such as void debug(), void trace(), void info(), etc. for each // severity level - }; } // namespace logs OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/logs/noop.h b/api/include/opentelemetry/logs/noop.h index c005fb6262..0ef6806b71 100644 --- a/api/include/opentelemetry/logs/noop.h +++ b/api/include/opentelemetry/logs/noop.h @@ -22,17 +22,17 @@ #include -#include "opentelemetry/context/runtime_context.h" -#include "opentelemetry/logs/logger.h" -#include "opentelemetry/logs/logger_provider.h" -#include "opentelemetry/nostd/unique_ptr.h" #include "opentelemetry/common/attribute_value.h" #include "opentelemetry/common/key_value_iterable.h" +#include "opentelemetry/context/runtime_context.h" #include "opentelemetry/core/timestamp.h" +#include "opentelemetry/logs/logger.h" +#include "opentelemetry/logs/logger_provider.h" #include "opentelemetry/logs/severity.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/span.h" #include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/nostd/unique_ptr.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_flags.h" #include "opentelemetry/trace/trace_id.h" @@ -50,8 +50,16 @@ namespace logs class NoopLogger final : public Logger { public: - - void Log(core::SystemTimestamp timestamp, Severity severity, nostd::string_view name, nostd::string_view body, const common::KeyValueIterable &resource, const common::KeyValueIterable &attributes, trace::TraceId trace_id, trace::SpanId span_id, trace::TraceFlags trace_flags) noexcept override{} + void Log(core::SystemTimestamp timestamp, + Severity severity, + nostd::string_view name, + nostd::string_view body, + const common::KeyValueIterable &resource, + const common::KeyValueIterable &attributes, + trace::TraceId trace_id, + trace::SpanId span_id, + trace::TraceFlags trace_flags) noexcept override + {} }; /** diff --git a/api/test/logs/logger_test.cc b/api/test/logs/logger_test.cc index 40682871ee..8013a8211d 100644 --- a/api/test/logs/logger_test.cc +++ b/api/test/logs/logger_test.cc @@ -48,7 +48,16 @@ TEST(Logger, NoopLog) // Define a basic Logger class class TestLogger : public Logger { - void Log(opentelemetry::core::SystemTimestamp timestamp, Severity severity, string_view name, string_view body, const opentelemetry::common::KeyValueIterable &resource, const opentelemetry::common::KeyValueIterable &attributes, opentelemetry::trace::TraceId trace_id, opentelemetry::trace::SpanId span_id, opentelemetry::trace::TraceFlags trace_flags) noexcept override{} + void Log(opentelemetry::core::SystemTimestamp timestamp, + Severity severity, + string_view name, + string_view body, + const opentelemetry::common::KeyValueIterable &resource, + const opentelemetry::common::KeyValueIterable &attributes, + opentelemetry::trace::TraceId trace_id, + opentelemetry::trace::SpanId span_id, + opentelemetry::trace::TraceFlags trace_flags) noexcept override + {} }; // Define a basic LoggerProvider class that returns an instance of the logger class defined above diff --git a/sdk/include/opentelemetry/sdk/logs/attribute_utils.h b/sdk/include/opentelemetry/sdk/logs/attribute_utils.h index e74d09a5a5..17b6ec01e0 100644 --- a/sdk/include/opentelemetry/sdk/logs/attribute_utils.h +++ b/sdk/include/opentelemetry/sdk/logs/attribute_utils.h @@ -15,19 +15,19 @@ namespace logs * replaces all non-owning references with owned copies. */ using LogRecordAttributeValue = nostd::variant, - std::vector, - std::vector, - std::vector, - std::vector, - std::vector, - std::vector>; + int32_t, + uint32_t, + int64_t, + uint64_t, + double, + std::string, + std::vector, + std::vector, + std::vector, + std::vector, + std::vector, + std::vector, + std::vector>; /** * Creates an owned copy (LogRecordAttributeValue) of a non-owning AttributeValue. diff --git a/sdk/include/opentelemetry/sdk/logs/exporter.h b/sdk/include/opentelemetry/sdk/logs/exporter.h index 8009e373c4..4c01d5dabf 100644 --- a/sdk/include/opentelemetry/sdk/logs/exporter.h +++ b/sdk/include/opentelemetry/sdk/logs/exporter.h @@ -65,8 +65,7 @@ class LogExporter * @param records a span of unique pointers to log records * @returns an ExportResult code (whether export was success or failure) */ - virtual ExportResult Export( - const nostd::span> &records) noexcept = 0; + virtual ExportResult Export(const nostd::span> &records) noexcept = 0; /** * Marks the exporter as ShutDown and cleans up any resources as required. diff --git a/sdk/include/opentelemetry/sdk/logs/log_record.h b/sdk/include/opentelemetry/sdk/logs/log_record.h index 806048f47b..a605bafdf8 100644 --- a/sdk/include/opentelemetry/sdk/logs/log_record.h +++ b/sdk/include/opentelemetry/sdk/logs/log_record.h @@ -18,8 +18,8 @@ #include #include -#include "opentelemetry/sdk/logs/attribute_utils.h" // same as traces/attribute_utils #include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/sdk/logs/attribute_utils.h" // same as traces/attribute_utils #include "opentelemetry/sdk/logs/recordable.h" #include "opentelemetry/version.h" @@ -37,56 +37,46 @@ namespace logs */ class LogRecord final : public Recordable { -private: +private: // Log Data Model fields - core::SystemTimestamp timestamp_; // uint64 nanoseconds since Unix epoch - opentelemetry::trace::TraceId trace_id_; // byte sequence - opentelemetry::trace::SpanId span_id_; // byte sequence - opentelemetry::trace::TraceFlags trace_flags_; // byte - opentelemetry::logs::Severity severity_; // Severity enum - nostd::string_view name_; // string - nostd::string_view body_; // currently a simple string, but should be changed "Any" type + core::SystemTimestamp timestamp_; // uint64 nanoseconds since Unix epoch + opentelemetry::trace::TraceId trace_id_; // byte sequence + opentelemetry::trace::SpanId span_id_; // byte sequence + opentelemetry::trace::TraceFlags trace_flags_; // byte + opentelemetry::logs::Severity severity_; // Severity enum + nostd::string_view name_; // string + nostd::string_view body_; // currently a simple string, but should be changed "Any" type AttributeMap resource_map_; AttributeMap attributes_map_; public: - /********** Setters for each field (overrides methods from the Recordable interface) ************/ /** - * Set the timestamp for this log. + * Set the timestamp for this log. * @param timestamp the timestamp of the event */ - void SetTimestamp(core::SystemTimestamp timestamp) noexcept override - { - timestamp_ = timestamp; - } + void SetTimestamp(core::SystemTimestamp timestamp) noexcept override { timestamp_ = timestamp; } /** - * Set the severity for this log. + * Set the severity for this log. * @param severity the severity of the event */ void SetSeverity(opentelemetry::logs::Severity severity) noexcept override { - severity_ = severity; + severity_ = severity; } - /** + /** * Set name for this log * @param name the name to set - */ - void SetName(nostd::string_view name) noexcept override - { - name_ = name; - } + */ + void SetName(nostd::string_view name) noexcept override { name_ = name; } /** * Set body field for this log. * @param message the body to set - */ - void SetBody(nostd::string_view message) noexcept override - { - body_ = message; - } + */ + void SetBody(nostd::string_view message) noexcept override { body_ = message; } /** * Set a resource for this log. @@ -115,12 +105,12 @@ class LogRecord final : public Recordable * Set trace id for this log. * @param trace_id the trace id to set */ - void SetTraceId(opentelemetry::trace::TraceId trace_id) noexcept override + void SetTraceId(opentelemetry::trace::TraceId trace_id) noexcept override { trace_id_ = trace_id; } - /** + /** * Set span id for this log. * @param span_id the span id to set */ @@ -135,7 +125,7 @@ class LogRecord final : public Recordable */ void SetTraceFlags(opentelemetry::trace::TraceFlags trace_flags) noexcept override { - trace_flags_ = trace_flags; + trace_flags_ = trace_flags; } /************************** Getters for each field ****************************/ @@ -182,7 +172,7 @@ class LogRecord final : public Recordable return attributes_map_.GetAttributes(); } - /** + /** * Get the trace id for this log * @return the trace id for this log */ @@ -199,9 +189,7 @@ class LogRecord final : public Recordable * @return the trace flags for this log */ opentelemetry::trace::TraceFlags GetTraceFlags() const noexcept { return trace_flags_; } - - }; } // namespace logs -} // namespace sdk +} // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/logs/logger.h b/sdk/include/opentelemetry/sdk/logs/logger.h index 1724e9e92c..a4166669b7 100644 --- a/sdk/include/opentelemetry/sdk/logs/logger.h +++ b/sdk/include/opentelemetry/sdk/logs/logger.h @@ -49,13 +49,22 @@ class Logger final : public opentelemetry::logs::Logger * @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 wtih the log event. + * @param attributes the attributes, stored as a 2D list of key/value pairs, that are associated + * wtih the log event. * @param trace_id the trace id associated with the log event. * @param span_id the span id associate with the log event. * @param trace_flags the trace flags associated with the log event. * @throws No exceptions under any circumstances. */ - void Log(core::SystemTimestamp timestamp, opentelemetry::logs::Severity severity, nostd::string_view name, nostd::string_view body, const common::KeyValueIterable &resource, const common::KeyValueIterable &attributes, trace::TraceId trace_id, trace::SpanId span_id, trace::TraceFlags trace_flags) noexcept override; - + void Log(core::SystemTimestamp timestamp, + opentelemetry::logs::Severity severity, + nostd::string_view name, + nostd::string_view body, + const common::KeyValueIterable &resource, + const common::KeyValueIterable &attributes, + trace::TraceId trace_id, + trace::SpanId span_id, + trace::TraceFlags trace_flags) noexcept override; + private: // The logger provider of this Logger. Uses a weak_ptr to avoid cyclic dependancy issues the with // logger provider diff --git a/sdk/include/opentelemetry/sdk/logs/logger_provider.h b/sdk/include/opentelemetry/sdk/logs/logger_provider.h index a79f486713..36f320d62d 100644 --- a/sdk/include/opentelemetry/sdk/logs/logger_provider.h +++ b/sdk/include/opentelemetry/sdk/logs/logger_provider.h @@ -19,8 +19,8 @@ #include #include #include -#include #include +#include #include "opentelemetry/logs/logger_provider.h" #include "opentelemetry/logs/noop.h" diff --git a/sdk/include/opentelemetry/sdk/logs/processor.h b/sdk/include/opentelemetry/sdk/logs/processor.h index 6cb5696ce8..7e0ab6d366 100644 --- a/sdk/include/opentelemetry/sdk/logs/processor.h +++ b/sdk/include/opentelemetry/sdk/logs/processor.h @@ -34,8 +34,7 @@ class LogProcessor public: virtual ~LogProcessor() = default; - -/** + /** * Create a log recordable. This requests a new log recordable from the * associated exporter. * @return a newly initialized recordable diff --git a/sdk/include/opentelemetry/sdk/logs/recordable.h b/sdk/include/opentelemetry/sdk/logs/recordable.h index 3deadb3272..ada25ef2a7 100644 --- a/sdk/include/opentelemetry/sdk/logs/recordable.h +++ b/sdk/include/opentelemetry/sdk/logs/recordable.h @@ -31,27 +31,27 @@ class Recordable virtual ~Recordable() = default; /** - * Set the timestamp for this log. + * Set the timestamp for this log. * @param timestamp the timestamp to set */ virtual void SetTimestamp(core::SystemTimestamp timestamp) noexcept = 0; /** - * Set the severity for this log. + * Set the severity for this log. * @param severity the severity of the event */ virtual void SetSeverity(opentelemetry::logs::Severity severity) noexcept = 0; - /** + /** * Set name for this log * @param name the name to set - */ + */ virtual void SetName(nostd::string_view name) noexcept = 0; /** * Set body field for this log. * @param message the body to set - */ + */ virtual void SetBody(nostd::string_view message) noexcept = 0; /** @@ -60,7 +60,7 @@ class Recordable * @param value the resource value to set */ virtual void SetResource(nostd::string_view key, - const opentelemetry::common::AttributeValue &value) noexcept = 0; + const opentelemetry::common::AttributeValue &value) noexcept = 0; /** * Set an attribute of a log. @@ -76,7 +76,7 @@ class Recordable */ virtual void SetTraceId(opentelemetry::trace::TraceId trace_id) noexcept = 0; - /** + /** * Set the span id for this log. * @param span_id the span id to set */ @@ -87,7 +87,6 @@ class Recordable * @param trace_flags the trace flags to set */ virtual void SetTraceFlags(opentelemetry::trace::TraceFlags trace_flags) noexcept = 0; - }; } // namespace logs } // namespace sdk diff --git a/sdk/src/logs/CMakeLists.txt b/sdk/src/logs/CMakeLists.txt index e0e4d6f000..326558a922 100644 --- a/sdk/src/logs/CMakeLists.txt +++ b/sdk/src/logs/CMakeLists.txt @@ -1,4 +1,5 @@ add_library(opentelemetry_logs logger_provider.cc logger.cc simple_log_processor.cc) -target_link_libraries(opentelemetry_logs opentelemetry_common opentelemetry_trace) +target_link_libraries(opentelemetry_logs opentelemetry_common + opentelemetry_trace) diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index cf1ee9b91d..60e8c7ed57 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -15,8 +15,8 @@ */ #include "opentelemetry/sdk/logs/logger.h" - #include "opentelemetry/sdk/logs/log_record.h" - #include "opentelemetry/trace/provider.h" +#include "opentelemetry/sdk/logs/log_record.h" +#include "opentelemetry/trace/provider.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk @@ -28,11 +28,19 @@ Logger::Logger(std::shared_ptr logger_provider) noexcept {} /** - * Create and popualte recordable with the log event's fields passed in. + * Create and popualte recordable with the log event's fields passed in. * The timestamp, severity, traceid, spanid, and traceflags, are injected * if the user does not specify them. - */ -void Logger::Log(core::SystemTimestamp timestamp, opentelemetry::logs::Severity severity, nostd::string_view name, nostd::string_view body, const common::KeyValueIterable &resource, const common::KeyValueIterable &attributes, opentelemetry::trace::TraceId trace_id, opentelemetry::trace::SpanId span_id, opentelemetry::trace::TraceFlags trace_flags) noexcept + */ +void Logger::Log(core::SystemTimestamp timestamp, + opentelemetry::logs::Severity severity, + nostd::string_view name, + nostd::string_view body, + const common::KeyValueIterable &resource, + const common::KeyValueIterable &attributes, + opentelemetry::trace::TraceId trace_id, + opentelemetry::trace::SpanId span_id, + opentelemetry::trace::TraceFlags trace_flags) noexcept { // If this logger does not have a processor, no need to create a log record auto processor = logger_provider_.lock()->GetProcessor(); @@ -50,11 +58,12 @@ void Logger::Log(core::SystemTimestamp timestamp, opentelemetry::logs::Severity } // Populate recordable fields - if (timestamp == this->default_timestamp_){ - // Inject current timestamp if none is specified by user + if (timestamp == this->default_timestamp_) + { + // Inject current timestamp if none is specified by user recordable->SetTimestamp(core::SystemTimestamp(std::chrono::system_clock::now())); - } - else + } + else { recordable->SetTimestamp(timestamp); } @@ -75,7 +84,6 @@ void Logger::Log(core::SystemTimestamp timestamp, opentelemetry::logs::Severity return true; }); - // Inject trace_id/span_id/trace_flags if none is set by user auto provider = opentelemetry::trace::Provider::GetTracerProvider(); auto tracer = provider->GetTracer("foo_library"); @@ -85,8 +93,8 @@ void Logger::Log(core::SystemTimestamp timestamp, opentelemetry::logs::Severity if (!trace_id.IsValid()) { recordable->SetTraceId(span_context.trace_id()); - } - else + } + else { recordable->SetTraceId(trace_id); } @@ -95,8 +103,8 @@ void Logger::Log(core::SystemTimestamp timestamp, opentelemetry::logs::Severity if (!span_id.IsValid()) { recordable->SetSpanId(span_context.span_id()); - } - else + } + else { recordable->SetSpanId(span_id); } @@ -105,8 +113,8 @@ void Logger::Log(core::SystemTimestamp timestamp, opentelemetry::logs::Severity if (!trace_flags.IsSampled()) { recordable->SetTraceFlags(span_context.trace_flags()); - } - else + } + else { recordable->SetTraceFlags(trace_flags); } diff --git a/sdk/src/logs/simple_log_processor.cc b/sdk/src/logs/simple_log_processor.cc index 29ca2ccfc0..d6b8dda477 100644 --- a/sdk/src/logs/simple_log_processor.cc +++ b/sdk/src/logs/simple_log_processor.cc @@ -32,7 +32,7 @@ SimpleLogProcessor::SimpleLogProcessor(std::unique_ptr &&exporter) : exporter_(std::move(exporter)) {} -std::unique_ptr SimpleLogProcessor::MakeRecordable() noexcept +std::unique_ptr SimpleLogProcessor::MakeRecordable() noexcept { return exporter_->MakeRecordable(); } @@ -41,8 +41,7 @@ std::unique_ptr SimpleLogProcessor::MakeRecordable() noexcept * Batches the log record it receives in a batch of 1 and immediately sends it * to the configured exporter */ -void SimpleLogProcessor::OnReceive( - std::unique_ptr &&record) noexcept +void SimpleLogProcessor::OnReceive(std::unique_ptr &&record) noexcept { nostd::span> batch(&record, 1); // Get lock to ensure Export() is never called concurrently diff --git a/sdk/test/logs/logger_provider_sdk_test.cc b/sdk/test/logs/logger_provider_sdk_test.cc index 6c7c825795..b028afdef9 100644 --- a/sdk/test/logs/logger_provider_sdk_test.cc +++ b/sdk/test/logs/logger_provider_sdk_test.cc @@ -16,9 +16,9 @@ #include "opentelemetry/logs/provider.h" #include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/sdk/logs/log_record.h" #include "opentelemetry/sdk/logs/logger.h" #include "opentelemetry/sdk/logs/logger_provider.h" -#include "opentelemetry/sdk/logs/log_record.h" #include @@ -70,7 +70,8 @@ TEST(LoggerProviderSDK, LoggerProviderLoggerArguments) class DummyProcessor : public LogProcessor { - std::unique_ptr MakeRecordable() noexcept { + std::unique_ptr MakeRecordable() noexcept + { return std::unique_ptr(new LogRecord); } diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index bb98dae84d..afa130e86c 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -14,8 +14,8 @@ * limitations under the License. */ -#include "opentelemetry/sdk/logs/logger.h" #include "opentelemetry/sdk/logs/log_record.h" +#include "opentelemetry/sdk/logs/logger.h" #include @@ -36,7 +36,7 @@ TEST(LoggerSDK, LogToNullProcessor) class DummyProcessor : public LogProcessor { - std::unique_ptr MakeRecordable() noexcept + std::unique_ptr MakeRecordable() noexcept { return std::unique_ptr(new LogRecord); } diff --git a/sdk/test/logs/simple_log_processor_test.cc b/sdk/test/logs/simple_log_processor_test.cc index 3ee78c97d8..c653401f47 100644 --- a/sdk/test/logs/simple_log_processor_test.cc +++ b/sdk/test/logs/simple_log_processor_test.cc @@ -1,7 +1,7 @@ #include "opentelemetry/sdk/logs/simple_log_processor.h" +#include "opentelemetry/nostd/span.h" #include "opentelemetry/sdk/logs/exporter.h" #include "opentelemetry/sdk/logs/log_record.h" -#include "opentelemetry/nostd/span.h" #include @@ -37,8 +37,7 @@ class TestExporter final : public LogExporter *batch_size_received = records.size(); for (auto &record : records) { - auto log_record = std::unique_ptr( - static_cast(record.release())); + auto log_record = std::unique_ptr(static_cast(record.release())); if (log_record != nullptr) { @@ -66,7 +65,8 @@ class TestExporter final : public LogExporter TEST(SimpleLogProcessorTest, SendReceivedLogsToExporter) { // Create a simple processor with a TestExporter attached - std::shared_ptr>> logs_received(new std::vector>); + std::shared_ptr>> logs_received( + new std::vector>); size_t batch_size_received = 0; std::unique_ptr exporter( @@ -87,7 +87,7 @@ TEST(SimpleLogProcessorTest, SendReceivedLogsToExporter) } // Test whether the processor's log sent matches the log record received by the exporter - EXPECT_EQ(logs_received->size(), num_logs); + EXPECT_EQ(logs_received->size(), num_logs); for (int i = 0; i < num_logs; i++) { EXPECT_EQ("Log", logs_received->at(i)->GetName()); From 58164181185619052950cd5ba9b2c0207fe7fdc6 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Thu, 10 Dec 2020 16:58:39 -0500 Subject: [PATCH 09/21] Remove extra --- api/include/opentelemetry/logs/logger.h | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index bc437432b2..1a746d6f51 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -155,16 +155,6 @@ class Logger // Set default severity then call Log(Severity, String message) method inline void Log(nostd::string_view message) noexcept { this->Log(default_severity_, message); } - // Set default time, body, resources, traceid, spanid, and traceflags, - // and then call Log(Severity, String message) method - // inline void Log(Severity severity, - // nostd::string_view name, - // const common::KeyValueIterable &attributes) noexcept - // { - // this->Log(default_timestamp_, severity, name, default_body_, default_resource_, attributes, - // default_trace_id_, default_span_id_, default_trace_flags_); - // } - // TODO: Add more overloaded Log(...) methods with different combiantions of parameters. // TODO: Add function aliases such as void debug(), void trace(), void info(), etc. for each From d2f85b1de66ddd08976991ba9fa2cc5c18a39de0 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Thu, 10 Dec 2020 18:19:35 -0500 Subject: [PATCH 10/21] Convert nostd to std types in SDK, fix typo --- api/include/opentelemetry/logs/logger.h | 4 ++-- sdk/include/opentelemetry/sdk/logs/log_record.h | 13 ++++++------- sdk/include/opentelemetry/sdk/logs/recordable.h | 1 - 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index 1a746d6f51..ed78e58f93 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -107,13 +107,13 @@ class Logger Severity severity, nostd::string_view name, nostd::string_view body, - const T &resources, + const T &resource, const U &attributes, trace::TraceId trace_id, trace::SpanId span_id, trace::TraceFlags trace_flags) noexcept { - Log(timestamp, severity, name, body, common::KeyValueIterableView(resources), + Log(timestamp, severity, name, body, common::KeyValueIterableView(resource), common::KeyValueIterableView(attributes), trace_id, span_id, trace_flags); } diff --git a/sdk/include/opentelemetry/sdk/logs/log_record.h b/sdk/include/opentelemetry/sdk/logs/log_record.h index a605bafdf8..3d518cece7 100644 --- a/sdk/include/opentelemetry/sdk/logs/log_record.h +++ b/sdk/include/opentelemetry/sdk/logs/log_record.h @@ -18,7 +18,6 @@ #include #include -#include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/sdk/logs/attribute_utils.h" // same as traces/attribute_utils #include "opentelemetry/sdk/logs/recordable.h" #include "opentelemetry/version.h" @@ -44,8 +43,8 @@ class LogRecord final : public Recordable opentelemetry::trace::SpanId span_id_; // byte sequence opentelemetry::trace::TraceFlags trace_flags_; // byte opentelemetry::logs::Severity severity_; // Severity enum - nostd::string_view name_; // string - nostd::string_view body_; // currently a simple string, but should be changed "Any" type + std::string name_; // string + std::string body_; // currently a simple string, but should be changed "Any" type AttributeMap resource_map_; AttributeMap attributes_map_; @@ -70,13 +69,13 @@ class LogRecord final : public Recordable * Set name for this log * @param name the name to set */ - void SetName(nostd::string_view name) noexcept override { name_ = name; } + void SetName(nostd::string_view name) noexcept override { name_ = std::string(name); } /** * Set body field for this log. * @param message the body to set */ - void SetBody(nostd::string_view message) noexcept override { body_ = message; } + void SetBody(nostd::string_view message) noexcept override { body_ = std::string(message); } /** * Set a resource for this log. @@ -146,13 +145,13 @@ class LogRecord final : public Recordable * Get the name of this log * @return the name of this log */ - nostd::string_view GetName() const noexcept { return name_; } + std::string GetName() const noexcept { return name_; } /** * Get the body of this log * @return the body of this log */ - nostd::string_view GetBody() const noexcept { return body_; } + std::string GetBody() const noexcept { return body_; } /** * Get the resource field for this log diff --git a/sdk/include/opentelemetry/sdk/logs/recordable.h b/sdk/include/opentelemetry/sdk/logs/recordable.h index ada25ef2a7..ddbd621ee4 100644 --- a/sdk/include/opentelemetry/sdk/logs/recordable.h +++ b/sdk/include/opentelemetry/sdk/logs/recordable.h @@ -4,7 +4,6 @@ #include "opentelemetry/common/key_value_iterable.h" #include "opentelemetry/core/timestamp.h" #include "opentelemetry/logs/severity.h" -#include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/common/empty_attributes.h" #include "opentelemetry/trace/span.h" #include "opentelemetry/trace/span_context.h" From 0d650afdee2c5bafb896a0e75b0b9dc5bf0433b0 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Fri, 11 Dec 2020 00:21:37 -0500 Subject: [PATCH 11/21] Remove enum numbering, simplify --- api/include/opentelemetry/logs/severity.h | 53 ++++++++++--------- .../opentelemetry/sdk/logs/attribute_utils.h | 7 +-- 2 files changed, 28 insertions(+), 32 deletions(-) diff --git a/api/include/opentelemetry/logs/severity.h b/api/include/opentelemetry/logs/severity.h index c66b77a190..ce90e951de 100644 --- a/api/include/opentelemetry/logs/severity.h +++ b/api/include/opentelemetry/logs/severity.h @@ -22,35 +22,36 @@ namespace logs { /** - * Severity Levels assigned to log events, based on Log Data Model + * Severity Levels assigned to log events, based on Log Data Model, + * with the addition of kInvalid (mapped to a severity number of 0). */ enum class Severity : uint8_t { - kInvalid = 0, - 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 + kInvalid, + kTrace, + kTrace2, + kTrace3, + kTrace4, + kDebug, + kDebug2, + kDebug3, + kDebug4, + kInfo, + kInfo2, + kInfo3, + kInfo4, + kWarn, + kWarn2, + kWarn3, + kWarn4, + kError, + kError2, + kError3, + kError4, + kFatal, + kFatal2, + kFatal3, + kFatal4 }; } // namespace logs diff --git a/sdk/include/opentelemetry/sdk/logs/attribute_utils.h b/sdk/include/opentelemetry/sdk/logs/attribute_utils.h index 17b6ec01e0..f29edbcaa9 100644 --- a/sdk/include/opentelemetry/sdk/logs/attribute_utils.h +++ b/sdk/include/opentelemetry/sdk/logs/attribute_utils.h @@ -74,12 +74,7 @@ struct AttributeConverter template LogRecordAttributeValue convertSpan(nostd::span vals) { - std::vector copy; - for (auto &val : vals) - { - copy.push_back(T(val)); - } - + const std::vector copy(vals.begin(), vals.end()); return LogRecordAttributeValue(std::move(copy)); } }; From 0fae1700440eac5713d132fbbf328f355bcb7775 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Fri, 11 Dec 2020 14:33:57 -0500 Subject: [PATCH 12/21] Set current time, remove extra "inline", log error as todo --- api/include/opentelemetry/logs/logger.h | 23 +++++++++---------- .../opentelemetry/sdk/logs/attribute_utils.h | 4 ---- sdk/src/logs/logger.cc | 1 + 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index ed78e58f93..3796deb0b3 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -43,13 +43,14 @@ class Logger { protected: // Default values to set for fields that the user doesn't specify - core::SystemTimestamp default_timestamp_; // uint64 nanoseconds since Unix epoch, default 0 - Severity default_severity_; // default 0 (kInvalid) - nostd::string_view default_name_; // default 0 length, nullptr data - nostd::string_view default_body_; // default 0 length, nullptr data. TODO: currently a simple - // string, but should be changed "Any" type - trace::TraceId default_trace_id_; // default 00000000000000000000000000000000 - trace::SpanId default_span_id_; // default 0000000000000000 + core::SystemTimestamp default_timestamp_ = + std::chrono::system_clock::now(); // uint64 nanoseconds since Unix epoch, default 0 + Severity default_severity_; // default 0 (kInvalid) + nostd::string_view default_name_; // default 0 length, nullptr data + nostd::string_view default_body_; // default 0 length, nullptr data. TODO: currently a simple + // string, but should be changed "Any" type + trace::TraceId default_trace_id_; // default 00000000000000000000000000000000 + trace::SpanId default_span_id_; // default 0000000000000000 trace::TraceFlags default_trace_flags_; // default 00 public: @@ -138,22 +139,20 @@ class Logger /** Wrapper methods that the user could call for convenience when logging **/ // Set default values for unspecified fields, then call the base Log() method - inline void Log(Severity severity, - nostd::string_view message, - core::SystemTimestamp time) noexcept + void Log(Severity severity, nostd::string_view message, core::SystemTimestamp time) noexcept { this->Log(time, severity, default_name_, message, {}, {}, default_trace_id_, default_span_id_, default_trace_flags_); } // Set default time, and call base Log(severity, message, time) method - inline void Log(Severity severity, nostd::string_view message) noexcept + void Log(Severity severity, nostd::string_view message) noexcept { this->Log(severity, message, default_timestamp_); } // Set default severity then call Log(Severity, String message) method - inline void Log(nostd::string_view message) noexcept { this->Log(default_severity_, message); } + void Log(nostd::string_view message) noexcept { this->Log(default_severity_, message); } // TODO: Add more overloaded Log(...) methods with different combiantions of parameters. diff --git a/sdk/include/opentelemetry/sdk/logs/attribute_utils.h b/sdk/include/opentelemetry/sdk/logs/attribute_utils.h index f29edbcaa9..770d8412e4 100644 --- a/sdk/include/opentelemetry/sdk/logs/attribute_utils.h +++ b/sdk/include/opentelemetry/sdk/logs/attribute_utils.h @@ -37,10 +37,6 @@ struct AttributeConverter LogRecordAttributeValue operator()(bool v) { return LogRecordAttributeValue(v); } LogRecordAttributeValue operator()(int32_t v) { return LogRecordAttributeValue(v); } LogRecordAttributeValue operator()(uint32_t v) { return LogRecordAttributeValue(v); } - /*LogRecordAttributeValue operator()(int v) - { - return LogRecordAttributeValue(static_cast(v)); - }*/ LogRecordAttributeValue operator()(int64_t v) { return LogRecordAttributeValue(v); } LogRecordAttributeValue operator()(uint64_t v) { return LogRecordAttributeValue(v); } LogRecordAttributeValue operator()(double v) { return LogRecordAttributeValue(v); } diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index 60e8c7ed57..6cacd028d0 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -54,6 +54,7 @@ void Logger::Log(core::SystemTimestamp timestamp, auto recordable = processor->MakeRecordable(); if (recordable == nullptr) { + // TODO: Error diagnostics should indicate "recordable creation failed" to user return; } From 76f00d76ed8c9c3520156d5c281f5a5fa1ac9a52 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Fri, 11 Dec 2020 15:01:41 -0500 Subject: [PATCH 13/21] Fix timestamp setting, update comments --- api/include/opentelemetry/logs/logger.h | 22 ++++++++-------------- sdk/src/logs/logger.cc | 17 ++++------------- 2 files changed, 12 insertions(+), 27 deletions(-) diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index 3796deb0b3..bdfff75328 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -43,23 +43,17 @@ class Logger { protected: // Default values to set for fields that the user doesn't specify - core::SystemTimestamp default_timestamp_ = - std::chrono::system_clock::now(); // uint64 nanoseconds since Unix epoch, default 0 - Severity default_severity_; // default 0 (kInvalid) - nostd::string_view default_name_; // default 0 length, nullptr data - nostd::string_view default_body_; // default 0 length, nullptr data. TODO: currently a simple - // string, but should be changed "Any" type - trace::TraceId default_trace_id_; // default 00000000000000000000000000000000 - trace::SpanId default_span_id_; // default 0000000000000000 - trace::TraceFlags default_trace_flags_; // default 00 + Severity default_severity_; // default 0 (severity of kInvalid) + nostd::string_view default_name_; // default 0 length, nullptr data + nostd::string_view default_body_; // default 0 length, nullptr data. TODO: currently a simple + // string, but should be changed "Any" type + trace::TraceId default_trace_id_; // default 00000000000000000000000000000000, in base 16 + trace::SpanId default_span_id_; // default 0000000000000000, in base 16 + trace::TraceFlags default_trace_flags_; // default 00, in base 16 public: 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; - /** * Each of the following overloaded Log(...) methods * creates a log message with the specific parameters passed. @@ -148,7 +142,7 @@ class Logger // Set default time, and call base Log(severity, message, time) method void Log(Severity severity, nostd::string_view message) noexcept { - this->Log(severity, message, default_timestamp_); + this->Log(severity, message, std::chrono::system_clock::now()); } // Set default severity then call Log(Severity, String message) method diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index 6cacd028d0..9adbcb5c09 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -28,7 +28,7 @@ Logger::Logger(std::shared_ptr logger_provider) noexcept {} /** - * Create and popualte recordable with the log event's fields passed in. + * Create and populate recordable with the log event's fields passed in. * The timestamp, severity, traceid, spanid, and traceflags, are injected * if the user does not specify them. */ @@ -49,7 +49,7 @@ void Logger::Log(core::SystemTimestamp timestamp, return; } - // TODO: Sampler logic (should include check for minSeverity) + // TODO: Sampler (should include check for minSeverity) auto recordable = processor->MakeRecordable(); if (recordable == nullptr) @@ -59,16 +59,7 @@ void Logger::Log(core::SystemTimestamp timestamp, } // Populate recordable fields - if (timestamp == this->default_timestamp_) - { - // Inject current timestamp if none is specified by user - recordable->SetTimestamp(core::SystemTimestamp(std::chrono::system_clock::now())); - } - else - { - recordable->SetTimestamp(timestamp); - } - + recordable->SetTimestamp(timestamp); recordable->SetSeverity(severity); recordable->SetName(name); recordable->SetBody(body); @@ -110,7 +101,7 @@ void Logger::Log(core::SystemTimestamp timestamp, recordable->SetSpanId(span_id); } - // Traceflag + // Traceflags if (!trace_flags.IsSampled()) { recordable->SetTraceFlags(span_context.trace_flags()); From dad6fe8ef3ad3ae70cf5b780c9bce859cdd6ebe7 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Fri, 11 Dec 2020 16:51:47 -0500 Subject: [PATCH 14/21] Log() inline comment params (no code changes) --- sdk/include/opentelemetry/sdk/logs/logger.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/logs/logger.h b/sdk/include/opentelemetry/sdk/logs/logger.h index a4166669b7..3e5d7471f1 100644 --- a/sdk/include/opentelemetry/sdk/logs/logger.h +++ b/sdk/include/opentelemetry/sdk/logs/logger.h @@ -45,12 +45,13 @@ class Logger final : public opentelemetry::logs::Logger /** * Writes a log record into the processor. * @param timestamp the timestamp the log record was created. - * @param name the name of the log event. * @param severity the severity level of the log event. + * @param name the name 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 resource the resources, stored as a 2D list of key/value pairs, that are associated + * with the log event. * @param attributes the attributes, stored as a 2D list of key/value pairs, that are associated - * wtih the log event. + * with the log event. * @param trace_id the trace id associated with the log event. * @param span_id the span id associate with the log event. * @param trace_flags the trace flags associated with the log event. From ad978e7994fe6d7f4b37fb982eb7bd713b1ae961 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Fri, 11 Dec 2020 17:00:01 -0500 Subject: [PATCH 15/21] More comment cleanup (no code changes) --- api/include/opentelemetry/logs/logger.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index bdfff75328..b1464377be 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -59,12 +59,13 @@ class Logger * creates a log message with the specific parameters passed. * * @param timestamp the timestamp the log record was created. - * @param name the name of the log event. * @param severity the severity level of the log event. + * @param name the name 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 resource the resources, stored as a 2D list of key/value pairs, that are associated + * with the log event. * @param attributes the attributes, stored as a 2D list of key/value pairs, that are associated - * wtih the log event. + * with the log event. * @param trace_id the trace id associated with the log event. * @param span_id the span id associate with the log event. * @param trace_flags the trace flags associated with the log event. From cec16507848f1f1900a4ba49c17cfd03a4a8be89 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Sat, 12 Dec 2020 03:57:17 -0500 Subject: [PATCH 16/21] Remove default vars, validate trace/spanid, tracer renamed, build file fixed --- api/include/opentelemetry/logs/logger.h | 45 ++++++++------------- api/include/opentelemetry/logs/noop.h | 6 +-- api/include/opentelemetry/logs/severity.h | 5 +++ api/test/logs/logger_test.cc | 6 +-- sdk/include/opentelemetry/sdk/logs/logger.h | 14 ++++--- sdk/src/logs/BUILD | 1 - sdk/src/logs/CMakeLists.txt | 3 +- sdk/src/logs/logger.cc | 17 ++++---- sdk/src/logs/logger_provider.cc | 2 +- 9 files changed, 48 insertions(+), 51 deletions(-) diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index b1464377be..fc486ec464 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -41,16 +41,6 @@ namespace logs **/ class Logger { -protected: - // Default values to set for fields that the user doesn't specify - Severity default_severity_; // default 0 (severity of kInvalid) - nostd::string_view default_name_; // default 0 length, nullptr data - nostd::string_view default_body_; // default 0 length, nullptr data. TODO: currently a simple - // string, but should be changed "Any" type - trace::TraceId default_trace_id_; // default 00000000000000000000000000000000, in base 16 - trace::SpanId default_span_id_; // default 0000000000000000, in base 16 - trace::TraceFlags default_trace_flags_; // default 00, in base 16 - public: virtual ~Logger() = default; @@ -58,7 +48,6 @@ class Logger * Each of the following overloaded Log(...) methods * creates a log message with the specific parameters passed. * - * @param timestamp the timestamp the log record was created. * @param severity the severity level of the log event. * @param name the name of the log event. * @param message the string message of the log (perhaps support std::fmt or fmt-lib format). @@ -69,6 +58,7 @@ class Logger * @param trace_id the trace id associated with the log event. * @param span_id the span id associate with the log event. * @param trace_flags the trace flags associated with the log event. + * @param timestamp the timestamp the log record was created. * @throws No exceptions under any circumstances. */ @@ -78,15 +68,15 @@ class Logger * * Note this takes in a KeyValueIterable for the resource and attributes fields. */ - virtual void Log(core::SystemTimestamp timestamp, - Severity severity, + virtual void Log(Severity severity, nostd::string_view name, nostd::string_view body, const common::KeyValueIterable &resource, const common::KeyValueIterable &attributes, trace::TraceId trace_id, trace::SpanId span_id, - trace::TraceFlags trace_flags) noexcept = 0; + trace::TraceFlags trace_flags, + core::SystemTimestamp timestamp) noexcept = 0; /*** Overloaded methods for KeyValueIterables ***/ /** @@ -99,45 +89,44 @@ class Logger class U, nostd::enable_if_t::value> * = nullptr, nostd::enable_if_t::value> * = nullptr> - void Log(core::SystemTimestamp timestamp, - Severity severity, + void Log(Severity severity, nostd::string_view name, nostd::string_view body, const T &resource, const U &attributes, trace::TraceId trace_id, trace::SpanId span_id, - trace::TraceFlags trace_flags) noexcept + trace::TraceFlags trace_flags, + core::SystemTimestamp timestamp) noexcept { - Log(timestamp, severity, name, body, common::KeyValueIterableView(resource), - common::KeyValueIterableView(attributes), trace_id, span_id, trace_flags); + Log(severity, name, body, common::KeyValueIterableView(resource), + common::KeyValueIterableView(attributes), trace_id, span_id, trace_flags, timestamp); } - void Log(core::SystemTimestamp timestamp, - Severity severity, + void Log(Severity severity, nostd::string_view name, nostd::string_view body, std::initializer_list> resource, std::initializer_list> attributes, trace::TraceId trace_id, trace::SpanId span_id, - trace::TraceFlags trace_flags) noexcept + trace::TraceFlags trace_flags, + core::SystemTimestamp timestamp) noexcept { - return this->Log(timestamp, severity, name, body, + return this->Log(severity, name, body, nostd::span>{ resource.begin(), resource.end()}, nostd::span>{ attributes.begin(), attributes.end()}, - trace_id, span_id, trace_flags); + trace_id, span_id, trace_flags, timestamp); } /** Wrapper methods that the user could call for convenience when logging **/ // Set default values for unspecified fields, then call the base Log() method - void Log(Severity severity, nostd::string_view message, core::SystemTimestamp time) noexcept + void Log(Severity severity, nostd::string_view message, core::SystemTimestamp timestamp) noexcept { - this->Log(time, severity, default_name_, message, {}, {}, default_trace_id_, default_span_id_, - default_trace_flags_); + this->Log(severity, "", message, {}, {}, {}, {}, {}, timestamp); } // Set default time, and call base Log(severity, message, time) method @@ -147,7 +136,7 @@ class Logger } // Set default severity then call Log(Severity, String message) method - void Log(nostd::string_view message) noexcept { this->Log(default_severity_, message); } + void Log(nostd::string_view message) noexcept { this->Log(Severity::kInfo, message); } // TODO: Add more overloaded Log(...) methods with different combiantions of parameters. diff --git a/api/include/opentelemetry/logs/noop.h b/api/include/opentelemetry/logs/noop.h index 0ef6806b71..8a19e1a677 100644 --- a/api/include/opentelemetry/logs/noop.h +++ b/api/include/opentelemetry/logs/noop.h @@ -50,15 +50,15 @@ namespace logs class NoopLogger final : public Logger { public: - void Log(core::SystemTimestamp timestamp, - Severity severity, + void Log(Severity severity, nostd::string_view name, nostd::string_view body, const common::KeyValueIterable &resource, const common::KeyValueIterable &attributes, trace::TraceId trace_id, trace::SpanId span_id, - trace::TraceFlags trace_flags) noexcept override + trace::TraceFlags trace_flags, + core::SystemTimestamp timestamp) noexcept override {} }; diff --git a/api/include/opentelemetry/logs/severity.h b/api/include/opentelemetry/logs/severity.h index ce90e951de..078c03bdc6 100644 --- a/api/include/opentelemetry/logs/severity.h +++ b/api/include/opentelemetry/logs/severity.h @@ -54,5 +54,10 @@ enum class Severity : uint8_t kFatal4 }; +const opentelemetry::nostd::string_view SeverityNumToText[25] = { + "INVALID", "TRACE", "TRACE2", "TRACE3", "TRACE4", "DEBUG", "DEBUG2", "DEBUG3", "DEBUG4", + "INFO", "INFO2", "INFO3", "INFO4", "WARN", "WARN2", "WARN3", "WARN4", "ERROR", + "ERROR2", "ERROR3", "ERROR4", "FATAL", "FATAL2", "FATAL3", "FATAL4"}; + } // namespace logs OPENTELEMETRY_END_NAMESPACE diff --git a/api/test/logs/logger_test.cc b/api/test/logs/logger_test.cc index 8013a8211d..d875a54c35 100644 --- a/api/test/logs/logger_test.cc +++ b/api/test/logs/logger_test.cc @@ -48,15 +48,15 @@ TEST(Logger, NoopLog) // Define a basic Logger class class TestLogger : public Logger { - void Log(opentelemetry::core::SystemTimestamp timestamp, - Severity severity, + void Log(Severity severity, string_view name, string_view body, const opentelemetry::common::KeyValueIterable &resource, const opentelemetry::common::KeyValueIterable &attributes, opentelemetry::trace::TraceId trace_id, opentelemetry::trace::SpanId span_id, - opentelemetry::trace::TraceFlags trace_flags) noexcept override + opentelemetry::trace::TraceFlags trace_flags, + opentelemetry::core::SystemTimestamp timestamp) noexcept override {} }; diff --git a/sdk/include/opentelemetry/sdk/logs/logger.h b/sdk/include/opentelemetry/sdk/logs/logger.h index 3e5d7471f1..c34e65ae33 100644 --- a/sdk/include/opentelemetry/sdk/logs/logger.h +++ b/sdk/include/opentelemetry/sdk/logs/logger.h @@ -40,11 +40,11 @@ class Logger final : public opentelemetry::logs::Logger * Initialize a new logger. * @param logger_provider The logger provider that owns this logger. */ - explicit Logger(std::shared_ptr logger_provider) noexcept; + explicit Logger(opentelemetry::nostd::string_view name, + std::shared_ptr logger_provider) noexcept; /** * Writes a log record into the processor. - * @param timestamp the timestamp the log record was created. * @param severity the severity level of the log event. * @param name the name of the log event. * @param message the string message of the log (perhaps support std::fmt or fmt-lib format). @@ -55,21 +55,25 @@ class Logger final : public opentelemetry::logs::Logger * @param trace_id the trace id associated with the log event. * @param span_id the span id associate with the log event. * @param trace_flags the trace flags associated with the log event. + * @param timestamp the timestamp the log record was created. * @throws No exceptions under any circumstances. */ - void Log(core::SystemTimestamp timestamp, - opentelemetry::logs::Severity severity, + void Log(opentelemetry::logs::Severity severity, nostd::string_view name, nostd::string_view body, const common::KeyValueIterable &resource, const common::KeyValueIterable &attributes, trace::TraceId trace_id, trace::SpanId span_id, - trace::TraceFlags trace_flags) noexcept override; + trace::TraceFlags trace_flags, + core::SystemTimestamp timestamp) noexcept override; private: // The logger provider of this Logger. Uses a weak_ptr to avoid cyclic dependancy issues the with // logger provider std::weak_ptr logger_provider_; + + // The name of this logger + opentelemetry::nostd::string_view logger_name_; }; } // namespace logs diff --git a/sdk/src/logs/BUILD b/sdk/src/logs/BUILD index 4829b3838f..c1a0542fa1 100644 --- a/sdk/src/logs/BUILD +++ b/sdk/src/logs/BUILD @@ -22,6 +22,5 @@ cc_library( deps = [ "//api", "//sdk:headers", - "//sdk/src/trace", ], ) diff --git a/sdk/src/logs/CMakeLists.txt b/sdk/src/logs/CMakeLists.txt index 326558a922..e2e7c2c915 100644 --- a/sdk/src/logs/CMakeLists.txt +++ b/sdk/src/logs/CMakeLists.txt @@ -1,5 +1,4 @@ add_library(opentelemetry_logs logger_provider.cc logger.cc simple_log_processor.cc) -target_link_libraries(opentelemetry_logs opentelemetry_common - opentelemetry_trace) +target_link_libraries(opentelemetry_logs opentelemetry_common) diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index 9adbcb5c09..054da1fdb0 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -23,8 +23,9 @@ namespace sdk { namespace logs { -Logger::Logger(std::shared_ptr logger_provider) noexcept - : logger_provider_(logger_provider) +Logger::Logger(opentelemetry::nostd::string_view name, + std::shared_ptr logger_provider) noexcept + : logger_name_(name), logger_provider_(logger_provider) {} /** @@ -32,15 +33,15 @@ Logger::Logger(std::shared_ptr logger_provider) noexcept * The timestamp, severity, traceid, spanid, and traceflags, are injected * if the user does not specify them. */ -void Logger::Log(core::SystemTimestamp timestamp, - opentelemetry::logs::Severity severity, +void Logger::Log(opentelemetry::logs::Severity severity, nostd::string_view name, nostd::string_view body, const common::KeyValueIterable &resource, const common::KeyValueIterable &attributes, opentelemetry::trace::TraceId trace_id, opentelemetry::trace::SpanId span_id, - opentelemetry::trace::TraceFlags trace_flags) noexcept + opentelemetry::trace::TraceFlags trace_flags, + core::SystemTimestamp timestamp) noexcept { // If this logger does not have a processor, no need to create a log record auto processor = logger_provider_.lock()->GetProcessor(); @@ -78,11 +79,11 @@ void Logger::Log(core::SystemTimestamp timestamp, // Inject trace_id/span_id/trace_flags if none is set by user auto provider = opentelemetry::trace::Provider::GetTracerProvider(); - auto tracer = provider->GetTracer("foo_library"); + auto tracer = provider->GetTracer(logger_name_); auto span_context = tracer->GetCurrentSpan()->GetContext(); // Traceid - if (!trace_id.IsValid()) + if (!trace_id.IsValid() && span_context.trace_id().IsValid()) { recordable->SetTraceId(span_context.trace_id()); } @@ -92,7 +93,7 @@ void Logger::Log(core::SystemTimestamp timestamp, } // Spanid - if (!span_id.IsValid()) + if (!span_id.IsValid() && span_context.span_id().IsValid()) { recordable->SetSpanId(span_context.span_id()); } diff --git a/sdk/src/logs/logger_provider.cc b/sdk/src/logs/logger_provider.cc index b040f9baab..6c978c3472 100644 --- a/sdk/src/logs/logger_provider.cc +++ b/sdk/src/logs/logger_provider.cc @@ -54,7 +54,7 @@ opentelemetry::nostd::shared_ptr LoggerProvider::Ge // If no logger with that name exists yet, create it and add it to the map of loggers opentelemetry::nostd::shared_ptr logger( - new Logger(this->shared_from_this())); + new Logger(name, this->shared_from_this())); loggers_[name.data()] = logger; return logger; } From ace34dd69288bc422dc7a043a129fb57cea39d2e Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Sat, 12 Dec 2020 04:17:32 -0500 Subject: [PATCH 17/21] Purpose of severity map --- api/include/opentelemetry/logs/severity.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/api/include/opentelemetry/logs/severity.h b/api/include/opentelemetry/logs/severity.h index 078c03bdc6..c54aaa77ff 100644 --- a/api/include/opentelemetry/logs/severity.h +++ b/api/include/opentelemetry/logs/severity.h @@ -54,6 +54,14 @@ enum class Severity : uint8_t kFatal4 }; +/** + * Internal mapping of the severity enum values above to the severity text that + * can be printed out by exporters. + * + * This is included because the specification recommends printing both severity + * number and text in log records. The convention uses all capital letters to follow + * the specification's convention as well. + */ const opentelemetry::nostd::string_view SeverityNumToText[25] = { "INVALID", "TRACE", "TRACE2", "TRACE3", "TRACE4", "DEBUG", "DEBUG2", "DEBUG3", "DEBUG4", "INFO", "INFO2", "INFO3", "INFO4", "WARN", "WARN2", "WARN3", "WARN4", "ERROR", From 26ed876e3f0c552741ec839411386e2d05a027c9 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Sat, 12 Dec 2020 13:15:43 -0500 Subject: [PATCH 18/21] use std::string for loggername, clean up trace injection --- sdk/include/opentelemetry/sdk/logs/logger.h | 2 +- sdk/src/logs/logger.cc | 34 ++++++++++++--------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/logs/logger.h b/sdk/include/opentelemetry/sdk/logs/logger.h index c34e65ae33..bf10374b08 100644 --- a/sdk/include/opentelemetry/sdk/logs/logger.h +++ b/sdk/include/opentelemetry/sdk/logs/logger.h @@ -73,7 +73,7 @@ class Logger final : public opentelemetry::logs::Logger std::weak_ptr logger_provider_; // The name of this logger - opentelemetry::nostd::string_view logger_name_; + std::string logger_name_; }; } // namespace logs diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index 054da1fdb0..fa17655a33 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -25,7 +25,7 @@ namespace logs { Logger::Logger(opentelemetry::nostd::string_view name, std::shared_ptr logger_provider) noexcept - : logger_name_(name), logger_provider_(logger_provider) + : logger_name_(std::string(name)), logger_provider_(logger_provider) {} /** @@ -82,35 +82,39 @@ void Logger::Log(opentelemetry::logs::Severity severity, auto tracer = provider->GetTracer(logger_name_); auto span_context = tracer->GetCurrentSpan()->GetContext(); + + // Leave these fields in the recordable empty if neither the passed in values + // nor the context values is valid (e.g. the application is not using traces) + // Traceid - if (!trace_id.IsValid() && span_context.trace_id().IsValid()) - { - recordable->SetTraceId(span_context.trace_id()); - } - else + if (trace_id.IsValid()) { recordable->SetTraceId(trace_id); + } + else if (span_context.trace_id().IsValid()) + { + recordable->SetTraceId(span_context.trace_id()); } // Spanid - if (!span_id.IsValid() && span_context.span_id().IsValid()) + if (span_id.IsValid()) { - recordable->SetSpanId(span_context.span_id()); + recordable->SetSpanId(span_id); } - else + else if (span_context.span_id().IsValid()) { recordable->SetSpanId(span_id); - } + } // Traceflags - if (!trace_flags.IsSampled()) - { - recordable->SetTraceFlags(span_context.trace_flags()); - } - else + if (trace_flags.IsSampled()) { recordable->SetTraceFlags(trace_flags); } + else if(span_context.trace_flags().IsSampled()) + { + recordable->SetTraceFlags(span_context.trace_flags()); + } // Send the log record to the processor processor->OnReceive(std::move(recordable)); From a03d17e58901fb17ade51c7deb6a6d803101cfdd Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Sat, 12 Dec 2020 17:35:54 -0500 Subject: [PATCH 19/21] Unit tests for LogRecord, reorder for consistency --- api/include/opentelemetry/logs/severity.h | 10 ++-- .../opentelemetry/sdk/logs/log_record.h | 40 +++++++------ sdk/src/logs/logger.cc | 11 ++-- sdk/test/logs/BUILD | 11 ++++ sdk/test/logs/CMakeLists.txt | 2 +- sdk/test/logs/log_record_test.cc | 59 +++++++++++++++++++ 6 files changed, 102 insertions(+), 31 deletions(-) create mode 100644 sdk/test/logs/log_record_test.cc diff --git a/api/include/opentelemetry/logs/severity.h b/api/include/opentelemetry/logs/severity.h index c54aaa77ff..a314c9695e 100644 --- a/api/include/opentelemetry/logs/severity.h +++ b/api/include/opentelemetry/logs/severity.h @@ -55,12 +55,12 @@ enum class Severity : uint8_t }; /** - * Internal mapping of the severity enum values above to the severity text that - * can be printed out by exporters. + * Mapping of the severity enum above, to a severity text string (in all caps). + * This severity text can be printed out by exporters. Capital letters follow the + * spec naming convention. * - * This is included because the specification recommends printing both severity - * number and text in log records. The convention uses all capital letters to follow - * the specification's convention as well. + * Included to follow the specification's recommendation to print both + * severity number and text in each log record. */ const opentelemetry::nostd::string_view SeverityNumToText[25] = { "INVALID", "TRACE", "TRACE2", "TRACE3", "TRACE4", "DEBUG", "DEBUG2", "DEBUG3", "DEBUG4", diff --git a/sdk/include/opentelemetry/sdk/logs/log_record.h b/sdk/include/opentelemetry/sdk/logs/log_record.h index 3d518cece7..73f71dd744 100644 --- a/sdk/include/opentelemetry/sdk/logs/log_record.h +++ b/sdk/include/opentelemetry/sdk/logs/log_record.h @@ -29,7 +29,7 @@ namespace logs { /** - * A default Event object to be passed in log statements, + * A default Recordable implemenation 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) * @@ -37,24 +37,20 @@ namespace logs class LogRecord final : public Recordable { private: - // Log Data Model fields - core::SystemTimestamp timestamp_; // uint64 nanoseconds since Unix epoch - opentelemetry::trace::TraceId trace_id_; // byte sequence - opentelemetry::trace::SpanId span_id_; // byte sequence - opentelemetry::trace::TraceFlags trace_flags_; // byte - opentelemetry::logs::Severity severity_; // Severity enum - std::string name_; // string - std::string body_; // currently a simple string, but should be changed "Any" type + // Default values are set by the respective data structures' constructors for all fields, + // except the severity field, which must be set manually (an enum with no default value). + opentelemetry::logs::Severity severity_ = opentelemetry::logs::Severity::kInvalid; AttributeMap resource_map_; AttributeMap attributes_map_; + std::string name_; + std::string body_; // Currently a simple string, but should be changed to "Any" type + opentelemetry::trace::TraceId trace_id_; + opentelemetry::trace::SpanId span_id_; + opentelemetry::trace::TraceFlags trace_flags_; + core::SystemTimestamp timestamp_; // uint64 nanoseconds since Unix epoch public: /********** Setters for each field (overrides methods from the Recordable interface) ************/ - /** - * Set the timestamp for this log. - * @param timestamp the timestamp of the event - */ - void SetTimestamp(core::SystemTimestamp timestamp) noexcept override { timestamp_ = timestamp; } /** * Set the severity for this log. @@ -127,13 +123,13 @@ class LogRecord final : public Recordable trace_flags_ = trace_flags; } - /************************** Getters for each field ****************************/ - /** - * Get the timestamp for this log - * @return the timestamp for this log + * Set the timestamp for this log. + * @param timestamp the timestamp of the event */ - core::SystemTimestamp GetTimestamp() const noexcept { return timestamp_; } + void SetTimestamp(core::SystemTimestamp timestamp) noexcept override { timestamp_ = timestamp; } + + /************************** Getters for each field ****************************/ /** * Get the severity for this log @@ -188,6 +184,12 @@ class LogRecord final : public Recordable * @return the trace flags for this log */ opentelemetry::trace::TraceFlags GetTraceFlags() const noexcept { return trace_flags_; } + + /** + * Get the timestamp for this log + * @return the timestamp for this log + */ + core::SystemTimestamp GetTimestamp() const noexcept { return timestamp_; } }; } // namespace logs } // namespace sdk diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index fa17655a33..5c2818deb1 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -82,15 +82,14 @@ void Logger::Log(opentelemetry::logs::Severity severity, auto tracer = provider->GetTracer(logger_name_); auto span_context = tracer->GetCurrentSpan()->GetContext(); - // Leave these fields in the recordable empty if neither the passed in values - // nor the context values is valid (e.g. the application is not using traces) + // nor the context values are valid (e.g. the application is not using traces) // Traceid if (trace_id.IsValid()) { recordable->SetTraceId(trace_id); - } + } else if (span_context.trace_id().IsValid()) { recordable->SetTraceId(span_context.trace_id()); @@ -104,17 +103,17 @@ void Logger::Log(opentelemetry::logs::Severity severity, else if (span_context.span_id().IsValid()) { recordable->SetSpanId(span_id); - } + } // Traceflags if (trace_flags.IsSampled()) { recordable->SetTraceFlags(trace_flags); } - else if(span_context.trace_flags().IsSampled()) + else if (span_context.trace_flags().IsSampled()) { recordable->SetTraceFlags(span_context.trace_flags()); - } + } // Send the log record to the processor processor->OnReceive(std::move(recordable)); diff --git a/sdk/test/logs/BUILD b/sdk/test/logs/BUILD index ccbbfd8c78..9e12e08ab9 100644 --- a/sdk/test/logs/BUILD +++ b/sdk/test/logs/BUILD @@ -31,3 +31,14 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) + +cc_test( + name = "log_record_test", + srcs = [ + "log_record_test.cc", + ], + deps = [ + "//sdk/src/logs", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/sdk/test/logs/CMakeLists.txt b/sdk/test/logs/CMakeLists.txt index 024c42c45a..f59c6a1926 100644 --- a/sdk/test/logs/CMakeLists.txt +++ b/sdk/test/logs/CMakeLists.txt @@ -1,5 +1,5 @@ foreach(testname logger_provider_sdk_test logger_sdk_test - simple_log_processor_test) + simple_log_processor_test log_record_test) add_executable(${testname} "${testname}.cc") target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} opentelemetry_logs) diff --git a/sdk/test/logs/log_record_test.cc b/sdk/test/logs/log_record_test.cc new file mode 100644 index 0000000000..774e726c54 --- /dev/null +++ b/sdk/test/logs/log_record_test.cc @@ -0,0 +1,59 @@ +#include "opentelemetry/sdk/logs/log_record.h" +#include "opentelemetry/nostd/variant.h" +#include "opentelemetry/trace/span_id.h" +#include "opentelemetry/trace/trace_id.h" + +#include + +using opentelemetry::sdk::logs::LogRecord; + +// Test what a default LogRecord with no fields set holds +TEST(LogRecord, GetDefaultValues) +{ + opentelemetry::trace::TraceId zero_trace_id; + opentelemetry::trace::SpanId zero_span_id; + opentelemetry::trace::TraceFlags zero_trace_flags; + LogRecord record; + + ASSERT_EQ(record.GetSeverity(), opentelemetry::logs::Severity::kInvalid); + ASSERT_EQ(record.GetName(), ""); + ASSERT_EQ(record.GetBody(), ""); + ASSERT_EQ(record.GetResource().size(), 0); + ASSERT_EQ(record.GetAttributes().size(), 0); + ASSERT_EQ(record.GetTraceId(), zero_trace_id); + ASSERT_EQ(record.GetSpanId(), zero_span_id); + ASSERT_EQ(record.GetTraceFlags(), zero_trace_flags); + ASSERT_EQ(record.GetTimestamp().time_since_epoch(), std::chrono::nanoseconds(0)); +} + +// Test LogRecord fields are properly set and get +TEST(LogRecord, SetAndGet) +{ + opentelemetry::trace::TraceId trace_id; + opentelemetry::trace::SpanId span_id; + opentelemetry::trace::TraceFlags trace_flags; + opentelemetry::core::SystemTimestamp now(std::chrono::system_clock::now()); + + // Set all fields of the LogRecord + LogRecord record; + record.SetSeverity(opentelemetry::logs::Severity::kInvalid); + record.SetName("Log name"); + record.SetBody("Message"); + record.SetResource("res1", (bool)true); + record.SetAttribute("attr1", (int64_t)314159); + record.SetTraceId(trace_id); + record.SetSpanId(span_id); + record.SetTraceFlags(trace_flags); + record.SetTimestamp(now); + + // Test that all fields match what was set + ASSERT_EQ(record.GetSeverity(), opentelemetry::logs::Severity::kInvalid); + ASSERT_EQ(record.GetName(), "Log name"); + ASSERT_EQ(record.GetBody(), "Message"); + ASSERT_EQ(opentelemetry::nostd::get(record.GetResource().at("res1")), 1); + ASSERT_EQ(opentelemetry::nostd::get(record.GetAttributes().at("attr1")), 314159); + ASSERT_EQ(record.GetTraceId(), trace_id); + ASSERT_EQ(record.GetSpanId(), span_id); + ASSERT_EQ(record.GetTraceFlags(), trace_flags); + ASSERT_EQ(record.GetTimestamp().time_since_epoch(), now.time_since_epoch()); +} From 61fea212307199981c71bfc9cd76dcaa8f450cc8 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Sat, 12 Dec 2020 18:56:36 -0500 Subject: [PATCH 20/21] New unit test for logger using recordable (code cov) --- sdk/test/logs/logger_sdk_test.cc | 38 +++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/sdk/test/logs/logger_sdk_test.cc b/sdk/test/logs/logger_sdk_test.cc index afa130e86c..e1993f15ec 100644 --- a/sdk/test/logs/logger_sdk_test.cc +++ b/sdk/test/logs/logger_sdk_test.cc @@ -34,13 +34,34 @@ TEST(LoggerSDK, LogToNullProcessor) logger->Log("Test log"); } -class DummyProcessor : public LogProcessor +class MockProcessor final : public LogProcessor { +private: + std::shared_ptr record_received_; + +public: + // A processor used for testing that keeps a track of the recordable it received + explicit MockProcessor(std::shared_ptr record_received) noexcept + : record_received_(record_received) + {} + std::unique_ptr MakeRecordable() noexcept { return std::unique_ptr(new LogRecord); } - void OnReceive(std::unique_ptr &&record) noexcept {} + // OnReceive stores the record it receives into the shared_ptr recordable passed into its + // constructor + void OnReceive(std::unique_ptr &&record) noexcept + { + // Cast the recordable received into a concrete LogRecord type + auto copy = std::shared_ptr(static_cast(record.release())); + + // Copy over the received log record's name, body, timestamp fields over to the recordable + // passed in the constructor + record_received_->SetSeverity(copy->GetSeverity()); + record_received_->SetBody(copy->GetBody()); + record_received_->SetTimestamp(copy->GetTimestamp()); + } bool ForceFlush(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept { return true; @@ -65,11 +86,16 @@ TEST(LoggerSDK, LogToAProcessor) ASSERT_EQ(logger, logger2); // Set a processor for the LoggerProvider - std::shared_ptr processor = std::shared_ptr(new DummyProcessor()); + auto shared_recordable = std::shared_ptr(new LogRecord()); + auto processor = std::shared_ptr(new MockProcessor(shared_recordable)); lp->SetProcessor(processor); ASSERT_EQ(processor, lp->GetProcessor()); - // Should later introduce a way to assert that - // the logger's processor is the same as "proc" - // and that the logger's processor is the same as lp's processor + // Check that the recordable created by the Log() statement is set properly + opentelemetry::core::SystemTimestamp now(std::chrono::system_clock::now()); + logger->Log(opentelemetry::logs::Severity::kWarn, "Message", now); + + ASSERT_EQ(shared_recordable->GetSeverity(), opentelemetry::logs::Severity::kWarn); + ASSERT_EQ(shared_recordable->GetBody(), "Message"); + ASSERT_EQ(shared_recordable->GetTimestamp().time_since_epoch(), now.time_since_epoch()); } From b28a759d0d095d61712ecc9111daa835373fc091 Mon Sep 17 00:00:00 2001 From: Karen Xu Date: Sat, 12 Dec 2020 21:39:24 -0500 Subject: [PATCH 21/21] Move attribute utils from logs to common, remove unused headers --- .../sdk/common/attribute_utils.h | 102 ++++++++++++++++ .../opentelemetry/sdk/logs/attribute_utils.h | 114 ------------------ .../opentelemetry/sdk/logs/log_record.h | 10 +- sdk/include/opentelemetry/sdk/logs/logger.h | 8 +- sdk/src/logs/logger.cc | 5 +- 5 files changed, 111 insertions(+), 128 deletions(-) create mode 100644 sdk/include/opentelemetry/sdk/common/attribute_utils.h delete mode 100644 sdk/include/opentelemetry/sdk/logs/attribute_utils.h diff --git a/sdk/include/opentelemetry/sdk/common/attribute_utils.h b/sdk/include/opentelemetry/sdk/common/attribute_utils.h new file mode 100644 index 0000000000..6d791d909f --- /dev/null +++ b/sdk/include/opentelemetry/sdk/common/attribute_utils.h @@ -0,0 +1,102 @@ +#pragma once + +#include +#include +#include "opentelemetry/common/attribute_value.h" +#include "opentelemetry/common/key_value_iterable_view.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace common +{ +/** + * A counterpart to AttributeValue that makes sure a value is owned. This + * replaces all non-owning references with owned copies. + */ +using OwnedAttributeValue = nostd::variant, + std::vector, + std::vector, + std::vector, + std::vector, + std::vector, + std::vector>; + +/** + * Creates an owned copy (OwnedAttributeValue) of a non-owning AttributeValue. + */ +struct AttributeConverter +{ + OwnedAttributeValue operator()(bool v) { return OwnedAttributeValue(v); } + OwnedAttributeValue operator()(int32_t v) { return OwnedAttributeValue(v); } + OwnedAttributeValue operator()(uint32_t v) { return OwnedAttributeValue(v); } + OwnedAttributeValue operator()(int64_t v) { return OwnedAttributeValue(v); } + OwnedAttributeValue operator()(uint64_t v) { return OwnedAttributeValue(v); } + OwnedAttributeValue operator()(double v) { return OwnedAttributeValue(v); } + OwnedAttributeValue operator()(nostd::string_view v) + { + return OwnedAttributeValue(std::string(v)); + } + OwnedAttributeValue operator()(nostd::span v) { return convertSpan(v); } + OwnedAttributeValue operator()(nostd::span v) { return convertSpan(v); } + OwnedAttributeValue operator()(nostd::span v) { return convertSpan(v); } + OwnedAttributeValue operator()(nostd::span v) { return convertSpan(v); } + OwnedAttributeValue operator()(nostd::span v) { return convertSpan(v); } + OwnedAttributeValue operator()(nostd::span v) { return convertSpan(v); } + OwnedAttributeValue operator()(nostd::span v) + { + return convertSpan(v); + } + + template + OwnedAttributeValue convertSpan(nostd::span vals) + { + const std::vector copy(vals.begin(), vals.end()); + return OwnedAttributeValue(std::move(copy)); + } +}; + +/** + * Class for storing attributes. + */ +class AttributeMap +{ +public: + // Contruct empty attribute map + AttributeMap(){}; + + // Contruct attribute map and populate with attributes + AttributeMap(const opentelemetry::common::KeyValueIterable &attributes) + { + attributes.ForEachKeyValue([&](nostd::string_view key, + opentelemetry::common::AttributeValue value) noexcept { + SetAttribute(key, value); + return true; + }); + } + + const std::unordered_map &GetAttributes() const noexcept + { + return attributes_; + } + + void SetAttribute(nostd::string_view key, + const opentelemetry::common::AttributeValue &value) noexcept + { + attributes_[std::string(key)] = nostd::visit(converter_, value); + } + +private: + std::unordered_map attributes_; + AttributeConverter converter_; +}; +} // namespace common +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/logs/attribute_utils.h b/sdk/include/opentelemetry/sdk/logs/attribute_utils.h deleted file mode 100644 index 770d8412e4..0000000000 --- a/sdk/include/opentelemetry/sdk/logs/attribute_utils.h +++ /dev/null @@ -1,114 +0,0 @@ -#pragma once - -#include -#include -#include "opentelemetry/common/attribute_value.h" -#include "opentelemetry/common/key_value_iterable_view.h" - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace sdk -{ -namespace logs -{ -/** - * A counterpart to AttributeValue that makes sure a value is owned. This - * replaces all non-owning references with owned copies. - */ -using LogRecordAttributeValue = nostd::variant, - std::vector, - std::vector, - std::vector, - std::vector, - std::vector, - std::vector>; - -/** - * Creates an owned copy (LogRecordAttributeValue) of a non-owning AttributeValue. - */ -struct AttributeConverter -{ - LogRecordAttributeValue operator()(bool v) { return LogRecordAttributeValue(v); } - LogRecordAttributeValue operator()(int32_t v) { return LogRecordAttributeValue(v); } - LogRecordAttributeValue operator()(uint32_t v) { return LogRecordAttributeValue(v); } - LogRecordAttributeValue operator()(int64_t v) { return LogRecordAttributeValue(v); } - LogRecordAttributeValue operator()(uint64_t v) { return LogRecordAttributeValue(v); } - LogRecordAttributeValue operator()(double v) { return LogRecordAttributeValue(v); } - LogRecordAttributeValue operator()(nostd::string_view v) - { - return LogRecordAttributeValue(std::string(v)); - } - LogRecordAttributeValue operator()(nostd::span v) { return convertSpan(v); } - LogRecordAttributeValue operator()(nostd::span v) - { - return convertSpan(v); - } - LogRecordAttributeValue operator()(nostd::span v) - { - return convertSpan(v); - } - LogRecordAttributeValue operator()(nostd::span v) - { - return convertSpan(v); - } - LogRecordAttributeValue operator()(nostd::span v) - { - return convertSpan(v); - } - LogRecordAttributeValue operator()(nostd::span v) { return convertSpan(v); } - LogRecordAttributeValue operator()(nostd::span v) - { - return convertSpan(v); - } - - template - LogRecordAttributeValue convertSpan(nostd::span vals) - { - const std::vector copy(vals.begin(), vals.end()); - return LogRecordAttributeValue(std::move(copy)); - } -}; - -/** - * Class for storing attributes. - */ -class AttributeMap -{ -public: - // Contruct empty attribute map - AttributeMap(){}; - - // Contruct attribute map and populate with attributes - AttributeMap(const opentelemetry::common::KeyValueIterable &attributes) - { - attributes.ForEachKeyValue([&](nostd::string_view key, - opentelemetry::common::AttributeValue value) noexcept { - SetAttribute(key, value); - return true; - }); - } - - const std::unordered_map &GetAttributes() const noexcept - { - return attributes_; - } - - void SetAttribute(nostd::string_view key, - const opentelemetry::common::AttributeValue &value) noexcept - { - attributes_[std::string(key)] = nostd::visit(converter_, value); - } - -private: - std::unordered_map attributes_; - AttributeConverter converter_; -}; -} // namespace logs -} // namespace sdk -OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/logs/log_record.h b/sdk/include/opentelemetry/sdk/logs/log_record.h index 73f71dd744..4cef50ca90 100644 --- a/sdk/include/opentelemetry/sdk/logs/log_record.h +++ b/sdk/include/opentelemetry/sdk/logs/log_record.h @@ -18,7 +18,7 @@ #include #include -#include "opentelemetry/sdk/logs/attribute_utils.h" // same as traces/attribute_utils +#include "opentelemetry/sdk/common/attribute_utils.h" // same as traces/attribute_utils #include "opentelemetry/sdk/logs/recordable.h" #include "opentelemetry/version.h" @@ -40,8 +40,8 @@ class LogRecord final : public Recordable // Default values are set by the respective data structures' constructors for all fields, // except the severity field, which must be set manually (an enum with no default value). opentelemetry::logs::Severity severity_ = opentelemetry::logs::Severity::kInvalid; - AttributeMap resource_map_; - AttributeMap attributes_map_; + common::AttributeMap resource_map_; + common::AttributeMap attributes_map_; std::string name_; std::string body_; // Currently a simple string, but should be changed to "Any" type opentelemetry::trace::TraceId trace_id_; @@ -153,7 +153,7 @@ class LogRecord final : public Recordable * Get the resource field for this log * @return the resource field for this log */ - const std::unordered_map &GetResource() const noexcept + const std::unordered_map &GetResource() const noexcept { return resource_map_.GetAttributes(); } @@ -162,7 +162,7 @@ class LogRecord final : public Recordable * Get the attributes for this log * @return the attributes for this log */ - const std::unordered_map &GetAttributes() const noexcept + const std::unordered_map &GetAttributes() const noexcept { return attributes_map_.GetAttributes(); } diff --git a/sdk/include/opentelemetry/sdk/logs/logger.h b/sdk/include/opentelemetry/sdk/logs/logger.h index bf10374b08..385ba3586b 100644 --- a/sdk/include/opentelemetry/sdk/logs/logger.h +++ b/sdk/include/opentelemetry/sdk/logs/logger.h @@ -17,13 +17,9 @@ #pragma once #include "opentelemetry/logs/logger.h" -#include "opentelemetry/nostd/shared_ptr.h" -#include "opentelemetry/nostd/string_view.h" -#include "opentelemetry/sdk/common/atomic_shared_ptr.h" #include "opentelemetry/sdk/logs/logger_provider.h" #include "opentelemetry/sdk/logs/processor.h" -#include #include OPENTELEMETRY_BEGIN_NAMESPACE @@ -60,8 +56,8 @@ class Logger final : public opentelemetry::logs::Logger void Log(opentelemetry::logs::Severity severity, nostd::string_view name, nostd::string_view body, - const common::KeyValueIterable &resource, - const common::KeyValueIterable &attributes, + const opentelemetry::common::KeyValueIterable &resource, + const opentelemetry::common::KeyValueIterable &attributes, trace::TraceId trace_id, trace::SpanId span_id, trace::TraceFlags trace_flags, diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index 5c2818deb1..6e201da04e 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - #include "opentelemetry/sdk/logs/logger.h" #include "opentelemetry/sdk/logs/log_record.h" #include "opentelemetry/trace/provider.h" @@ -36,8 +35,8 @@ Logger::Logger(opentelemetry::nostd::string_view name, void Logger::Log(opentelemetry::logs::Severity severity, nostd::string_view name, nostd::string_view body, - const common::KeyValueIterable &resource, - const common::KeyValueIterable &attributes, + const opentelemetry::common::KeyValueIterable &resource, + const opentelemetry::common::KeyValueIterable &attributes, opentelemetry::trace::TraceId trace_id, opentelemetry::trace::SpanId span_id, opentelemetry::trace::TraceFlags trace_flags,