From 9c767df50754a0066f57b6a0add1656669a0ee1c Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Thu, 16 May 2024 21:48:14 +0200 Subject: [PATCH] [API] Return NoopLogRecord from NoopLogger (#2668) Co-authored-by: Yuri Shkuro --- api/include/opentelemetry/logs/event_logger.h | 4 --- api/include/opentelemetry/logs/logger.h | 4 --- api/include/opentelemetry/logs/noop.h | 30 ++++++++++++++++++- api/test/logs/logger_test.cc | 4 ++- sdk/src/logs/logger.cc | 6 ---- 5 files changed, 32 insertions(+), 16 deletions(-) diff --git a/api/include/opentelemetry/logs/event_logger.h b/api/include/opentelemetry/logs/event_logger.h index 2ede6f3fa1..988142fbe2 100644 --- a/api/include/opentelemetry/logs/event_logger.h +++ b/api/include/opentelemetry/logs/event_logger.h @@ -64,10 +64,6 @@ class EventLogger return; } nostd::unique_ptr log_record = delegate_logger->CreateLogRecord(); - if (!log_record) - { - return; - } IgnoreTraitResult( detail::LogRecordSetterTrait::type>::template Set( diff --git a/api/include/opentelemetry/logs/logger.h b/api/include/opentelemetry/logs/logger.h index 403e884272..6448ff6901 100644 --- a/api/include/opentelemetry/logs/logger.h +++ b/api/include/opentelemetry/logs/logger.h @@ -100,10 +100,6 @@ class Logger void EmitLogRecord(ArgumentType &&... args) { nostd::unique_ptr log_record = CreateLogRecord(); - if (!log_record) - { - return; - } EmitLogRecord(std::move(log_record), std::forward(args)...); } diff --git a/api/include/opentelemetry/logs/noop.h b/api/include/opentelemetry/logs/noop.h index b4590c987f..9876f40bfd 100644 --- a/api/include/opentelemetry/logs/noop.h +++ b/api/include/opentelemetry/logs/noop.h @@ -34,11 +34,39 @@ class NoopLogger final : public Logger public: const nostd::string_view GetName() noexcept override { return "noop logger"; } - nostd::unique_ptr CreateLogRecord() noexcept override { return nullptr; } + nostd::unique_ptr CreateLogRecord() noexcept override + { + /* + * Do not return memory shared between threads, + * a `new` + `delete` for each noop record can not be avoided, + * due to the semantic of unique_ptr. + */ + return nostd::unique_ptr(new NoopLogRecord()); + } using Logger::EmitLogRecord; void EmitLogRecord(nostd::unique_ptr &&) noexcept override {} + +private: + class NoopLogRecord : public LogRecord + { + public: + NoopLogRecord() = default; + ~NoopLogRecord() override = default; + + void SetTimestamp(common::SystemTimestamp /* timestamp */) noexcept override {} + void SetObservedTimestamp(common::SystemTimestamp /* timestamp */) noexcept override {} + void SetSeverity(logs::Severity /* severity */) noexcept override {} + void SetBody(const common::AttributeValue & /* message */) noexcept override {} + void SetAttribute(nostd::string_view /* key */, + const common::AttributeValue & /* value */) noexcept override + {} + void SetEventId(int64_t /* id */, nostd::string_view /* name */) noexcept override {} + void SetTraceId(const trace::TraceId & /* trace_id */) noexcept override {} + void SetSpanId(const trace::SpanId & /* span_id */) noexcept override {} + void SetTraceFlags(const trace::TraceFlags & /* trace_flags */) noexcept override {} + }; }; /** diff --git a/api/test/logs/logger_test.cc b/api/test/logs/logger_test.cc index a9567a6d3f..43d7afc685 100644 --- a/api/test/logs/logger_test.cc +++ b/api/test/logs/logger_test.cc @@ -28,9 +28,11 @@ TEST(Logger, GetLoggerDefault) auto lp = Provider::GetLoggerProvider(); const std::string schema_url{"https://opentelemetry.io/schemas/1.11.0"}; auto logger = lp->GetLogger("TestLogger", "opentelelemtry_library", "", schema_url); - auto name = logger->GetName(); EXPECT_NE(nullptr, logger); + auto name = logger->GetName(); EXPECT_EQ(name, "noop logger"); + auto record = logger->CreateLogRecord(); + EXPECT_NE(nullptr, record); } // Test the two additional overloads for GetLogger() diff --git a/sdk/src/logs/logger.cc b/sdk/src/logs/logger.cc index 7f85ddd6a3..972b380efc 100644 --- a/sdk/src/logs/logger.cc +++ b/sdk/src/logs/logger.cc @@ -33,12 +33,6 @@ const nostd::string_view Logger::GetName() noexcept nostd::unique_ptr Logger::CreateLogRecord() noexcept { - // If this logger does not have a processor, no need to create a log recordable - if (!context_) - { - return nullptr; - } - auto recordable = context_->GetProcessor().MakeRecordable(); recordable->SetObservedTimestamp(std::chrono::system_clock::now());