Repository: symfony/monolog-bundle (php)

Issue:
Collision between ElasticSearchHandler and ElasticaHandler
Hi,

I'm trying to set up monolog-bundle with native Elasticsearch cliente (introduced in monolog 2.0).
But it seems that I'm doing something wrong and I can't make it work.

Reviewing the code itself I've seen the following lines in MonologExtension:

https://github.com/symfony/monolog-bundle/blob/fde12fc628162787a4e53877abadc30047fd868b/DependencyInjection/MonologExtension.php#L983-L1011

There you can see that "elasticsearch" is always replaced with the ElasticaHandler in the array merge on line 1006.
Apart from that, I think the elastic handler should be ElasticsearchHandler, not ElasticSearchHandler (note te captial "S").

I locally did those changes but couldn't make it work and I'm lost.
Does symfony/monolog-bundle support the native Elasticsearch client? Or am I doing something wrong?


Many many thanks for your help

Pull request:
Deprecate elasticsearch type, and split it into elastica and elastic_search types
Fixes #424

Patch:
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index 53917e0..5d3ac1e 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -18,6 +18,9 @@ jobs:
                     -   php: '7.4'
                         deps: lowest
                         deprecations: max[self]=0
+                    -   php: '8.1'
+                        deps: highest
+                        monolog: '3.*'
         steps:
             -   name: Checkout
                 uses: actions/checkout@v2
diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
index eb22f4c..7dea024 100644
--- a/DependencyInjection/Configuration.php
+++ b/DependencyInjection/Configuration.php
@@ -81,7 +81,18 @@ use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
  *   - [level]: level name or int value, defaults to DEBUG
  *   - [bubble]: bool, defaults to true
  *
- * - elasticsearch:
+ * - elastic_search:
+ *   - elasticsearch:
+ *      - id: optional if host is given
+ *      - host: elastic search host name, with scheme (e.g. "https://127.0.0.1:9200")
+ *      - [user]: elastic search user name
+ *      - [password]: elastic search user password
+ *   - [index]: index name, defaults to monolog
+ *   - [document_type]: document_type, defaults to logs
+ *   - [level]: level name or int value, defaults to DEBUG
+ *   - [bubble]: bool, defaults to true
+ *
+ * - elastica:
  *   - elasticsearch:
  *      - id: optional if host is given
  *      - host: elastic search host name. Do not prepend with http(s)://
@@ -878,9 +889,9 @@ class Configuration implements ConfigurationInterface
                     ->thenInvalid('What must be set is either the host or the id.')
                     ->end()
                 ->end()
-                ->scalarNode('index')->defaultValue('monolog')->end() // elasticsearch
-                ->scalarNode('document_type')->defaultValue('logs')->end() // elasticsearch
-                ->scalarNode('ignore_error')->defaultValue(false)->end() // elasticsearch
+                ->scalarNode('index')->defaultValue('monolog')->end() // elasticsearch & elastic_search & elastica
+                ->scalarNode('document_type')->defaultValue('logs')->end() // elasticsearch & elastic_search & elastica
+                ->scalarNode('ignore_error')->defaultValue(false)->end() // elasticsearch & elastic_search & elastica
             ->end()
         ;
     }
@@ -1035,17 +1046,18 @@ class Configuration implements ConfigurationInterface
                                          $verbosity
                                     ));
                                 }
-                                if (!is_numeric($level)) {
-                                    $levelConstant = 'Monolog\Logger::'.$level;
-                                    if (!defined($levelConstant)) {
-                                        throw new InvalidConfigurationException(sprintf(
-                                            'The configured minimum log level "%s" for verbosity "%s" is invalid as it is not defined in Monolog\Logger.',
-                                             $level, $verbosity
-                                        ));
+
+                                try {
+                                    if (Logger::API === 3) {
+                                        $level = Logger::toMonologLevel($level)->value;
+                                    } else {
+                                        $level = Logger::toMonologLevel(is_numeric($level) ? (int) $level : $level);
                                     }
-                                    $level = constant($levelConstant);
-                                } else {
-                                    $level = (int) $level;
+                                } catch (\Psr\Log\InvalidArgumentException $e) {
+                                    throw new InvalidConfigurationException(sprintf(
+                                        'The configured minimum log level "%s" for verbosity "%s" is invalid as it is not defined in Monolog\Logger.',
+                                         $level, $verbosity
+                                    ));
                                 }
 
                                 $map[constant($verbosityConstant)] = $level;
diff --git a/DependencyInjection/MonologExtension.php b/DependencyInjection/MonologExtension.php
index d1f49d9..f323939 100644
--- a/DependencyInjection/MonologExtension.php
+++ b/DependencyInjection/MonologExtension.php
@@ -305,38 +305,50 @@ class MonologExtension extends Extension
             break;
 
         case 'elasticsearch':
+            @trigger_error('The "elasticsearch" handler type is deprecated in MonologBundle since version 3.8.0, use the "elastica" type instead, or switch to the official Elastic client using the "elastic_search" type.', E_USER_DEPRECATED);
+            // no break
+
+        case 'elastica':
+        case 'elastic_search':
             if (isset($handler['elasticsearch']['id'])) {
-                $elasticaClient = new Reference($handler['elasticsearch']['id']);
+                $client = new Reference($handler['elasticsearch']['id']);
             } else {
-                // elastica client new definition
-                $elasticaClient = new Definition('Elastica\Client');
-                $elasticaClientArguments = [
-                    'host' => $handler['elasticsearch']['host'],
-                    'port' => $handler['elasticsearch']['port'],
-                    'transport' => $handler['elasticsearch']['transport'],
-                ];
-
-                if (isset($handler['elasticsearch']['user'], $handler['elasticsearch']['password'])) {
-                    $elasticaClientArguments = array_merge(
-                        $elasticaClientArguments,
-                        [
-                            'headers' => [
-                                'Authorization' => 'Basic ' . base64_encode($handler['elasticsearch']['user'] . ':' . $handler['elasticsearch']['password'])
-                            ]
-                        ]
-                    );
+                if ($handler['type'] === 'elastic_search') {
+                    // v8 has a new Elastic\ prefix
+                    $client = new Definition(class_exists('Elastic\Elasticsearch\Client') ? 'Elastic\Elasticsearch\Client' : 'Elasticsearch\Client');
+                    $clientArguments = [
+                        'host' => $handler['elasticsearch']['host'],
+                    ];
+
+                    if (isset($handler['elasticsearch']['user'], $handler['elasticsearch']['password'])) {
+                        $clientArguments['basicAuthentication'] = [$handler['elasticsearch']['user'], $handler['elasticsearch']['password']];
+                    }
+                } else {
+                    $client = new Definition('Elastica\Client');
+
+                    $clientArguments = [
+                        'host' => $handler['elasticsearch']['host'],
+                        'port' => $handler['elasticsearch']['port'],
+                        'transport' => $handler['elasticsearch']['transport'],
+                    ];
+
+                    if (isset($handler['elasticsearch']['user'], $handler['elasticsearch']['password'])) {
+                        $clientArguments['headers'] = [
+                            'Authorization' => 'Basic ' . base64_encode($handler['elasticsearch']['user'] . ':' . $handler['elasticsearch']['password'])
+                        ];
+                    }
                 }
 
-                $elasticaClient->setArguments([
-                    $elasticaClientArguments
+                $client->setArguments([
+                    $clientArguments
                 ]);
 
-                $elasticaClient->setPublic(false);
+                $client->setPublic(false);
             }
 
             // elastica handler definition
             $definition->setArguments([
-                $elasticaClient,
+                $client,
                 [
                     'index' => $handler['index'],
                     'type' => $handler['document_type'],
@@ -1021,7 +1033,9 @@ class MonologExtension extends Extension
         ];
 
         $v2HandlerTypesAdded = [
+            'elastica' => 'Monolog\Handler\ElasticaHandler',
             'elasticsearch' => 'Monolog\Handler\ElasticaHandler',
+            'elastic_search' => 'Monolog\Handler\ElasticsearchHandler',
             'fallbackgroup' => 'Monolog\Handler\FallbackGroupHandler',
             'noop' => 'Monolog\Handler\NoopHandler',
         ];
@@ -1032,22 +1046,36 @@ class MonologExtension extends Extension
             'slackbot',
         ];
 
-        if (Logger::API === 2) {
+        $v3HandlerTypesRemoved = [
+            'swift_mailer',
+        ];
+
+        if (Logger::API >= 2) {
             $typeToClassMapping = array_merge($typeToClassMapping, $v2HandlerTypesAdded);
             foreach($v2HandlerTypesRemoved as $v2HandlerTypeRemoved) {
                 unset($typeToClassMapping[$v2HandlerTypeRemoved]);
             }
         }
 
+        if (Logger::API >= 3) {
+            foreach($v3HandlerTypesRemoved as $v3HandlerTypeRemoved) {
+                unset($typeToClassMapping[$v3HandlerTypeRemoved]);
+            }
+        }
+
         if (!isset($typeToClassMapping[$handlerType])) {
             if (Logger::API === 1 && array_key_exists($handlerType, $v2HandlerTypesAdded)) {
                 throw new \InvalidArgumentException(sprintf('"%s" was added in Monolog v2, please upgrade if you wish to use it.', $handlerType));
             }
 
-            if (Logger::API === 2 && array_key_exists($handlerType, $v2HandlerTypesRemoved)) {
+            if (Logger::API >= 2 && array_key_exists($handlerType, $v2HandlerTypesRemoved)) {
                 throw new \InvalidArgumentException(sprintf('"%s" was removed in Monolog v2.', $handlerType));
             }
 
+            if (Logger::API >= 3 && array_key_exists($handlerType, $v3HandlerTypesRemoved)) {
+                throw new \InvalidArgumentException(sprintf('"%s" was removed in Monolog v3.', $handlerType));
+            }
+
             throw new \InvalidArgumentException(sprintf('There is no handler class defined for handler "%s".', $handlerType));
         }
 
diff --git a/composer.json b/composer.json
index 1298311..d30d233 100644
--- a/composer.json
+++ b/composer.json
@@ -21,7 +21,7 @@
         "symfony/dependency-injection": "^4.4 || ^5.0 || ^6.0",
         "symfony/config": "~4.4 || ^5.0 || ^6.0",
         "symfony/http-kernel": "~4.4 || ^5.0 || ^6.0",
-        "monolog/monolog": "~1.22 || ~2.0"
+        "monolog/monolog": "^1.22 || ^2.0 || ^3.0"
     },
     "require-dev": {
         "symfony/yaml": "~4.4 || ^5.0 || ^6.0",


Test patch:
diff --git a/Tests/DependencyInjection/ConfigurationTest.php b/Tests/DependencyInjection/ConfigurationTest.php
index 71d3601..c9b6a23 100644
--- a/Tests/DependencyInjection/ConfigurationTest.php
+++ b/Tests/DependencyInjection/ConfigurationTest.php
@@ -191,6 +191,10 @@ class ConfigurationTest extends TestCase
 
     public function testWithSwiftMailerHandler()
     {
+        if (\Monolog\Logger::API >= 3) {
+            $this->markTestSkipped('This test requires Monolog v1 or v2');
+        }
+
         $configs = [
             [
                 'handlers' => [
@@ -273,7 +277,7 @@ class ConfigurationTest extends TestCase
                         'verbosity_levels' => [
                             'VERBOSITY_NORMAL' => 'NOTICE',
                             'verbosity_verbose' => 'info',
-                            'VERBOSITY_very_VERBOSE' => 150
+                            'VERBOSITY_very_VERBOSE' => '200'
                         ]
                     ]
                 ]
@@ -286,7 +290,7 @@ class ConfigurationTest extends TestCase
         $this->assertSame([
             OutputInterface::VERBOSITY_NORMAL => Logger::NOTICE,
             OutputInterface::VERBOSITY_VERBOSE => Logger::INFO,
-            OutputInterface::VERBOSITY_VERY_VERBOSE => 150,
+            OutputInterface::VERBOSITY_VERY_VERBOSE => 200,
             OutputInterface::VERBOSITY_QUIET => Logger::ERROR,
             OutputInterface::VERBOSITY_DEBUG => Logger::DEBUG
             ], $config['handlers']['console']['verbosity_levels']);
diff --git a/Tests/DependencyInjection/FixtureMonologExtensionTest.php b/Tests/DependencyInjection/FixtureMonologExtensionTest.php
index 5515ff2..3cbb192 100644
--- a/Tests/DependencyInjection/FixtureMonologExtensionTest.php
+++ b/Tests/DependencyInjection/FixtureMonologExtensionTest.php
@@ -184,6 +184,10 @@ abstract class FixtureMonologExtensionTest extends DependencyInjectionTest
 
     public function testSingleEmailRecipient()
     {
+        if (\Monolog\Logger::API >= 3) {
+            $this->markTestSkipped('This test requires Monolog v1 or v2');
+        }
+
         $container = $this->getContainer('single_email_recipient');
 
         $this->assertEquals([
@@ -212,6 +216,10 @@ abstract class FixtureMonologExtensionTest extends DependencyInjectionTest
 
     public function testMultipleEmailRecipients()
     {
+        if (\Monolog\Logger::API >= 3) {
+            $this->markTestSkipped('This test requires Monolog v1 or v2');
+        }
+
         $container = $this->getContainer('multiple_email_recipients');
 
         $this->assertEquals([
@@ -249,7 +257,7 @@ abstract class FixtureMonologExtensionTest extends DependencyInjectionTest
     public function testPsr3MessageProcessingDisabledOnNullHandler()
     {
         if (\Monolog\Logger::API < 2) {
-            $this->markTestSkipped('This test requires Monolog v2');
+            $this->markTestSkipped('This test requires Monolog v2 or above');
         }
         $container = $this->getContainer('process_psr_3_messages_null');
 
@@ -263,7 +271,7 @@ abstract class FixtureMonologExtensionTest extends DependencyInjectionTest
     public function testHandlersV2()
     {
         if (\Monolog\Logger::API < 2) {
-            $this->markTestSkipped('This test requires Monolog v2');
+            $this->markTestSkipped('This test requires Monolog v2 or above');
         }
         $this->getContainer('handlers');
 
diff --git a/Tests/DependencyInjection/MonologExtensionTest.php b/Tests/DependencyInjection/MonologExtensionTest.php
index 50d9758..52307b9 100644
--- a/Tests/DependencyInjection/MonologExtensionTest.php
+++ b/Tests/DependencyInjection/MonologExtensionTest.php
@@ -297,8 +297,8 @@ class MonologExtensionTest extends DependencyInjectionTest
 
     public function testRavenHandlerWhenConfigurationIsWrong()
     {
-        if (Logger::API === 2) {
-            $this->markTestSkipped('Not valid for V2');
+        if (Logger::API !== 1) {
+            $this->markTestSkipped('Only valid on Monolog V1');
 
             return;
         }
@@ -313,8 +313,8 @@ class MonologExtensionTest extends DependencyInjectionTest
 
     public function testRavenHandlerWhenADSNIsSpecified()
     {
-        if (Logger::API === 2) {
-            $this->markTestSkipped('Not valid for V2');
+        if (Logger::API !== 1) {
+            $this->markTestSkipped('Only valid on Monolog V1');
 
             return;
         }
@@ -339,8 +339,8 @@ class MonologExtensionTest extends DependencyInjectionTest
 
     public function testRavenHandlerWhenADSNAndAClientAreSpecified()
     {
-        if (Logger::API === 2) {
-            $this->markTestSkipped('Not valid for V2');
+        if (Logger::API !== 1) {
+            $this->markTestSkipped('Only valid on Monolog V1');
 
             return;
         }
@@ -359,8 +359,8 @@ class MonologExtensionTest extends DependencyInjectionTest
 
     public function testRavenHandlerWhenAClientIsSpecified()
     {
-        if (Logger::API === 2) {
-            $this->markTestSkipped('Not valid for V2');
+        if (Logger::API !== 1) {
+            $this->markTestSkipped('Only valid on Monolog V1');
 
             return;
         }
@@ -692,8 +692,8 @@ class MonologExtensionTest extends DependencyInjectionTest
      */
     public function testV2AddedOnV1($handlerType)
     {
-        if (Logger::API === 2) {
-            $this->markTestSkipped('Not valid for V2');
+        if (Logger::API !== 1) {
+            $this->markTestSkipped('Only valid on Monolog V1');
 
             return;
         }