Make the map of PSR3 to RFC 5424 log level constants available as a constant

Created on 22 March 2024, 2 months ago
Updated 5 May 2024, 28 days ago

Problem/Motivation

Drupal has a map of PSR3 log level constants to RFC 5424 log level constants, but hides it in a protected variable. This map should arguably be available as a constant.

Steps to reproduce

Proposed resolution

Deprecate the Drupal\Core\Logger\LoggerChannel::$levelTranslation protected variable in favor of a new constant Drupal\Core\Logger\LoggerChannel::LEVEL_TRANSLATION (or Drupal\Core\Logger\LoggerChannel::LEVEL_MAP)

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

✨ Feature request
Status

Needs work

Version

11.0 πŸ”₯

Component
BaseΒ  β†’

Last updated less than a minute ago

Created by

πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @mfb
  • Status changed to Needs review 2 months ago
  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco
  • Pipeline finished with Success
    2 months ago
    Total: 608s
    #125840
  • Status changed to RTBC 2 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Change to a constant makes sense.

  • Status changed to Needs review 2 months ago
  • πŸ‡¦πŸ‡ΊAustralia mstrelan

    Wonder if we'd be better off making RfcLogLevel in to an enum and move the mapping to a function there. Setting NR for the idea, please set to NW if you agree or back to RTBC if not.

  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco

    @mstrelan I think you're suggesting an API change, if the RfcLogLevels would be objects rather than simple constants? That doesn't seem worth the developer friction to do.

    Ok, I guess you could have an enum that also has the original constants pointing to the enum values, and there would be no API change, but then, what's the point?

    Note also that Drupal\Core\Logger\RfcLogLevel is basically drupal's (moar RFC-compliant) version of Psr\Log\LogLevel, so it seems reasonable for them to have the same architecture.

  • Merge request !7178Draft: RfcLogLevel Enum β†’ (Open) created by mstrelan
  • πŸ‡¦πŸ‡ΊAustralia mstrelan

    This is what I had in mind, happy to be told it's not worth it, just thought it was worth considering.

    https://git.drupalcode.org/project/drupal/-/merge_requests/7178

  • Pipeline finished with Failed
    2 months ago
    Total: 177s
    #129060
  • Status changed to Needs work 2 months ago
  • The Needs Review Queue Bot β†’ tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide β†’ to find step-by-step guides for working with issues.

  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco

    @mstrelan: Seems not worth it for the reasons I said in #6 but, if you want to discuss you could open a new issue? It seems only tangentially related to this issue, which was just to make the level map available, not to change any public APIs.

  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco

    Well, in the contrib module that this would've been useful for, I ended up introducing my own LogLevel enum to manage all the log level logic :)

    So, I don't really need this anymore; feel free to close this issue as far as I'm concerned (or, perhaps the original feature request could still be useful for other developers).

  • πŸ‡³πŸ‡ΏNew Zealand quietone New Zealand

    This looks familiar. Searching found an earlier issue for this which I closed as a duplicate.

  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco

    @quietone I actually suggested closing this issue, as it was pretty trivial to copy the map into two contrib modules where this feature could've been useful. Btw, I don't see how that other issue is a duplicate?

Production build 0.69.0 2024