Resolve #22 by using contributed new function from BohwaZ to fix depr…

[seamuslee001]

…ecation issue in php8.1

ping @jparise not sure what you think of this and the package I got was found here https://packagist.org/packages/php81_bc/strftime which was based off this gist https://gist.github.com/bohwaz/42fc223031e2b2dd2585aab159a20f30

[seamuslee001]

so there seem to be some difficulties with supporting this on 5.6 but also not sure how important it is to still support PHP5.6 these days

[williamdes]

Ref: https://bugs.launchpad.net/ubuntu/+source/php-log/+bug/2006748

[williamdes]

It's a bit overkill?
There is no other way?

[seamuslee001]

@williamdes I think previously I found that if someone had pulled in https://packagist.org/packages/php81_bc/strftime via composer then without this if there were errors

[jparise]

This is a pretty heavy solution (in terms of both the code dependency and runtime overhead).

I think I'd prefer a solution like this:

• Choose an "ideal" formatter (e.g. date_format) that we'd want to use going forward.
• Provide a helper function that converts existing strftime format strings to the new format, and use it to automatically "upgrade" existing timeFormat configuration values to the new dateFormat configuration format when logger instances are constructed (and warn when the old timeFormat keys are used).
• Significantly bump the package version to reflect these changes.

[seamuslee001]

@jparise so the php8.1-strftime.php function is designed to do the first part of 2. As for the 2nd part do we assume that if they have % in the string it will be the strftime format or not?

[jparise]

@jparise so the php8.1-strftime.php function is designed to do the first part of 2. As for the 2nd part do we assume that if they have % in the string it will be the strftime format or not?

I would differentiate the two formats based on configuration key names: timeFormat vs dateFormat.

[seamuslee001]

@jparise given that strftime did locale work as well as formatting a date do you think this might be appropriate https://www.php.net/manual/en/intldateformatter.format.php it would mean having to have php-intl installed or should we do some kind of fall back process if no php-intl use date_format?

[jparise]

@jparise given that strftime did locale work as well as formatting a date do you think this might be appropriate https://www.php.net/manual/en/intldateformatter.format.php it would mean having to have php-intl installed or should we do some kind of fall back process if no php-intl use date_format?

I'm not sure I see a strong benefit to internationalization support in log lines. The output isn't meant to be user-facing.

[seamuslee001]

I'm happy to drop the locale support if it is your preference I was just thinking about it from a consistency point of view

[totten]

I'm not sure I see a strong benefit to internationalization support in log lines. The output isn't meant to be user-facing.

That's kind of the rub (as per "80-20 rule") - date-time functionality (generally conceived) includes elements that are internationalized. But for logging specifically, you could hardcode one format (ISO 8601) and it would serve 80% of the use-cases. It's the last 20% that's harder. There are edge-cases around the PHP version, PHP extensions+packages, timezones, tzdata, locales, locale data, formatting-placeholders... not to mention the subjective expectations of the various downstream apps+users.

FWIW... you might consider a property timeFormatter (@var callable):

/**
 * @var callable
 */
var $_timeFormatter;

public function __construct(...) {
    ...
    if (!empty($conf['timeFormatter'])) {
        $this->_timeFormatter = $conf['timeFormatter'];
    } elseif (!empty($conf['timeFormat'])) {
       // Support for older `timeFormat`. Only useful on systems that support strftime() well. 
       $this->_timeFormatter = function() {
          return strftime($conf['timeFormat']);
        };
    }
    else {
        // Traditional default format
        $this->_timeFormatter = function() { return date("M d H:i:s"); }
    }

That still accepts timeFormat. It gives the same default behavior as always, and the default is compatible with all environments. For the long-tail of timezone/locale/version/dependency issues, it allows the downstream to choose a balance.

$log = new Log_foo(..., [
    // Basic formatting rule
    'timeFormatter' => fn() => date('Y-m-d H:i:s O'),

    // Alternative formatting rules. Diff approaches to timezones, locales, placeholders, dependencies, etc
    // 'timeFormatter' => fn() => gmdate('Y-m-d H:i:s O'),
    // 'timeFormatter' => fn() => gmdate('c'),
    // 'timeFormatter' => fn() => strftime("%b %d %H:%M:%S"),
    // 'timeFormatter' => fn() => \PHP81_BC\strftime("%b %d %H:%M:%S", NULL, 'C')),
    // 'timeFormatter' => fn() => \PHP81_BC\strftime("%b %d %H:%M:%S", NULL, $GLOBALS['myapp']->getStandardTimezone())),
]);

---

Anyway, that's a suggestion. But the other approaches are also fine.

[jparise]

I'm happy to drop the locale support if it is your preference I was just thinking about it from a consistency point of view

Yes, understood. My reaction was primarily about a dependency on intl being available, which seems like a unnecessarily strong requirement for a logging framework.

I have no general concern about providing locale-aware timestamps in log output.

[sveneld]

Hi.
So what you decided? Will be current pull request merged or another solution will be provided?

[jparise]

I don't plan on merging this pull request in its current form, but it could be revised based on the discussion above.

[sveneld]

@jparise look my pull request #25

[jparise]

Thanks for your contribution! I'm going to close this in favor of the approach used by #25.