Use logger channel services consistently, instead of the factory object

How would you have a dynamic channel in that case?

I have some generic services for dealing with migrations and I have one channel per migration, fetched dynamically from the factory. This lets me present migration specific logs in the UI.

If we had to declare the services ahead of time that would mean I'd need my own factory, which feels awkward when core already has

Perhaps we should document when to use one pattern over the other?

Sounds good to me

I postponed #3254307: [PP-1] Make clearer that classes in contributed modules should not generally add the LoggerChannelTrait trait on this because I think that however we change the documentation it should reflect the best practices implemented here.

Reran the tests and appears to be 1 failure.

Sweet!

Seeing all green and the changes look good to me. Really need a better phrase but all I got right now haha.

Is there any assertion we could add that would prevent the coverage issue from #12 from regressing?

I grepped the remaining calls to get('logger.factory'). Most look unrelated, or are getting a channel dynamically, or are looping over N channels, or doing other stuff for tests. However, there is this:

core/lib/Drupal/Core/DrupalKernel.php:      $this->container->get('logger.factory')->get('DrupalKernel')->error('Container cannot be saved to cache.');

I realize the Drupal kernel is not a module, but could we use the same pattern there?

Also in non-module instances, the entity reference field formatter: EntityReferenceEntityFormatter:

        $this->loggerFactory->get('entity')->error('Recursive rendering detected when rendering entity %entity_type: %entity_id, using the %field_name field on the %parent_entity_type:%parent_bundle %parent_entity_id entity. Aborting rendering.', [
          '%entity_type' => $entity->getEntityTypeId(),
          '%entity_id' => $entity->id(),
          '%field_name' => $items->getName(),
          '%parent_entity_type' => $items->getFieldDefinition()->getTargetEntityTypeId(),
          '%parent_bundle' => $items->getFieldDefinition()->getTargetBundle(),
          '%parent_entity_id' => $items->getEntity()->id(),
        ]);

Then there are a couple usages in media still where the logger factory is injected as a service. In MediaEmbed:

    if (static::$recursiveRenderDepth[$recursive_render_id] > EntityReferenceEntityFormatter::RECURSIVE_RENDER_LIMIT) {
      $this->loggerFactory->get('media')->error('During rendering of embedded media: recursive rendering detected for %entity_id. Aborting rendering.', [
        '%entity_id' => $media->id(),
      ]);
      return [];
    }
  }

In OEmbedResourceConstraintValidator:

  /**                                                                           
   * Constructs a new OEmbedResourceConstraintValidator.                        
   *                                                                            
   * @param \Drupal\media\OEmbed\UrlResolverInterface $url_resolver             
   *   The oEmbed URL resolver service.                                         
   * @param \Drupal\media\OEmbed\ResourceFetcherInterface $resource_fetcher     
   *   The resource fetcher service.                                            
   * @param \Drupal\Core\Logger\LoggerChannelFactoryInterface $logger_factory   
   *   The logger service.                                                      
   */
  public function __construct(UrlResolverInterface $url_resolver, ResourceFetcherInterface $resource_fetcher, LoggerChannelFactoryInterface $logger_factory) {
    $this->urlResolver = $url_resolver;
    $this->resourceFetcher = $resource_fetcher;
    $this->logger = $logger_factory->get('media');
  }

@longwave (#3)

My longer-term thinking on this also ties into autowiring. We can't autowire loggers directly because different services want to use different channels. But eventually we can use argument binding like this in e.g. user.services.yml:

services:
  _defaults:
    autowire: true
    bind:
      Psr\Log\LoggerInterface: '@logger.channel.user'

I would love to see this!
Do we have any issue where plan to add support for 'bind' in '_defaults'?
I did not find any. I only saw that in #3021898-18: Support _defaults key in service.yml files for public, tags and autowire settings it was removed from the PR of that issue.

Here we go,
#3336739: Service arguments by name or type in *.services.yml with autowiring
#3336746: Support arguments binding by name or type in *.services.yml files

What would be the equivalent logger channel service to \Drupal::logger('file system') ?

UPD: I think it has to be declared with a proper name and keep using space within an argument.
See #3073684: [pp-1] Deprecate system_check_directory() where introduced logger.channel.file_system channel service.

The reason I prefer injecting the factory is performance, instantiating a channel that isn't used (and log channels are very often not used, only when something goes wrong) is fairly expensive last time I checked, Or at least the first time it happens it's expensive.

It's similar to entity storages and config objects although definitely not that expensive. We changed core to not create entity storages in __construct() and it's generally discouraged to instantiate config objects in __construct() too and store the factory instead.

Can we not use service closures for this problem per https://www.drupal.org/node/3451855 ?