Repository: open-telemetry/opentelemetry-php (php)

Issue:
log errors/warnings to console if no logger provided?
I noticed that java sends logs to console (with default severity INFO) if no other logger has been configured
https://opentelemetry.io/docs/instrumentation/java/manual/#logging-and-error-handling

Since our default behaviour is to not emit errors and warnings unless a user explicitly provides a psr3 logger, sending to console might be more helpful.

Pull request:
default logging to console
Rather than not logging by default, log to console via trigger_error (>= warning), and allow the LoggerHolder mechanism to be used for greater control over logging (including disabling, eg with a NullLogger)
Closes #897

Patch:
diff --git a/examples/traces/troubleshooting/setting_up_logging.php b/examples/traces/troubleshooting/setting_up_logging.php
index 49f0539f..8cd4f3c1 100644
--- a/examples/traces/troubleshooting/setting_up_logging.php
+++ b/examples/traces/troubleshooting/setting_up_logging.php
@@ -5,14 +5,14 @@ require __DIR__ . '/../../../vendor/autoload.php';
 
 use Monolog\Handler\StreamHandler;
 use Monolog\Logger;
-use OpenTelemetry\SDK\Common\Log\LoggerHolder;
+use OpenTelemetry\API\Common\Log\LoggerHolder;
 use OpenTelemetry\SDK\Trace\TracerProviderFactory;
 use Psr\Log\LogLevel;
 
 echo 'Starting SettingUpLogging example' . PHP_EOL;
 
-//create a Logger, and register it with library's logger holder. The library will use this logger
-//for all of its internal logging (errors, warnings, etc)
+//By default, opentelemetry's internal logging (errors, warnings, etc) will be output to console.
+//You can instead provide a psr-3 logger to provide greater control of logging output:
 LoggerHolder::set(
     new Logger('otel-php', [new StreamHandler(STDOUT, LogLevel::DEBUG)])
 );
diff --git a/src/API/Behavior/LogsMessagesTrait.php b/src/API/Behavior/LogsMessagesTrait.php
index 9efa4c2f..adeb9603 100644
--- a/src/API/Behavior/LogsMessagesTrait.php
+++ b/src/API/Behavior/LogsMessagesTrait.php
@@ -9,10 +9,40 @@ use Psr\Log\LogLevel;
 
 trait LogsMessagesTrait
 {
+    private static function shouldLog(string $level): bool
+    {
+        return in_array($level, [LogLevel::ERROR, LogLevel::WARNING, LogLevel::CRITICAL, LogLevel::EMERGENCY]);
+    }
+
+    private static function map(string $level)
+    {
+        switch ($level) {
+            case LogLevel::WARNING:
+                return E_USER_WARNING;
+            case LogLevel::ERROR:
+            case LogLevel::CRITICAL:
+            case LogLevel::EMERGENCY:
+                return E_USER_ERROR;
+            default:
+                return E_USER_NOTICE;
+        }
+    }
+
     private static function doLog(string $level, string $message, array $context): void
     {
-        $context['source'] = get_called_class();
-        LoggerHolder::get()->log($level, $message, $context);
+        $logger = LoggerHolder::get();
+        if ($logger !== null) {
+            $context['source'] = get_called_class();
+            $logger->log($level, $message, $context);
+        } elseif (self::shouldLog($level)) {
+            $message = sprintf(
+                '%s: %s in %s',
+                $message,
+                (array_key_exists('exception', $context) && $context['exception'] instanceof \Throwable) ? $context['exception']->getMessage() : '',
+                get_called_class()
+            );
+            trigger_error($message, self::map($level));
+        }
     }
 
     protected static function logDebug(string $message, array $context = []): void
diff --git a/src/API/Common/Log/LoggerHolder.php b/src/API/Common/Log/LoggerHolder.php
index c39214b0..6d2c8c48 100644
--- a/src/API/Common/Log/LoggerHolder.php
+++ b/src/API/Common/Log/LoggerHolder.php
@@ -21,13 +21,14 @@ final class LoggerHolder
 
     /**
      * @suppress PhanTypeMismatchReturnNullable
+     * @internal
      */
-    public static function get(): LoggerInterface
+    public static function get(): ?LoggerInterface
     {
-        return self::$logger ?? new NullLogger();
+        return self::$logger;
     }
 
-    public static function set(LoggerInterface $logger): void
+    public static function set(?LoggerInterface $logger): void
     {
         self::$logger = $logger;
     }


Test patch:
diff --git a/tests/Unit/SDK/Behavior/LogsMessagesTraitTest.php b/tests/Unit/API/Behavior/LogsMessagesTraitTest.php
similarity index 61%
rename from tests/Unit/SDK/Behavior/LogsMessagesTraitTest.php
rename to tests/Unit/API/Behavior/LogsMessagesTraitTest.php
index 26158f0b..a517e427 100644
--- a/tests/Unit/SDK/Behavior/LogsMessagesTraitTest.php
+++ b/tests/Unit/API/Behavior/LogsMessagesTraitTest.php
@@ -2,7 +2,7 @@
 
 declare(strict_types=1);
 
-namespace OpenTelemetry\Tests\Unit\SDK\Behavior;
+namespace OpenTelemetry\Tests\Unit\API\Behavior;
 
 use OpenTelemetry\API\Behavior\LogsMessagesTrait;
 use OpenTelemetry\API\Common\Log\LoggerHolder;
@@ -30,6 +30,30 @@ class LogsMessagesTraitTest extends TestCase
         LoggerHolder::unset();
     }
 
+    public function test_defaults_to_trigger_error_with_warning(): void
+    {
+        $message = 'something went wrong';
+        LoggerHolder::unset();
+        $this->assertFalse(LoggerHolder::isSet());
+        $instance = $this->createInstance();
+
+        $this->expectWarning();
+        $this->expectWarningMessageMatches(sprintf('/%s/', $message));
+        $instance->run('logWarning', 'foo', ['exception' => new \Exception($message)]);
+    }
+
+    public function test_defaults_to_trigger_error_with_error(): void
+    {
+        $message = 'something went wrong';
+        LoggerHolder::unset();
+        $this->assertFalse(LoggerHolder::isSet());
+        $instance = $this->createInstance();
+
+        $this->expectError();
+        $this->expectErrorMessageMatches(sprintf('/%s/', $message));
+        $instance->run('logError', 'foo', ['exception' => new \Exception($message)]);
+    }
+
     /**
      * @testdox Proxies logging methods through to logger
      * @dataProvider logLevelProvider
@@ -60,9 +84,9 @@ class LogsMessagesTraitTest extends TestCase
         return new class() {
             use LogsMessagesTrait;
             //accessor for protected trait methods
-            public function run(string $method, string $message): void
+            public function run(string $method, string $message, array $context = []): void
             {
-                $this->{$method}($message);
+                $this->{$method}($message, $context);
             }
         };
     }
diff --git a/tests/Unit/API/Trace/Propagation/TraceContextPropagatorTest.php b/tests/Unit/API/Trace/Propagation/TraceContextPropagatorTest.php
index d2a1b39e..46716336 100644
--- a/tests/Unit/API/Trace/Propagation/TraceContextPropagatorTest.php
+++ b/tests/Unit/API/Trace/Propagation/TraceContextPropagatorTest.php
@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace OpenTelemetry\Tests\Unit\API\Trace\Propagation;
 
+use OpenTelemetry\API\Common\Log\LoggerHolder;
 use OpenTelemetry\API\Trace\Propagation\TraceContextPropagator;
 use OpenTelemetry\API\Trace\SpanContext;
 use OpenTelemetry\API\Trace\SpanContextInterface;
@@ -14,6 +15,7 @@ use OpenTelemetry\Context\Context;
 use OpenTelemetry\Context\ContextInterface;
 use OpenTelemetry\SDK\Trace\Span;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
 
 /**
  * @covers \OpenTelemetry\API\Trace\Propagation\TraceContextPropagator
@@ -32,6 +34,7 @@ class TraceContextPropagatorTest extends TestCase
 
     protected function setUp(): void
     {
+        LoggerHolder::set(new NullLogger());
         $this->traceContextPropagator = TraceContextPropagator::getInstance();
         $this->traceState = (new TraceState())->with('bar', 'baz')->with('foo', 'bar');
     }
diff --git a/tests/Unit/API/Trace/TraceStateTest.php b/tests/Unit/API/Trace/TraceStateTest.php
index 96ad3d48..926fe324 100644
--- a/tests/Unit/API/Trace/TraceStateTest.php
+++ b/tests/Unit/API/Trace/TraceStateTest.php
@@ -4,8 +4,10 @@ declare(strict_types=1);
 
 namespace OpenTelemetry\Tests\API\Unit\Trace;
 
+use OpenTelemetry\API\Common\Log\LoggerHolder;
 use OpenTelemetry\API\Trace\TraceState;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
 use function str_repeat;
 use function strlen;
 
@@ -14,6 +16,11 @@ use function strlen;
  */
 class TraceStateTest extends TestCase
 {
+    public function setUp(): void
+    {
+        LoggerHolder::set(new NullLogger());
+    }
+
     public function test_get_tracestate_value(): void
     {
         $tracestate = new TraceState('vendor1=value1');
diff --git a/tests/Unit/SDK/Metrics/MeterProviderFactoryTest.php b/tests/Unit/SDK/Metrics/MeterProviderFactoryTest.php
index 1ca27d13..cad3791b 100644
--- a/tests/Unit/SDK/Metrics/MeterProviderFactoryTest.php
+++ b/tests/Unit/SDK/Metrics/MeterProviderFactoryTest.php
@@ -5,11 +5,13 @@ declare(strict_types=1);
 namespace OpenTelemetry\Example\Unit\SDK\Metrics;
 
 use AssertWell\PHPUnitGlobalState\EnvironmentVariables;
+use OpenTelemetry\API\Common\Log\LoggerHolder;
 use OpenTelemetry\API\Metrics\MeterInterface;
 use OpenTelemetry\SDK\Common\Configuration\KnownValues;
 use OpenTelemetry\SDK\Common\Configuration\Variables;
 use OpenTelemetry\SDK\Metrics\MeterProviderFactory;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
 
 /**
  * @covers \OpenTelemetry\SDK\Metrics\MeterProviderFactory
@@ -18,6 +20,11 @@ class MeterProviderFactoryTest extends TestCase
 {
     use EnvironmentVariables;
 
+    public function setUp(): void
+    {
+        LoggerHolder::set(new NullLogger());
+    }
+
     public function tearDown(): void
     {
         $this->restoreEnvironmentVariables();
diff --git a/tests/Unit/SDK/Propagation/PropagatorFactoryTest.php b/tests/Unit/SDK/Propagation/PropagatorFactoryTest.php
index b5767052..db627037 100644
--- a/tests/Unit/SDK/Propagation/PropagatorFactoryTest.php
+++ b/tests/Unit/SDK/Propagation/PropagatorFactoryTest.php
@@ -6,6 +6,7 @@ namespace OpenTelemetry\Tests\Unit\SDK\Propagation;
 
 use AssertWell\PHPUnitGlobalState\EnvironmentVariables;
 use OpenTelemetry\API\Baggage\Propagation\BaggagePropagator;
+use OpenTelemetry\API\Common\Log\LoggerHolder;
 use OpenTelemetry\API\Trace\Propagation\TraceContextPropagator;
 use OpenTelemetry\Context\Propagation\MultiTextMapPropagator;
 use OpenTelemetry\Context\Propagation\NoopTextMapPropagator;
@@ -14,6 +15,7 @@ use OpenTelemetry\SDK\Common\Configuration\KnownValues;
 use OpenTelemetry\SDK\Common\Configuration\Variables;
 use OpenTelemetry\SDK\Propagation\PropagatorFactory;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
 
 /**
  * @covers OpenTelemetry\SDK\Propagation\PropagatorFactory
@@ -22,6 +24,11 @@ class PropagatorFactoryTest extends TestCase
 {
     use EnvironmentVariables;
 
+    public function setUp(): void
+    {
+        LoggerHolder::set(new NullLogger());
+    }
+
     public function tearDown(): void
     {
         $this->restoreEnvironmentVariables();
diff --git a/tests/Unit/SDK/SdkAutoloaderTest.php b/tests/Unit/SDK/SdkAutoloaderTest.php
index 24ecb50d..1960cee4 100644
--- a/tests/Unit/SDK/SdkAutoloaderTest.php
+++ b/tests/Unit/SDK/SdkAutoloaderTest.php
@@ -5,9 +5,11 @@ declare(strict_types=1);
 namespace OpenTelemetry\Tests\Unit\SDK;
 
 use AssertWell\PHPUnitGlobalState\EnvironmentVariables;
+use OpenTelemetry\API\Common\Log\LoggerHolder;
 use OpenTelemetry\SDK\Common\Configuration\Variables;
 use OpenTelemetry\SDK\SdkAutoloader;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
 
 /**
  * @covers \OpenTelemetry\SDK\SdkAutoloader
@@ -16,6 +18,11 @@ class SdkAutoloaderTest extends TestCase
 {
     use EnvironmentVariables;
 
+    public function setUp(): void
+    {
+        LoggerHolder::set(new NullLogger());
+    }
+
     public function tearDown(): void
     {
         SdkAutoloader::shutdown();
diff --git a/tests/Unit/SDK/Trace/SpanExporter/AbstractExporterTest.php b/tests/Unit/SDK/Trace/SpanExporter/AbstractExporterTest.php
index 2adbd99c..ce138fcb 100644
--- a/tests/Unit/SDK/Trace/SpanExporter/AbstractExporterTest.php
+++ b/tests/Unit/SDK/Trace/SpanExporter/AbstractExporterTest.php
@@ -6,12 +6,14 @@ namespace OpenTelemetry\Tests\Unit\SDK\Trace\SpanExporter;
 
 use Mockery;
 use Mockery\Adapter\Phpunit\MockeryTestCase;
+use OpenTelemetry\API\Common\Log\LoggerHolder;
 use OpenTelemetry\SDK\Common\Export\TransportInterface;
 use OpenTelemetry\SDK\Common\Future\CompletedFuture;
 use OpenTelemetry\SDK\Common\Future\ErrorFuture;
 use OpenTelemetry\SDK\Common\Future\FutureInterface;
 use OpenTelemetry\SDK\Trace\SpanExporterInterface;
 use OpenTelemetry\Tests\Unit\SDK\Util\SpanData;
+use Psr\Log\NullLogger;
 
 /**
  * @psalm-suppress UndefinedInterfaceMethod
@@ -23,6 +25,7 @@ abstract class AbstractExporterTest extends MockeryTestCase
 
     public function setUp(): void
     {
+        LoggerHolder::set(new NullLogger());
         $this->future = Mockery::mock(FutureInterface::class);
         $this->future->allows([
             'map' => $this->future,
diff --git a/tests/Unit/SDK/Trace/SpanProcessor/BatchSpanProcessorTest.php b/tests/Unit/SDK/Trace/SpanProcessor/BatchSpanProcessorTest.php
index ed857baf..f9687b36 100644
--- a/tests/Unit/SDK/Trace/SpanProcessor/BatchSpanProcessorTest.php
+++ b/tests/Unit/SDK/Trace/SpanProcessor/BatchSpanProcessorTest.php
@@ -31,6 +31,7 @@ use OpenTelemetry\SDK\Trace\SpanProcessorInterface;
 use OpenTelemetry\Tests\Unit\SDK\Util\TestClock;
 use Psr\Log\LoggerInterface;
 use Psr\Log\LogLevel;
+use Psr\Log\NullLogger;
 
 /**
  * @covers \OpenTelemetry\SDK\Trace\SpanProcessor\BatchSpanProcessor
@@ -41,6 +42,7 @@ class BatchSpanProcessorTest extends MockeryTestCase
 
     protected function setUp(): void
     {
+        LoggerHolder::set(new NullLogger());
         $this->testClock = new TestClock();
 
         ClockFactory::setDefault($this->testClock);
diff --git a/tests/Unit/SDK/Trace/SpanProcessor/SimpleSpanProcessorTest.php b/tests/Unit/SDK/Trace/SpanProcessor/SimpleSpanProcessorTest.php
index f09a75de..174a186f 100644
--- a/tests/Unit/SDK/Trace/SpanProcessor/SimpleSpanProcessorTest.php
+++ b/tests/Unit/SDK/Trace/SpanProcessor/SimpleSpanProcessorTest.php
@@ -21,6 +21,7 @@ use OpenTelemetry\SDK\Trace\SpanProcessor\SimpleSpanProcessor;
 use OpenTelemetry\Tests\Unit\SDK\Util\SpanData;
 use Psr\Log\LoggerInterface;
 use Psr\Log\LogLevel;
+use Psr\Log\NullLogger;
 
 /**
  * @covers OpenTelemetry\SDK\Trace\SpanProcessor\SimpleSpanProcessor
@@ -43,6 +44,7 @@ class SimpleSpanProcessorTest extends MockeryTestCase
 
     protected function setUp(): void
     {
+        LoggerHolder::set(new NullLogger());
         $this->readWriteSpan = Mockery::mock(ReadWriteSpanInterface::class);
         $this->readableSpan = Mockery::mock(ReadableSpanInterface::class);