[API] Return NoopLogRecord from NoopLogger (#2668)

marcalff · yurishkuro · web-flow · commit 9c767df50754 · 2024-05-16T21:48:14.000+02:00
Co-authored-by: Yuri Shkuro &lt;yurishkuro@users.noreply.github.com&gt;
diff --git 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<LogRecord> log_record = delegate_logger->CreateLogRecord();
-    if (!log_record)
-    {
-      return;
-    }
 
     IgnoreTraitResult(
         detail::LogRecordSetterTrait<typename std::decay<ArgumentType>::type>::template Set(
diff --git 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<LogRecord> log_record = CreateLogRecord();
-    if (!log_record)
-    {
-      return;
-    }
 
     EmitLogRecord(std::move(log_record), std::forward<ArgumentType>(args)...);
   }
diff --git 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<LogRecord> CreateLogRecord() noexcept override { return nullptr; }
+  nostd::unique_ptr<LogRecord> 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<LogRecord>(new NoopLogRecord());
+  }
 
   using Logger::EmitLogRecord;
 
   void EmitLogRecord(nostd::unique_ptr<LogRecord> &&) 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
@@ -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
@@ -33,12 +33,6 @@ const nostd::string_view Logger::GetName() noexcept
 
 nostd::unique_ptr<opentelemetry::logs::LogRecord> 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());

Original file line number	Diff line number	Diff line change
`@@ -100,10 +100,6 @@ class Logger`
`100`	`100`	`void EmitLogRecord(ArgumentType &&... args)`
`101`	`101`	`{`
`102`	`102`	`nostd::unique_ptr<LogRecord> log_record = CreateLogRecord();`
`103`		`- if (!log_record)`
`104`		`- {`
`105`		`- return;`
`106`		`- }`
`107`	`103`
`108`	`104`	`EmitLogRecord(std::move(log_record), std::forward<ArgumentType>(args)...);`
`109`	`105`	`}`