LoggerInterface - Last Feedback Call

[Jordi Boggiano]

Heya,

I just updated everything according to the feedback from last week and
according to the bylaw proposal for naming things.

PR for the spec: https://github.com/php-fig/fig-standards/pull/60
Spec as HTML:
https://github.com/Seldaek/fig-standards/blob/logger-interface/proposed/logger-interface.md
psr/log package: https://github.com/Seldaek/log

I would like to start a vote in a few days tops unless someone has a
major objection.


The only feedback point I personally still want to discuss is this one
from Larry Garfield:

> * This may have been discussed and I missed it, or it was only
> oblique, but doesn't having a context key un-wrapped in % increase
> the potential for collisions? Wouldn't it make more sense to do:
>
> $log->error('The user %user% broke things.', array('%user%' =>
> 'Larry'));
>
> That way, any context key with %-wrappings is automatically
> understood to be intended as a substituted value.

Right now it is specified as:

$log->error('The user %user% broke things.', array('user' => 'Larry'));

IMO it is fine as is because the chance of having %foo% in the message
while *not* wanting to have it replaced by a key combined with having a
random context array that contains the foo as a key by accident are
quite slim. That said I can understand Larry's point, but nobody
answered him on that so I wanted to bring attention to it again.

Cheers

--
Jordi Boggiano
@seldaek - http://nelm.io/jordi

[guilher...@gmail.com]

Regarding the variable replacement, I'm pro using ICU's MessageFormatter. =)

http://docs.php.net/messageformatter
http://userguide.icu-project.org/formatparse/messages

Cheers,

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To post to this group, send email to php...@googlegroups.com.
To unsubscribe from this group, send email to php-fig+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

--
Guilherme Blanco
MSN: guilher...@hotmail.com
GTalk: guilhermeblanco
Toronto - ON/Canada

[Jordi Boggiano]

On 09.12.2012 20:45, guilher...@gmail.com wrote:
> Regarding the variable replacement, I'm pro using ICU's MessageFormatter. =)
>
> http://docs.php.net/messageformatter
> http://userguide.icu-project.org/formatparse/messages

I don't know what others think about this, but ICU's API is overly
complex for the logging use case IMO. We really don't need all the
flexibility, and for the simple case it seems quite painful to use. And
then there's the fact that intl isn't present on many installs which is
another pain.

[Sebastian Krebs]

2012/12/9 Jordi Boggiano <j.bog...@seld.be>

On 09.12.2012 20:45, guilher...@gmail.com wrote:
> Regarding the variable replacement, I'm pro using ICU's MessageFormatter. =)
>
> http://docs.php.net/messageformatter
> http://userguide.icu-project.org/formatparse/messages

I don't know what others think about this, but ICU's API is overly
complex for the logging use case IMO. We really don't need all the
flexibility, and for the simple case it seems quite painful to use. And
then there's the fact that intl isn't present on many installs which is
another pain.

Yes, It's a separate package on Ubuntu Debian and one cannot assume, that it is available.

 

Cheers

--
Jordi Boggiano
@seldaek - http://nelm.io/jordi

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To post to this group, send email to php...@googlegroups.com.
To unsubscribe from this group, send email to php-fig+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

--
github.com/KingCrunch

[Tim Otten]

On Dec 9, 2012, at 3:12 PM, Sebastian Krebs wrote:

2012/12/9 Jordi Boggiano <j.bog...@seld.be>

On 09.12.2012 20:45, guilher...@gmail.com wrote:
> Regarding the variable replacement, I'm pro using ICU's MessageFormatter. =)
>
> http://docs.php.net/messageformatter
> http://userguide.icu-project.org/formatparse/messages

I don't know what others think about this, but ICU's API is overly
complex for the logging use case IMO. We really don't need all the
flexibility, and for the simple case it seems quite painful to use. And
then there's the fact that intl isn't present on many installs which is
another pain.

Yes, It's a separate package on Ubuntu Debian and one cannot assume, that it is available.

It's also not part of the PHP builds included with MAMP or OS X. Between Debian-derivatives and OS X, I think that means the "msgfmt" library would be an obstacle for *most* (if not quite all) PHP developers. (Anecdotal evidence: based on sprints/trainings I've attended, Debian & OSX account for 50-80% of PHP devs.)

It would have been nice, though, to use a library that's part of PHP core.

[Paul Dragoonis]

I'm really happy with the proposal.

As for Larry's concern, I believe if someone is going to store arbitrary data with percentages in them, they should run their logging data through some kind of str_replace() to make sure that doesn't occur. Bit of an edge case imo.

Paul.

[Paul Jones]

On Dec 9, 2012, at 7:19 PM, Paul Dragoonis wrote:

> I'm really happy with the proposal.
>
> As for Larry's concern, I believe if someone is going to store arbitrary data with percentages in them, they should run their logging data through some kind of str_replace() to make sure that doesn't occur. Bit of an edge case imo.

I don't mean to be the party pooper here, but I'm really not a fan of the %delimiters%. Is there any way at all we can add a setDelimiters($left, $right) method to the interface? Not a deal breaker if not, but *man* it would be nice to let projects use their own delmiters. Lithium and others have {foo}, Aura has {:foo}, Drupal has %foo%, I'm sure someone out there has {{foo}}. It's the kind of thing that can be set once before the Logger implementation is injected into whatever needs it.


-- pmj

[Jordi Boggiano]

Heya,

> I don't mean to be the party pooper here, but I'm really not a fan of
> the %delimiters%. Is there any way at all we can add a
> setDelimiters($left, $right) method to the interface? Not a deal
> breaker if not, but *man* it would be nice to let projects use their
> own delmiters. Lithium and others have {foo}, Aura has {:foo},
> Drupal has %foo%, I'm sure someone out there has {{foo}}. It's the
> kind of thing that can be set once before the Logger implementation
> is injected into whatever needs it.

The way I see it, if Aura uses say Doctrine, and Doctrine would accept a
LoggerInterface and log stuff to it using %foo%, it just works with the
current proposal.

If Aura changes the delimiters, the Doctrine messages suddenly don't get
any replacement anymore.

On the other hand, if Aura wants to wrap the logger and also do
replacement on {:foo}, it probably won't hurt anyone since I guess
nobody will have this in their log message. The chance for conflict
could be reduced even more if you only do that in the logger you give to
the application/Aura code and not to libraries.

My point is that providing BC (or even keeping the current state forever
if you don't want to call it BC) in a framework is very easy since you
are usually in control of the logger. On the other hand, we need to
provide a set of baseline features for libraries so they can hook in
with reasonable expectations and don't have to jump through hoops to
make logging work.

[Paul Jones]

On Dec 10, 2012, at 1:28 AM, Jordi Boggiano wrote:

> Heya,
>
>> I don't mean to be the party pooper here, but I'm really not a fan of
>> the %delimiters%. Is there any way at all we can add a
>> setDelimiters($left, $right) method to the interface? Not a deal
>> breaker if not, but *man* it would be nice to let projects use their
>> own delmiters. Lithium and others have {foo}, Aura has {:foo},
>> Drupal has %foo%, I'm sure someone out there has {{foo}}. It's the
>> kind of thing that can be set once before the Logger implementation
>> is injected into whatever needs it.
>
> The way I see it, if Aura uses say Doctrine, and Doctrine would accept a
> LoggerInterface and log stuff to it using %foo%, it just works with the
> current proposal.
>
> If Aura changes the delimiters, the Doctrine messages suddenly don't get
> any replacement anymore.
>
> On the other hand, if Aura wants to wrap the logger and also do
> replacement on {:foo}, it probably won't hurt anyone since I guess
> nobody will have this in their log message. The chance for conflict
> could be reduced even more if you only do that in the logger you give to
> the application/Aura code and not to libraries.

I want to play this out a little bit to show what I mean, even if it's tilting at windmills. The underlying point I'm making is that, no matter what, you have to construct the logger to inject it into the object that needs logging, and that's the point at which you can set delimiters.

Let's say you have an implementation of LoggerInterface called logger, and two classes from two different projects that use different delimiters as a matter of project consistency: Foo (uses %token%) and Bar (uses {token}).

You can do this when building objects from each one:

class Foo
{
public function __construct(LoggerInterface $logger)
{
$logger->debug('Constructed %class%', array(
'class' => __CLASS__,
));
}
}

class Bar
{
public function __construct(LoggerInterface $logger)
{
$logger->debug('Constructed {class}', array(
'class' => __CLASS__,
));
}
}

// instantiate class Foo
$logfoo = new Logger;
$logfoo->setDelimiters('%', '%');
$foo = new Foo($logfoo);

// instantiate class Bar
$logbar = new Logger;
$logfoo->setDelimiters('{', '}');
$bar = new Foo($logbar);

So if Doctrine uses a logger with %foo% tokens, it's trivial to replace it with any other implementation, as long as you set the delimiters when constructing the implementation to be injected. You can use the same implementation with different tokens for different included libraries.

Looking at it now, there would need to be a getDelimiters() method too.

Hope that makes sense.


-- pmj

[Paul Dragoonis]

I'm happy with a setDelimiters() methods, it gives people flexibility and solves the problem of "what if my delimiter is X or Y"

[Jordi Boggiano]

Heya,

> So if Doctrine uses a logger with %foo% tokens, it's trivial to
> replace it with any other implementation, as long as you set the
> delimiters when constructing the implementation to be injected. You
> can use the same implementation with different tokens for different
> included libraries.

As I explained I can see why you would want to customize the delimiters
in the framework itself (BC and whatnot), and for that I think we can
agree it's doable without having the logger interface allow this. You
can easily wrap/extend the Logger.

For libraries, I would expect none of them currently use any delimiters
since there is no common interface to begin with. From what I have seen
either they have their own very simple interfaces or in most cases they
have no logging capability at all. Therefore having only one option in
the spec (%foo% and no customizability) makes it much easier for
everyone. You don't need to check if the delimiters are the right one,
or configure delimiters specifically for every library. Just inject the
logger and all is well. Then wrap it if needed for your framework's needs.

[Matthew Lanigan]

Does that actually result in an increase in interoperability, though? It seems that if each framework hard-codes those delimiters in their Logger implementations (which I believe is what you are suggesting -- please correct me if I've misinterpreted), interoperability is being significantly hindered, not helped along.

With PMJ's proposed setDelimiters() method, interoperability is inherently increased as nobody is required to rewrite anything when, for example, switching from a Logger implementation provided by one framework to one provided by another.

Matthew

 

Cheers

--
Jordi Boggiano
@seldaek - http://nelm.io/jordi

[Paul Jones]

On Dec 10, 2012, at 10:23 AM, Matthew Lanigan wrote:

> With PMJ's proposed setDelimiters() method, interoperability is inherently increased as nobody is required to rewrite anything when, for example, switching from a Logger implementation provided by one framework to one provided by another.

Right on.


-- pmj

[Jordi Boggiano]

> Does that actually result in an increase in interoperability, though? It
> seems that if each framework hard-codes those delimiters in their Logger
> implementations (which I believe is what you are suggesting -- please
> correct me if I've misinterpreted), interoperability is being
> significantly hindered, not helped along.
>
> With PMJ's proposed setDelimiters() method, interoperability is
> inherently increased as nobody is required to rewrite anything when, for
> example, switching from a Logger implementation provided by one
> framework to one provided by another.

If every library does a setDelimiters() call when the logger is
injected, you need at least one logger per library to avoid clashes.
That's not really helpful to interoperability as I see it.

What I mean by the framework wrapping the logger to provide its own
delimiters is that it should still support %foo% anymore, and just add
support for {foo} *in addition*, otherwise it's breaking the
interface/PSR which indeed would be bad for interoperability.

[Evert Pot]

On Dec 10, 2012, at 5:10 PM, Paul Dragoonis <drag...@gmail.com> wrote:

> I'm happy with a setDelimiters() methods, it gives people flexibility and solves the problem of "what if my delimiter is X or Y"

The implication is that it's no longer really possible to have a shared instance of the Logger. Everything that needs a Logger injected will need to get a fresh instance, as they may change the delimiter.

In addition, it may be logical to assume that everyone of these instances to a central logging object, which does the actual work. Every instance will need to convert their internal delimiters to the central logger, or the central logger will need to check which delimiters are currently active and then do the conversion to the internal format.

So the result is that one of two things need to happen:

1. Make sure that the PSR states that instances need to be unique for every object or context that requires logging.

Or:

2. As a best practice, always call setDelimiters before making any logging calls. (and maybe reset the delimiters back to what they were after use?)


Frankly, I feel neither of these are optional, and if either Paul or Paul feel strongly about using their own delimiters, they may be better off wrapping the Logging interface into an API for internal use.

After all, the only place where this is actually relevant is *within* Aura or PPI, it makes a lot more sense to keep the interface simple, and let you guys wrap in the interface.

Evert

[Moisa Teodor]

Hi all,

If wrappers/adapters are needed anyway, I wonder if it's of any use for the Logger interface to also handle substitution. The message could be pre-processed by the adapter just before it is passed to the Logger instance. The Log adapter could use a Logger instance that is injected.

While the Logger itself would be interchangeable with a different Logger implementation, the Adapter would be library specific, which is the way it should be, since different libraries have different logging requirements (some might need simple logging while others might need complex processing like log message localization).

What do you think ?

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To post to this group, send email to php...@googlegroups.com.
To unsubscribe from this group, send email to php-fig+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

--
Doru Moisa
web: http;//moisadoru.wordpress.com
tel: +40 720 861 922
Bucharest, Romania

[Paul Jones]

On Dec 10, 2012, at 10:22 AM, Evert Pot wrote:

> On Dec 10, 2012, at 5:10 PM, Paul Dragoonis <drag...@gmail.com> wrote:
>
>> I'm happy with a setDelimiters() methods, it gives people flexibility and solves the problem of "what if my delimiter is X or Y"
>
> The implication is that it's no longer really possible to have a shared instance of the Logger. Everything that needs a Logger injected will need to get a fresh instance, as they may change the delimiter.

If you want to share logger instances, you can place them in a service locator or DI container, one per set of delimiters used by the various libraries in your project. My guess is it's likely to be at most two or three, which I don't think is a heavy burden.



-- pmj

[Jordi Boggiano]

> If wrappers/adapters are needed anyway, I wonder if it's of any use for
> the Logger interface to also handle substitution. The message could be
> pre-processed by the adapter just before it is passed to the Logger
> instance. The Log adapter could use a Logger instance that is injected.

Wrappers/adapters are *not* needed. They are only needed if you want
custom delimiters, which I don't expect many people will want since many
frameworks/libs either don't do logging at all or do it without
placeholders.

> While the Logger itself would be interchangeable with a different Logger
> implementation, the Adapter would be library specific, which is the way
> it should be, since different libraries have different logging
> requirements (some might need simple logging while others might need
> complex processing like log message localization).
>
> What do you think ?

I think we already discussed this at length :) Localization and such
should be done at the log display, not on input. That means a system
like drupal requiring localization will use log handlers that store the
log records intact (including placeholders) and replace them at display
time.

If a library or framework wants custom delimiters, it can very well
received a raw LoggerInterface implementation and wrap it into its own
adapter at its convenience. Nobody from the outside needs to know. The
adapter can just do preg_replace('#\{(\w+)\}#', '%$1%', $message); and
then send the stuff along to the original logger object.

[Jordi Boggiano]

Heya,

> If you want to share logger instances, you can place them in a service locator or DI container, one per set of delimiters used by the various libraries in your project. My guess is it's likely to be at most two or three, which I don't think is a heavy burden.

That means you need to *know* about which library needs which kind of
delimiter. The interface becomes almost worthless because you can't just
give a logger to everyone that accepts one, you need to keep track of
their delimiters too.

On the other hand, as I just wrote in the previous mail, you can do that
wrapping as a logger consumer and in that case the LoggerInterface and
the outside world needs to know nothing about your custom placeholder
delimiters.

I really would not want to legitimize the use of custom delimiters,
because it makes life harder for everyone and provides barely any
benefit to those that really need the custom delimiters.

The point of standardization is that we all fall in line. Maybe it will
take 5 years, and until then you can ship an adapter with your code, no
big deal. But if we make it ok to be different, nobody will ever adjust.
Even worse, the new code will be written in various ways instead of one
unique way.

[Paul Jones]

On Dec 10, 2012, at 10:52 AM, Jordi Boggiano wrote:

> Heya,
>
>> If you want to share logger instances, you can place them in a service locator or DI container, one per set of delimiters used by the various libraries in your project. My guess is it's likely to be at most two or three, which I don't think is a heavy burden.
>
> That means you need to *know* about which library needs which kind of
> delimiter. The interface becomes almost worthless because you can't just
> give a logger to everyone that accepts one, you need to keep track of
> their delimiters too.
>
> On the other hand, as I just wrote in the previous mail, you can do that
> wrapping as a logger consumer and in that case the LoggerInterface and
> the outside world needs to know nothing about your custom placeholder
> delimiters.

/me nods

If that's the case, then we may not need to specify delimiters of any kind at all. The interface to access the logging methods is going to be separate from the actual storing of the logs. The storing layer can interpolate the tokens, or store them, or do what it likes with them.


-- pmj

[Johannes Schmitt]

The main benefit of placeholders is for small standalone libraries, frameworks can also benefit from this, although less as they might have very specific needs.

So, imo we should keep placeholders, but not allow multiple delimiters.

Cheers,
Johannes

[Paul Jones]

On Dec 10, 2012, at 10:35 AM, Moisa Teodor wrote:

> Hi all,
>
> If wrappers/adapters are needed anyway, I wonder if it's of any use for the Logger interface to also handle substitution. The message could be pre-processed by the adapter just before it is passed to the Logger instance. The Log adapter could use a Logger instance that is injected.
>
> While the Logger itself would be interchangeable with a different Logger implementation, the Adapter would be library specific, which is the way it should be, since different libraries have different logging requirements (some might need simple logging while others might need complex processing like log message localization).
>
> What do you think ?

It might look something like this; token replacement is in the writer, not the logger. This allows the LoggerInterface to be used without set/get delimiters, and you can drop in any writer/replacer you want for your project (and you can customize it when you need to handle messages from other projects). It gets the LoggerInterface (and the PSR) out of the business of specifying what projects should use as token delimiters.

<?php
class FileLogWriter implements LogWriterInterface
{
protected $file;
protected $replacer;
public function __construct($file, ReplacerInterface $replacer)
{
$this->file = $file;
$this->replacer = $replacer;
}
public function write($level, $message, $context)
{
$message = $level . ': '
. $this->replacer->replace($message, $context);
file_put_contents($this->file, $message . PHP_EOL, FILE_APPEND);
}
}

class Logger
{
protected $writer;
public function __construct(LogWriterInterface $writer)
{
$this->writer = $writer;
}
public function debug($message, $context)
{
$this->writer->write('debug', $message, $context);
}
}


-- pmj

[Bernhard Schussek]

I might be missing the point, but why don't we pass the parameters to the context in the same way as they appear in the message?

$logger->debug('Constructed %class%', array(

    '%class%' => __CLASS__,
));

Then everyone can use whatever he sees fit.

Bernhard

--

Bernhard Schussek
Blog: http://webmozarts.com
Twitter: http://twitter.com/webmozart

2012/12/10 Paul Jones <pmjo...@gmail.com>

[Jordi Boggiano]

On 10.12.2012 18:22, Bernhard Schussek wrote:
> I might be missing the point, but why don't we pass the parameters to
> the context in the same way as they appear in the message?
>
> $logger->debug('Constructed %class%', array(
> '%class%' => __CLASS__,
> ));
>
> Then everyone can use whatever he sees fit.

Using %class% as key for consistency is one thing, but I am not sure
what you suggest is a good idea, because that would mean the
implementation would just replace all keys in the context, which could
easily lead to conflicts with the message if you are not careful or use
user input and such as context data.

[Moisa Teodor]

Totally agree, a LogWriter interface would definitely add many benefits to the Logger package. 

Furthermore, the Logger interface could also be adapted to allow for using multiple LogWriters (for example when you need to log all log messages in a rolling file plus also send exceptional errors via email through a shutdown handler). 

Thanks!

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To post to this group, send email to php...@googlegroups.com.
To unsubscribe from this group, send email to php-fig+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Paul Jones]

On Dec 10, 2012, at 11:47 AM, Moisa Teodor wrote:

> Totally agree, a LogWriter interface would definitely add many benefits to the Logger package.

To be clear, I am *not* suggesting that we add a new interface to the proposal. I think the "writer" part of it is up to implementors. My only objection, such as it is, is to the specifying of particular delimiters within messages at the LoggerInterface level.


-- pmj

[Moisa Teodor]

Fair enough :)

On Mon, Dec 10, 2012 at 7:56 PM, Paul Jones <pmjo...@gmail.com> wrote:

To be clear, I am *not* suggesting that we add a new interface to the proposal.  I think the "writer" part of it is up to implementors.  My only objection, such as it is, is to the specifying of particular delimiters within messages at the LoggerInterface level.

--

[Kirill Chebunin]

I will add a huge portion of critic. But this is PSR. It should be close to perfect for users and vendors.

1. I still can't understand what is the difference between

$logger->debug('Some info with %parameter%', ['%parameter%' => $parameter]);
and
$logger->debug(sprintf('Some info with %s', $parameter), ['parameter' => $parameter]);
This feature adds really small advantage, but will require an implementation in ALL libraries with the same simple code.

2. We have methods like error, warn, etc in the interface, so ALL libraries will have to implement this methods, but this methods will have the same code in 99.9% of situations
public function error($message, array $context = [])
{
    $this->log(LogLevel::ERROR, $message, $context);
}
More over a client code will expect such logic.

So It's better to have an interface with 1 log($level, $message, array $context = array()) method and have a wrapper with placeholders, level shortcut methods etc. Than each library will implement only 1 main method.


3. Log levels do not have any order. It's better to use default int values instead of strings from linked rfc
              0       Emergency: system is unusable
              1       Alert: action must be taken immediately
              2       Critical: critical conditions
              3       Error: error conditions
              4       Warning: warning conditions
              5       Notice: normal but significant condition
              6       Informational: informational messages
              7       Debug: debug-level messages

4. Log level class can have default Enum methods to get all values, or get value from name, etc.

2012/12/10 Bernhard Schussek <bsch...@gmail.com>

--
Best Regards,
Kirill chEbba Chebunin,
Programmer to the bone,
Moscow, Russia.

[Bernhard Schussek]

2012/12/10 Jordi Boggiano <j.bog...@seld.be>

Using %class% as key for consistency is one thing, but I am not sure
what you suggest is a good idea, because that would mean the
implementation would just replace all keys in the context, which could
easily lead to conflicts with the message if you are not careful or use
user input and such as context data.

As long as you don't use the user input as context *keys* you should be fine, not? (and as long as you choose the pattern wisely) For the record, the Symfony Translator is doing exactly the same thing.

[Lukas Kahwe Smith]

On Dec 10, 2012, at 20:25 , Kirill Chebunin <i...@chebba.org> wrote:

> I will add a huge portion of critic. But this is PSR. It should be close to perfect for users and vendors.
>
> 1. I still can't understand what is the difference between
>
> $logger->debug('Some info with %parameter%', ['%parameter%' => $parameter]);
> and
> $logger->debug(sprintf('Some info with %s', $parameter), ['parameter' => $parameter]);
> This feature adds really small advantage, but will require an implementation in ALL libraries with the same simple code.

as was explained the reason is so that its possible to translate the message on display rather than on write. to be able to do this the static part of the message needs to be logged separately from the parameters.

> 2. We have methods like error, warn, etc in the interface, so ALL libraries will have to implement this methods, but this methods will have the same code in 99.9% of situations
> public function error($message, array $context = [])
> {
> $this->log(LogLevel::ERROR, $message, $context);
> }
> More over a client code will expect such logic.
>
> So It's better to have an interface with 1 log($level, $message, array $context = array()) method and have a wrapper with placeholders, level shortcut methods etc. Than each library will implement only 1 main method.

this was also discussed before its simply more convenient for users to have separate methods and given the little effort it requires implementors to deal with this, it makes sense to tailor things to users of the API.

> 3. Log levels do not have any order. It's better to use default int values instead of strings from linked rfc
> 0 Emergency: system is unusable
> 1 Alert: action must be taken immediately
> 2 Critical: critical conditions
> 3 Error: error conditions
> 4 Warning: warning conditions
> 5 Notice: normal but significant condition
> 6 Informational: informational messages
> 7 Debug: debug-level messages

i am not sure about the full pro's and con's here, but i prefer strings as the numbers on their own are meaningless which can become a problem inside configuration files. ie. if i configure a minimal log level i want to define it as a string rather than an integer. of course i could have some helper method that converts a string into an integer.

i think someone suggested adding a comparison method which accepts one of the strings, but i am really not sure. this again seems to me like something where going with strings its more tailored towards the ends of API users rather than implementors.

> 4. Log level class can have default Enum methods to get all values, or get value from name, etc.

can you elaborate?

regards,
Lukas Kahwe Smith
m...@pooteeweet.org

[Larry Garfield]

On 12/10/12 4:24 PM, Lukas Kahwe Smith wrote:
>
> On Dec 10, 2012, at 20:25 , Kirill Chebunin <i...@chebba.org> wrote:
>
>> I will add a huge portion of critic. But this is PSR. It should be close to perfect for users and vendors.
>>
>> 1. I still can't understand what is the difference between
>>
>> $logger->debug('Some info with %parameter%', ['%parameter%' => $parameter]);
>> and
>> $logger->debug(sprintf('Some info with %s', $parameter), ['parameter' => $parameter]);
>> This feature adds really small advantage, but will require an implementation in ALL libraries with the same simple code.
>
> as was explained the reason is so that its possible to translate the message on display rather than on write. to be able to do this the static part of the message needs to be logged separately from the parameters.

It's also not just language translation, although that's a major part of
it. Forcing the string to be "complete" at this stage violates "filter
on output", because the sanitization you need to do to the replacement
value may be different depending on if you're writing to Syslog, to SQL,
to a custom SQL setup that will get displayed using Drupal's display
tools, to a custom Mongo setup that will get displayed using ezPublish's
display tools, to an SMS message for admins... You really need the
output context to know what to do with the replacement value.

>> 3. Log levels do not have any order. It's better to use default int values instead of strings from linked rfc
>> 0 Emergency: system is unusable
>> 1 Alert: action must be taken immediately
>> 2 Critical: critical conditions
>> 3 Error: error conditions
>> 4 Warning: warning conditions
>> 5 Notice: normal but significant condition
>> 6 Informational: informational messages
>> 7 Debug: debug-level messages
>
> i am not sure about the full pro's and con's here, but i prefer strings as the numbers on their own are meaningless which can become a problem inside configuration files. ie. if i configure a minimal log level i want to define it as a string rather than an integer. of course i could have some helper method that converts a string into an integer.
>
> i think someone suggested adding a comparison method which accepts one of the strings, but i am really not sure. this again seems to me like something where going with strings its more tailored towards the ends of API users rather than implementors.

I can see both points here. Using ints allows some interesting
optimizations (like setting minimum debug levels, or storing in an int
field in the database), but strings are better for usability.

Using strings, how would we go about setting a minimum message
threshold? If we can answer that, then I think strings win. If not...
eh, I honestly don't have a strong enough opinion to fight about it
either way.

--Larry Garfield

[Tim Otten]

It would be good to document the rationale behind (a) variable substitution and (b) the names of the helper functions in some kind of document. Both are points that raised some discussion/controversy, and they're likely to get asked again in the future. It would make sense to document in either the PSR or an associated FAQ.

[Lukas Kahwe Smith]

On Dec 11, 2012, at 24:30 , Tim Otten <t...@onebitwise.com> wrote:

> It would be good to document the rationale behind (a) variable substitution and (b) the names of the helper functions in some kind of document. Both are points that raised some discussion/controversy, and they're likely to get asked again in the future. It would make sense to document in either the PSR or an associated FAQ.

+1

[Larry Garfield]

Some sort of "redux" of various PSRs would be very valuable, I agree.
It doesn't need to be part of the PSR, so we can write it in hindsight
after the PSR is approved, but we shouldn't forget to make room for it.

--Larry Garfield

[Paul Jones]

Not sure where this leaves us, then. Do most folks still feel that it's in the scope of a LoggerInterface to define what the placeholder delimiters ought to be, or have some opinions been modified to hold that placeholder delimiters are *not* in scope, and best left to implementors? (I used to be of the former opinion, but now I am of the latter.)


-- pmj

[Matthew Lanigan]

As the purpose of PSRs (including this one) is ostensibly to promote interoperability, I have to agree that defining placeholder delimiters should be considered beyond the scope of this document. In my opinion, doing does does not simply fail to promote interoperability, it actively inhibits it.

Matthew Lanigan

 

-- pmj

[Tim Otten]

On Dec 10, 2012, at 7:38 PM, Matthew Lanigan wrote:

As the purpose of PSRs (including this one) is ostensibly to promote interoperability, I have to agree that defining placeholder delimiters should be considered beyond the scope of this document. In my opinion, doing does does not simply fail to promote interoperability, it actively inhibits it.

Excuse me for being a dunderhead, but is that a typo or are we on different planets? Is that saying:

(a) The specification of placeholder-handling MUST be within scope to promote functional interoperability. OMITTING placeholders from the specification will actively inhibit functional interoperability.

(b) The specification of placeholder-handling MUST NOT be within scope to promote functional interoperability. INCLUDING placeholders in the specification will actively inhibit functional interoperability.

I agree with "a" and disagree with "b" for the reasons that have been previously discussed on this list.

[Matthew Lanigan]

On Mon, Dec 10, 2012 at 8:10 PM, Tim Otten <t...@onebitwise.com> wrote:

On Dec 10, 2012, at 7:38 PM, Matthew Lanigan wrote:

As the purpose of PSRs (including this one) is ostensibly to promote interoperability, I have to agree that defining placeholder delimiters should be considered beyond the scope of this document. In my opinion, doing does does not simply fail to promote interoperability, it actively inhibits it.

Excuse me for being a dunderhead, but is that a typo or are we on different planets? Is that saying:

(a) The specification of placeholder-handling MUST be within scope to promote functional interoperability. OMITTING placeholders from the specification will actively inhibit functional interoperability.

(b) The specification of placeholder-handling MUST NOT be within scope to promote functional interoperability. INCLUDING placeholders in the specification will actively inhibit functional interoperability.

I agree with "a" and disagree with "b" for the reasons that have been previously discussed on this list.

We're not talking about placeholders, but rather placeholder delimiters (e.g. wrapping placeholders in "%" characters). Defining delimiters is beyond scope and inhibits functional interoperability -- unless compliant frameworks are required to use only the delimiters defined in the PSR, which I think is an unrealistic requirement.

Matthew Lanigan

[Tim Otten]

https://groups.google.com/d/msg/php-fig/yL_wDyiT6GY/9oSHoolgWJMJ

[Tim Otten]

My previous message was perhaps curt. Please read the link for some background on how placeholders affect functional interoperability:

https://groups.google.com/d/msg/php-fig/yL_wDyiT6GY/9oSHoolgWJMJ

While I generally stand by that, there are some addenda:

(1) The PSR LoggerInterface is not and cannot be a drop-in replacement for any existing log interface. To be compliant, an application or framework only needs to provide *some* implementation of the interface that can be used when integrating third-party libraries -- the application or framework does not need to replace all their existing log statements with new PSR-based log-statements. Application/framework developers have considerable latitude to maintain (or abandon) their proprietary log interfaces. The PSR primarily affects library-developers who want to write portable code. Thus, as Jordi pointed out earlier, conflicts with existing notations DO NOT present an issue of functional-interoperability.

(2) A couple people (I don't remember who -- sorry) raised the idea of including the "delimiters" in the $context keys. This is a promising idea: (a) it allows different people to use different delimiters, and (b) it doesn't require extra configuration for every single library, and (c) it works with translation/escaping/etc, and (d) it doesn't require juggling multiple instances of LoggerInterface. Example:

$log->info("Unhandled exception while loading user %user%", array(

 "%user%" => $username,

));

Jordi pointed out that this has some complication, but maybe that complication is more acceptable than the bikeshed arguments about delimiters. Specifically, the complication is that we allow $context to provide extra data which is not intended for interpolation, and there could be accidental interactions. For example, the draft PSR defines a special key "exception". If someone used that key as follows:

$log->info("Unhandled exception while loading user %user%", array(

 "%user%" => $username,

 "exception" => $e, // an instance of class Exception

));

then the second word of the message would be accidentally substituted. This can be mitigated by changing the 'exception' key to something unlikely to cause collisions (like "[[exception]]"):

$log->info("Unhandled exception while loading user %user%", array(

 "%user%" => $username,

 "[[exception]]" => $e

));

A minor side-benefit of this approach is that it becomes trivially easy for log-implementers to perform string interpolation -- just call strtr()!

On Dec 10, 2012, at 8:14 PM, Matthew Lanigan wrote:

[Ivan Enderlin]

Hi all,

On 10/12/12 20:25, Kirill Chebunin wrote:

I will add a huge portion of critic. But this is PSR. It should be close to perfect for users and vendors.

1. I still can't understand what is the difference between

$logger->debug('Some info with %parameter%', ['%parameter%' => $parameter]);
and
$logger->debug(sprintf('Some info with %s', $parameter), ['parameter' => $parameter]);
This feature adds really small advantage, but will require an implementation in ALL libraries with the same simple code.

And why not

    $logger->debug('Some info with %s.', [$parameter], …)

The printf is done inside the debug() method.

2. We have methods like error, warn, etc in the interface, so ALL libraries will have to implement this methods, but this methods will have the same code in 99.9% of situations
public function error($message, array $context = [])
{
    $this->log(LogLevel::ERROR, $message, $context);
}
More over a client code will expect such logic.

So It's better to have an interface with 1 log($level, $message, array $context = array()) method and have a wrapper with placeholders, level shortcut methods etc. Than each library will implement only 1 main method.

+1.
And why not reordering arguments, such as:

    Logger::log($message, $level = LogLevel::DEBUG, $context = []);

The default level value would depend of the implementation (most of the time, it is an error or a debug).
Thus, we can quickly write:

    $logger->log('Foobar');

3. Log levels do not have any order. It's better to use default int values instead of strings from linked rfc
              0       Emergency: system is unusable
              1       Alert: action must be taken immediately
              2       Critical: critical conditions
              3       Error: error conditions
              4       Warning: warning conditions
              5       Notice: normal but significant condition
              6       Informational: informational messages
              7       Debug: debug-level messages

Levels (which are known as “priorities” as defined in “The BSD Syslog Protocol”, please see RFC3164, section 4.1.1 “PRI Part”, “ Table 2. syslog Message Severities”) sometimes include another one: “8 Test: test messages”. This is not in the RFC but some implementations do that. Also, it could be useful for a lot of tools.

4. Log level class can have default Enum methods to get all values, or get value from name, etc.

Thoughts?

Cheers.

-- 
Ivan Enderlin
Developer of Hoa
http://hoa.42/ or http://hoa-project.net/

PhD. student at DISC/Femto-ST (Vesontio) and INRIA (Cassis)
http://disc.univ-fcomte.fr/ and http://www.inria.fr/

Member of HTML and WebApps Working Group of W3C
http://w3.org/

[Jordi Boggiano]

>> 2. We have methods like error, warn, etc in the interface, so ALL
>> libraries will have to implement this methods, but this methods will
>> have the same code in 99.9% of situations
>> public function error($message, array $context = [])
>> {
>> $this->log(LogLevel::ERROR, $message, $context);
>> }
>> More over a client code will expect such logic.
>>
>> So It's better to have an interface with 1 log($level, $message, array
>> $context = array()) method and have a wrapper with placeholders, level
>> shortcut methods etc. Than each library will implement only 1 main method.
> +1.
> And why not reordering arguments, such as:
>
> Logger::log($message, $level = LogLevel::DEBUG, $context = []);
>
> The default level value would depend of the implementation (most of the
> time, it is an error or a debug).
> Thus, we can quickly write:
>
> $logger->log('Foobar');

And instead logging to a random level you could also quickly write:

$logger->debug('Foobar');

That way you are explicit about what your log record is. Having a
default level makes no sense at all that I can see. The whole point of
levels is to be able to filter. If you want to rely on a default it
probably means you want to configure your logger to accept *all*
messages from DEBUG and up.

[Jordi Boggiano]

Heya,

I think that sums up the current state of the discussion on placeholders
and their delimiters.

I still am of the opinion that the current state is the best approach
because it's very clearly explained and can not lead to conflicts as
demonstrated by the exception example above. The *only* downside is that
people *might* (unless they provide their own adapter) have to get used
to a placeholder they don't currently use, which is really the most
trivial of all things I would say. Again: this does *not* prevent you in
your framework from using your own placeholders given that you either
wrap the implementation or use a custom implementation that supports
them. It means that for standalone libraries using the %foo%
placeholders is the recommended way.

I don't know if people are disagreeing with that because it is the only
thing they have to say and I asked for opinions or if it is really a big
issue, but I would like to clear that out so we can move forward with a
vote.

[Jason Judge]

On Monday, 10 December 2012 01:25:17 UTC, pmjones wrote:

I don't mean to be the party pooper here, but I'm really not a fan of the %delimiters%.  Is there any way at all we can add a setDelimiters($left, $right) method to the interface?  Not a deal breaker if not, but *man* it would be nice to let projects use their own delmiters.  Lithium and others have {foo}, Aura has {:foo}, Drupal has %foo%, I'm sure someone out there has {{foo}}.  It's the kind of thing that can be set once before the Logger implementation is injected into whatever needs it.

I think if setting the delimiters is an option, then there should also be a rule for matching whatever is inside the delimiters. Also rules for escaping the delimiters for when you want to use them verbatim. Just defining delimiters is only half the story IMO.

-- Jason

[Jason Judge]

On Tuesday, 11 December 2012 08:11:02 UTC, Jordi Boggiano wrote:

And instead logging to a random level you could also quickly write:

    $logger->debug('Foobar');

That way you are explicit about what your log record is. Having a
default level makes no sense at all that I can see. The whole point of
levels is to be able to filter. If you want to rely on a default it
probably means you want to configure your logger to accept *all*
messages from DEBUG and up.

This is what SugarCRM does, and it is quite handy. Though it doesn't support variable placeholders, which is certainly not so handy.

The debug level strings all have numeric values:

        'debug'      => 100,
        'info'       => 70,
        'warn'       => 50,
        'deprecated' => 40,
        'error'      => 25,
        'fatal'      => 10,
        'security'   => 5,
        'off'        => 0,

They are pretty arbitrarily distributed over 0 to 100, but it does allow custom levels to be added in between if custom modules find that useful. The logger has a catch-all method (__call) which maps $log->foo(...) onto a logging level.

I would never describe SugarCRM as a great example of how an application or framework should be coded, but I just thought I would toss this in as an example.

-- Jason

[FGM at GMail]

Actually, I would say that, while placeholders are very necessary,
having them include delimiters is not such a necessity, especially if
the substitutable parameters were in a subkey instead of first-level
objects.

But even if they are first-level objects, the logger itself probably
doesn't care about them, stuffing them in the context as it sees fit,
and the log display, which is the one caring about them, does not
typically need placeholders to be defined by a standard delimiter: a
logger user (say, Drupal) could very well define its '@foo', '%bar',
'!baz' placeholder formats, and the log display user /for the same
tool/ could then interpret them as intended by the logger user, the
logger itself being only a transport able to carry the event with its
placeholders and the values to be bound to them.

2012/12/11 Jason Judge <jason...@consil.co.uk>:

> --
> You received this message because you are subscribed to the Google Groups
> "PHP Framework Interoperability Group" group.
> To post to this group, send email to php...@googlegroups.com.
> To unsubscribe from this group, send email to
> php-fig+u...@googlegroups.com.

> To view this discussion on the web visit
> https://groups.google.com/d/msg/php-fig/-/-ArhtKCh4R0J.

[Kirill Chebunin]

2012/12/11 Lukas Kahwe Smith <m...@pooteeweet.org>

On Dec 10, 2012, at 20:25 , Kirill Chebunin <i...@chebba.org> wrote:

> I will add a huge portion of critic. But this is PSR. It should be close to perfect for users and vendors.
>
> 1. I still can't understand what is the difference between
>
> $logger->debug('Some info with %parameter%', ['%parameter%' => $parameter]);
> and
> $logger->debug(sprintf('Some info with %s', $parameter), ['parameter' => $parameter]);
> This feature adds really small advantage, but will require an implementation in ALL libraries with the same simple code.

as was explained the reason is so that its possible to translate the message on display rather than on write. to be able to do this the static part of the message needs to be logged separately from the parameters.

Yep, already got this point. Sorry for misunderstanding. It looks reasonable. Only the one moment: may be some escape sequence are required to prevent replace parameter replace if you have a text with %. ex. double percent sign $this->info('Use parameter placeholder %%parameter%%') or backslash ('Use parameter placeholder \%parameter\%')
 

> 2. We have methods like error, warn, etc in the interface, so ALL libraries will have to implement this methods, but this methods will have the same code in 99.9% of situations
> public function error($message, array $context = [])
> {
>     $this->log(LogLevel::ERROR, $message, $context);
> }
> More over a client code will expect such logic.
>
> So It's better to have an interface with 1 log($level, $message, array $context = array()) method and have a wrapper with placeholders, level shortcut methods etc. Than each library will implement only 1 main method.

this was also discussed before its simply more convenient for users to have separate methods and given the little effort it requires implementors to deal with this, it makes sense to tailor things to users of the API.

It can be done with separating log writing and common log functions.

/**
 * Main writer interface with one required method
 */
interface LoggerAdapterInterface
{
    public function log($level, $message, array $context = array());
}

/**
 * Wrapper with common function to reduce code duplication
 */
class Logger
{
    private $adapter;

    public function __construct(LoggerAdapterInterface $adapter)
    {
        $this->adapter = $adapter;
    }

    public function getAdapter()
    {
        return $this->adapter;
    }

    /**
     * This method can have another parameter order, ex. ($message, $level = LogLevel::DEBUG, array $context = array())
     */
    public function log($level, $message, array $context = array());
    {
        $this->adapter->log($level, $message, $context);
    }

    public function alert($message, array $context = array());
    {
        $this->log(Loglevel::ALERT, $message, $context);
    }

    /* ... other shortcut methods */
}

/**
 * Null logeer code is reduced and the same effect will be with all implementations
 */
class NullLoggerAdapter implements LoggerAdapterInterface
{
    public function log($level, $message, array $context = array());
    {
        // do nothing
    }
}
 

> 3. Log levels do not have any order. It's better to use default int values instead of strings from linked rfc
>               0       Emergency: system is unusable
>               1       Alert: action must be taken immediately
>               2       Critical: critical conditions
>               3       Error: error conditions
>               4       Warning: warning conditions
>               5       Notice: normal but significant condition
>               6       Informational: informational messages
>               7       Debug: debug-level messages

i am not sure about the full pro's and con's here, but i prefer strings as the numbers on their own are meaningless which can become a problem inside configuration files. ie. if i configure a minimal log level i want to define it as a string rather than an integer. of course i could have some helper method that converts a string into an integer.

i think someone suggested adding a comparison method which accepts one of the strings, but i am really not sure. this again seems to me like something where going with strings its more tailored towards the ends of API users rather than implementors.

I think it's better to have order in "core" with integers and names for configuration in support methods. Also it's closer to rfc5424
 

> 4. Log level class can have default Enum methods to get all values, or get value from name, etc.

can you elaborate?

Somethin like this

class LogLevel
{
    const EMERGENCY = 0;
    const ALERT     = 1;
    const CRITICAL  = 2;
    const ERROR     = 3;
    const WARNING   = 4;
    const NOTICE    = 5;
    const INFO      = 6;
    const DEBUG     = 7;

    public static function getLevels()
    {
        return array(
            'emergency' => self::EMERGENCY,
            'alert'     => self::ALERT,
            'critical'  => self::CRITICAL,
            'error'     => self::ERROR,
            'warning'   => self::WARNING,
            'notice'    => self::NOTICE,
            'info'      => self::INFO,
            'debug'     => self::DEBUG
        );
    }

    public static function valueOf($name)
    {
        $name = strtolower($name);
        $levels = self::getLevels();
        if (!isset($levels[$name])) {
            throw new \UnexpectedValueException(sprintf('Unknown log level name "%s"', $name));
        }

        return $levels[$name];
    }

    public static function nameOf($value)
    {
        $levels = array_flip(self::getLevels());
        if (!isset($levels[$value])) {
            throw new \UnexpectedValueException(sprintf('Unknown log level with value "%s"', $value));
        }

        return $levels[$value];
    }
}

Util methods helps getting all values and converting names to values and vice versa (Similar to Java Enum methods).
If using strings we can remove convert methods, but will have to add "compare" function for level ordering.


One more thing. There is a note about throwing Psr\Log\InvalidArgumentException.
* First of all it's better to use more descriptive name (and don't use default short name of InvalidArgumentException). ex UnexpectedLevelException/WrongLevelException/InvalidLevelException.
* Do we really need custom level support? If no, there is no need to define an exception for such case, because it's logic error (wrong method call) and client must check what he provides in the level parameter. If we want this support, than this exception should be runtime and we need to have some more functions to work with custom levels.

 

regards,
Lukas Kahwe Smith
m...@pooteeweet.org

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To post to this group, send email to php...@googlegroups.com.
To unsubscribe from this group, send email to php-fig+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Jason Judge]

Isn't the main point about being consistent with the placeholders to allow one framework to use packages from any number of different sources, and know that they will all log messages using the same placeholder syntax? There may only be one rendering point in your application that puts the error message in front of your user, or into a log file, but the messages could have been generated by any number of third-party packages, all of which need to be using the same placeholder syntax to be understood by the rendering stage.

So although the display level could use any format for placeholders that the developer desires, if various packages in the application cannot agree on which format *they* are using, then how would the two ends get matched up? I'm almost beginning to think that each package that does any logging needs to declare what syntax it is going to use, and that has to propagate up the chain with the message. That seems a but convoluted though.

[Jason Judge]

Of course, by head is stuck in practical programming land, so apologies if this is way off the mark for setting standards, which I realise try not to get too involved in the implementation.

[Johannes Schmitt]

Since the numeric argument has been made a couple of times now, and it indeed has some advantages; I've added something to the log-level class to allow for this: https://github.com/Seldaek/log/pull/1

Let me know what you think.

--

You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To post to this group, send email to php...@googlegroups.com.
To unsubscribe from this group, send email to php-fig+u...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msg/php-fig/-/XZs-yP8QQO8J.

[Jordi Boggiano]

Heya,

> Yep, already got this point. Sorry for misunderstanding. It looks
> reasonable. Only the one moment: may be some escape sequence are
> required to prevent replace parameter replace if you have a text with %.
> ex. double percent sign $this->info('Use parameter placeholder
> %%parameter%%') or backslash ('Use parameter placeholder \%parameter\%')

Having % is maybe not so rare, but having %foo% in a legit string is
almost unheard of. Also %foo% if you have no "foo" in the context is
left alone as %foo%, so the chance for conflict is so low that I don't
think adding the complexity of escaping is worth it.

> It can be done with separating log writing and common log functions.
>
> /**
> * Main writer interface with one required method
> */
> interface LoggerAdapterInterface
> {
> public function log($level, $message, array $context = array());
> }
>
> /**
> * Wrapper with common function to reduce code duplication
> */
> class Logger
> {
> private $adapter;
>
> public function __construct(LoggerAdapterInterface $adapter)
> {
> $this->adapter = $adapter;
> }
>
> public function getAdapter()
> {
> return $this->adapter;
> }
>
> /**
> * This method can have another parameter order, ex. ($message,
> $level = LogLevel::DEBUG, array $context = array())
> */
> public function log($level, $message, array $context = array());
> {
> $this->adapter->log($level, $message, $context);
> }
>
> public function alert($message, array $context = array());
> {
> $this->log(Loglevel::ALERT, $message, $context);
> }
>
> /* ... other shortcut methods */
> }

This and your proposal for int log level actually leads to more
complexity in the Logger implementations. As an example here is the code
I had to change in monolog to make it compatible with the PSR:

https://github.com/Seldaek/monolog/compare/psr

As you can see, the log() function can very simply take the name of the
level and get the equivalent monolog constant (i.e. ERROR is 400 to
monolog). If I got 3 in log(). I would have to call the PSR thing to
convert 3 to ERROR. And then assuming monolog already had a log method
(which is not so unlikely in other libs), and it would use 3 as a
constant for the value of NOTICE. Then you call log(3, 'foo') and the
library *can not know* what the intent is. It means you may have to
break BC to add support for the PSR, which will slow down adoption.

Because of this I feel that the current way is superior.

> One more thing. There is a note about throwing
> Psr\Log\InvalidArgumentException.
> * First of all it's better to use more descriptive name (and don't use
> default short name of InvalidArgumentException). ex
> UnexpectedLevelException/WrongLevelException/InvalidLevelException.
> * Do we really need custom level support? If no, there is no need to
> define an exception for such case, because it's logic error (wrong
> method call) and client must check what he provides in the level
> parameter. If we want this support, than this exception should be
> runtime and we need to have some more functions to work with custom levels.

Some want custom level support, and it doesn't cost us much to leave the
door to it by specifying the exception. I agree that
InvalidLevelException makes more sense though. I will update.

[Jordi Boggiano]

On 11.12.2012 11:06, Johannes Schmitt wrote:
> Since the numeric argument has been made a couple of times now, and it
> indeed has some advantages; I've added something to the log-level class
> to allow for this: https://github.com/Seldaek/log/pull/1
>
> Let me know what you think.

See my previous mail for a reason why I think it's a bad idea. Then your
PR also adds custom log level capability in the PSR itself, which is
something completely different and I don't think that's good to have in
the "interface". Just as a concrete example from an implementor's point
of view. Monolog needs to know about log levels and their names to build
up a log record, so in this case if you add a custom level it would blow
up when you start using it unless we add a way to reverse the level to a
name in the interface as well. But that's something specific to monolog,
so IMO it would be better to just let people that want custom levels
extend the implementation they use. It would mean they can't use any
logger implementation interchangeably anymore, but at least they can
still give their logger instance to a PSR-supporting lib.

[Drak]

I think I am able to  agree with Jordi's view now,  after considering the thread.

It's clear and simple.

Drak

[Johannes Schmitt]

I'm not proposing to allow to pass integers to the log() method, but simply to associate a severity with a log-level. The log() method would still only take the string representation.

As I don't propose to allow integers to be passed to the log() method, the ``register`` method will have no effect on the implementation of it. Ofc, passing any string will bind your usage against a specific implementation and will not work with all implementors anymore, but that is not a problem created by the ``register`` method as that is really only about comparing severities of log-levels. As an alternative, we could change the ``compareSeverities`` method to also accept integers as levels so that for custom levels you would pass the severity you associate with them, but do not need to register them first.

You could argue that this is too implementation specific, but severities are very common for loggers, we even have them implicitly in the PSR, so we can as well provide some tools for implementors to compare them. Again, I do not propose a change to the LoggerInterface itself it still only takes strings for levels.

Cheers,
Johannes

[Kirill Chebunin]

Full Monolog implemenation for my proposal (without placeholders, it will be the same processor)

class Logger implements LoggerAdapterInterface
{
    // All old methods and properties

    /**
     * {@inheritDoc}

     */
    public function log($level, $message, array $context = array())

    {
        try {
            $levelName = LogLevel::nameOf($level);
        } catch (\UnexpectedValueException $e) {
            throw new InvalidLevelException($level, $e);
        }

        if (!defined(__CLASS__.'::'.strtoupper($levelName))) {
            throw new InvalidLevelException($level);
        }

        // no return because interface method is void
        $this->addRecord($level, $message, $context);
    }
}

The implementation of PSR will always be as:
* Map Psr level with internal levels (You always need this mapping no matter ints or strings are used. But it's always a bad idea to use direct values of constants in any code)
* Implement log methods (in my proposal you need to implement only 1 significant method)
* Implement placeholder support

2012/12/11 Jordi Boggiano <j.bog...@seld.be>

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To post to this group, send email to php...@googlegroups.com.
To unsubscribe from this group, send email to php-fig+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Jordi Boggiano]

On 11.12.2012 12:07, Kirill Chebunin wrote:
> Full Monolog implemenation for my proposal (without placeholders, it
> will be the same processor)
>
> class Logger implements LoggerAdapterInterface
> {
> // All old methods and properties
>
> /**
> * {@inheritDoc}
> */
> public function log($level, $message, array $context = array())
> {
> try {
> $levelName = LogLevel::nameOf($level);
> } catch (\UnexpectedValueException $e) {
> throw new InvalidLevelException($level, $e);
> }
>
> if (!defined(__CLASS__.'::'.strtoupper($levelName))) {
> throw new InvalidLevelException($level);
> }
>
> // no return because interface method is void
> $this->addRecord($level, $message, $context);
> }
> }

This ignores the fact that $level might conflict/clash between current
usage of monolog and the PSR levels. That's what I tried to explain
before. It makes writing BC implementations of the spec harder to have
numbers as levels.

[Jordi Boggiano]

On 11.12.2012 11:49, Johannes Schmitt wrote:
> I'm not proposing to allow to pass integers to the log() method, but
> simply to associate a severity with a log-level. The log() method would
> still only take the string representation.
>
> As I don't propose to allow integers to be passed to the log() method,
> the ``register`` method will have no effect on the implementation of it.
> Ofc, passing any string will bind your usage against a specific
> implementation and will not work with all implementors anymore, but that
> is not a problem created by the ``register`` method as that is really
> only about comparing severities of log-levels. As an alternative, we
> could change the ``compareSeverities`` method to also accept integers as
> levels so that for custom levels you would pass the severity you
> associate with them, but do not need to register them first.
>
> You could argue that this is too implementation specific, but severities
> are very common for loggers, we even have them implicitly in the PSR, so
> we can as well provide some tools for implementors to compare them.
> Again, I do not propose a change to the LoggerInterface itself it still
> only takes strings for levels.

Severities are common, but *custom* ones should not be. By adding this
to the psr/log code, we legitimize the use of custom severities which is
IMO a bad thing. And by having it static we run the risk of having two
libraries adding the same custom level with different numerical
severities. Not good.

Overall I just feel this adds complexity and chance for conflict for no
good reason. Custom log levels might be added in specific cases for
application stuff, but it's very seldom and application specific code
should not be taken into consideration here. This can be handled by
implementators just fine.

If there is a broad support for compareSeverities($levelA, $levelB) - I
am not really against that method even though it's kinda trivial and
probably not needed by implementors, but the rest is harmful IMO.

[Kirill Chebunin]

Do you mean that current Monolog log method uses another level values? In common scenario it always true for all log libaries (that's why i said we ALWAYS need to map PSR level to library levels) and if you need to save BC the best way to write separate Adapter.

class PsrAdapter implements LoggerAdapterInterface
{
    private $logger;

    public function __construct(Logger $logger)
    {
        $this->logger = $logger;

    }

    /**
     * {@inheritDoc}
     */
    public function log($level, $message, array $context = array())
    {
        try {
            $levelName = LogLevel::nameOf($level);
        } catch (\UnexpectedValueException $e) {
            throw new InvalidLevelException($level, $e);
        }

        if (!defined(__NAMESPACE__ . '\Logger::'.strtoupper($levelName))) {

            throw new InvalidLevelException($level);
        }

        // no return because interface method is void

        $this->logger->addRecord($level, $message, $context);
    }
}

2012/12/11 Jordi Boggiano <j.bog...@seld.be>

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To post to this group, send email to php...@googlegroups.com.
To unsubscribe from this group, send email to php-fig+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Johannes Schmitt]

I've updated the proposal to remove the notion of custom levels, so now it's just some usability tool to compare the severity of the provided levels; shouldn't hurt.

As a side-note, I noticed that you placed @inheritDoc annotations in the NullLogger. I think these should be removed for now as the semantics of @inheritDoc are only vaguely defined:
- Does it copy the parent doc comment literally?
- Does it only copy the description?
- What happens if parameter names of child and parent classes are different? Does it magically resolve this?
- etc.

The easiest for now, is to just copy the parent comment until we have the doc comment PSR accepted.

Cheers,
Johannes

[Chuck Burgess]

From phpDocumentor's perspective, most of the parent docblock info will be inherited without needing {@inheritdoc}. Its original purpose was solely for inheriting the parent's long description when you write an entire child docblock rather than inherit the parent's... it is a placeholder for where that long description gets inserted.

[Jordi Boggiano]

On 11.12.2012 14:01, Johannes Schmitt wrote:
> I've updated the proposal to remove the notion of custom levels, so now
> it's just some usability tool to compare the severity of the provided
> levels; shouldn't hurt.

Thinking about it some more, I still don't really see the use case. It
does not benefit users as far as I can see, and implementors very likely
will already have some existing facilities for level sorting and the
easiest there is to just create a map: PSR level => internal level.

I don't really like cluttering the psr package with stuff that won't
benefit anyone or will *maybe* save 3 lines of code for implementors
given that there will be few implementations but many users looking at
it that will have more to grasp and might just be confused by some
things (like the fact that levels point to severities even though those
are only internal).

In short, I would like if things we add could be backed by use cases
instead of adding stuff on principle. To me it looks like it's feature
complete.

> As a side-note, I noticed that you placed @inheritDoc annotations in the
> NullLogger. I think these should be removed for now as the semantics of
> @inheritDoc are only vaguely defined:
> - Does it copy the parent doc comment literally?
> - Does it only copy the description?
> - What happens if parameter names of child and parent classes are
> different? Does it magically resolve this?
> - etc.
>
> The easiest for now, is to just copy the parent comment until we have
> the doc comment PSR accepted.

Fixed that by just copying the docblocks. Since it should never change
anyway the duplication doesn't do much harm.

[Drak]

On 11 December 2012 13:34, Jordi Boggiano <j.bog...@seld.be> wrote:

On 11.12.2012 14:01, Johannes Schmitt wrote:
> I've updated the proposal to remove the notion of custom levels, so now
> it's just some usability tool to compare the severity of the provided
> levels; shouldn't hurt.

Thinking about it some more, I still don't really see the use case. It
does not benefit users as far as I can see, and implementors very likely
will already have some existing facilities for level sorting and the
easiest there is to just create a map: PSR level => internal level.

I was just about to say the same.

Regards,

Drak

[Paul Jones]

On Dec 11, 2012, at 2:24 AM, Jordi Boggiano wrote:

[regarding delimiters in placeholders]

> I don't know if people are disagreeing with that because it is the only
> thing they have to say and I asked for opinions or if it is really a big
> issue, but I would like to clear that out so we can move forward with a
> vote.

It wasn't a big issue to me before, but it has become one. If projects want to do placeholder substitution, they should write an adapter or wrapper for it. If one project has to wrap, they should all have to wrap.


-- pmj

[Jordi Boggiano]

>> I don't know if people are disagreeing with that because it is the only
>> thing they have to say and I asked for opinions or if it is really a big
>> issue, but I would like to clear that out so we can move forward with a
>> vote.
>
> It wasn't a big issue to me before, but it has become one. If projects want to do placeholder substitution, they should write an adapter or wrapper for it. If one project has to wrap, they should all have to wrap.

That sounds a bit like "If it's not our delimiter then it can't be any
delimiter!", which is not cool at all. I hope I misinterpreted.

At the risk of repeating myself however, I do not think we should care
about frameworks/projects. Virtually all of them will have to create an
adapter/wrapper for one reason or another.

What I do think we should care about, is libraries that at the moment
have *no delimiters* because they have *no logging whatsoever*. This PSR
allows them to log, and should allow them the use of placeholders in a
unified way.

[Evert Pot]

On Dec 11, 2012, at 4:22 PM, Jordi Boggiano <j.bog...@seld.be> wrote:

>>> I don't know if people are disagreeing with that because it is the only
>>> thing they have to say and I asked for opinions or if it is really a big
>>> issue, but I would like to clear that out so we can move forward with a
>>> vote.
>>
>> It wasn't a big issue to me before, but it has become one. If projects want to do placeholder substitution, they should write an adapter or wrapper for it. If one project has to wrap, they should all have to wrap.
>
> That sounds a bit like "If it's not our delimiter then it can't be any
> delimiter!", which is not cool at all. I hope I misinterpreted.

Sounds the same to me.

pmj: could you clarify why it's an issue for you to wrap the logger so you can use your own delimiters?

It seems like a proxy object would be very simple to create, and to me this would even seem like the superior pattern regardless.

Evert

[Chuck Burgess]

I took that to mean that if we have to choose one way to do this in order to incorporate it into the PSR package, and then see most usage-in-the-wild have to adapt around it, then we should just leave it out of the PSR package and let the wild handle its adapting as needed, rather than the package making a spec at all.

CRB
about.me/ashnazg

Evert

[Paul Jones]

On Dec 11, 2012, at 9:37 AM, Chuck Burgess wrote:

> I took that to mean that if we have to choose one way to do this in order to incorporate it into the PSR package, and then see most usage-in-the-wild have to adapt around it, then we should just leave it out of the PSR package and let the wild handle its adapting as needed, rather than the package making a spec at all.

Exactly my point. Thanks for helping to clarify.


-- pmj

[Chuck Burgess]

In rereading my reply, I think my point got lost when I could word my last sentence to my satisfaction and then left it off...

To me, the point becomes that if handling delimiters is a special case, it's not necessarily a strong enough case to become part of the package spec... by being package spec, it becomes a requirement for implementers.  I'm not convinced that all implementers should have to build something to accommodate this special case, when it is feasible to leave it out of the spec and allow implementers to adapt themselves as needed to meet what does appear in the spec.

CRB
about.me/ashnazg

[Lukas Kahwe Smith]

I think that would be quite shortsighted. We have come to an agreement that it would be good to offer placeholders so lets add it to the PSR. Now picking the delimiter despite there supposedly not being a clear choice based on the member projects will indeed mean that potentially one member will by chance get the delimiter they are already using, while a bunch of others will need to add an adapter. Not ideal, but so it goes. There will be a new major version some day for each of these other members to align themselves with the PSR.

But if we say that in case there is no existing majority for stuff like this, we effectively say that the given feature can never be covered by a PSR, that would be a sad affair.

Now picking the delimiter can for all I care be done in a pre-vote or whatever, but lets not worry about if someone gets an easier life than others. Lets think bigger than that.

[Paul Jones]

On Dec 11, 2012, at 10:02 AM, Chuck Burgess wrote:

> In rereading my reply, I think my point got lost when I could word my last sentence to my satisfaction and then left it off...
>
> To me, the point becomes that if handling delimiters is a special case, it's not necessarily a strong enough case to become part of the package spec... by being package spec, it becomes a requirement for implementers. I'm not convinced that all implementers should have to build something to accommodate this special case, when it is feasible to leave it out of the spec and allow implementers to adapt themselves as needed to meet what does appear in the spec.

Indeed. As I've said for some time now, it is my opinion that PSRs should reflect what is in common use by the member projects as they exist. Where those projects differ, we need to do some research to find out what the differences are and where the areas of commonality exist. If there are no common uses, we should not try to write standards from scratch.

With that in mind:

The method signatures presented by LoggerInterface look to me like they are pretty commonly adopted by a wide range of member projects, although I have not done the research. I'm sure someone would have spoken up by now if they were widely at variance.

But the delimiter characters do *not* look like they are commonly adopted. So we need to either (1) do some research, as with previous surveys, to find out what is commonly used, or (2) leave it out entirely.


-- pmj

[Lukas Kahwe Smith]

On Dec 11, 2012, at 17:02 , Chuck Burgess <demon...@gmail.com> wrote:

> In rereading my reply, I think my point got lost when I could word my last sentence to my satisfaction and then left it off...
>
> To me, the point becomes that if handling delimiters is a special case, it's not necessarily a strong enough case to become part of the package spec... by being package spec, it becomes a requirement for implementers. I'm not convinced that all implementers should have to build something to accommodate this special case, when it is feasible to leave it out of the spec and allow implementers to adapt themselves as needed to meet what does appear in the spec.

i think there have been plenty of people that have said that while the feature isnt a must must have, its important enough to add it. now if suddenly it no longer is that important, then we can simply move this to a separate interface that extends the base one.

[Paul Jones]

On Dec 11, 2012, at 10:09 AM, Lukas Kahwe Smith wrote:

> But if we say that in case there is no existing majority for stuff like this, we effectively say that the given feature can never be covered by a PSR, that would be a sad affair.

I disagree. In my opinion, the mission of the group is to find areas of commonality among member projects, and codify those areas of commonality. Where common practices do not exist, I do *not* think it is our mission to create standards out of whole cloth.


-- pmj

[Paul Jones]

Or we can leave it out entirely, and start a new PSR proposal centered around placeholder delimiters, one that does a proper survey of member projects.


-- pmj

[Lukas Kahwe Smith]

a survey shouldnt take too long to do (we have already had several projects mentioning their delimiters which started this whole mess). but sure we can also move this to a separate PSR but i really dont get the sudden shift here.

[Chuck Burgess]

I can't remember... did the idea already come up for making the log message itself into a first class object, whose methods would include such issues as handling placeholders and delimiters?  I haven't walked this idea through its paces in my head yet, to gauge its feasibility...

CRB
about.me/ashnazg

[Evert Pot]

I would be happy with just going with the most popular delimiter format, if it exists.

I don't think there's a lot of technical reasons for going with for example "{{ }}" instead of "% %", but making delimiters flexible makes the interface harder to use for literally every implementor.

So just checking this again, because this was skipped over in the last reply.

pmj: if you hypothetically had to provide a wrapper that converts one delimiter-format to another, what would be your main issue with that? And: why do you think that that would be a worse situation than making delimiters configurable?

Evert

[Evert Pot]

On Dec 11, 2012, at 5:22 PM, Chuck Burgess <demon...@gmail.com> wrote:

> I can't remember... did the idea already come up for making the log message itself into a first class object, whose methods would include such issues as handling placeholders and delimiters? I haven't walked this idea through its paces in my head yet, to gauge its feasibility...

Yea, I still hold the opinion that the $message should be a string, or something implementing MessageInterface.
IMHO: Any advanced features could be added to the interface.

But, that wasn't a very popular opinion.

Evert

[Paul Jones]

On Dec 11, 2012, at 10:22 AM, Evert Pot wrote:

> pmj: if you hypothetically had to provide a wrapper that converts one delimiter-format to another, what would be your main issue with that? And: why do you think that that would be a worse situation than making delimiters configurable?

My main issue isn't about the wrappers, per se. It's that we're picking a delimiter "because we need something to standardize on" but not finding out what's actually in most common use. If there is a set of delimiters that are clearly more commonly used than others, then we should use that one. If there's no common agreement, then we need to look at what's being used and pick from those.

As it stands, I don't think placeholder delimiters "MUST" ;-) be part of the LoggerInterface proposal. I think they may be, and maybe even should be, but only after we do the research. In fact, in the interest of getting LoggerInterface out the door, I think it's perfectly acceptable to leave delimiters out of the proposal, then move directly to a delimiter proposal that will be the standard for *all* placeholder delimiters in strings, not just for logging.


-- pmj

[justin]

this.

--j

[Evert Pot]

Thanks Paul, that makes a lot of sense to me.

Evert

[Larry Garfield]

I disagree with that. :-)

The goal of this group is to foster and encourage interoperability
between different projects through useful standardization.

A policy of "we can only bless what is already de facto" means this
group is utterly irrelevant, because we can't do anything but nod and
smile once something has "won in the marketplace". If we do that, then
we will never establish any standards because PHP is a highly variable
marketplace.

We should set our sights a little bit higher than documenting HTML 3.2...

--Larry Garfield

[Larry Garfield]

What the delimiter standard is is less important, IMO, than the fact
there is one. Without it, the spec is incomplete, insecure, and largely
useless. My statement earlier still stands: I'm happy with "Jordi, pick
something", because having something is more important than the
bikeshed. (At the time, the only response to that statement was someone
else echoing "Jordi, pick somthing" <g>.)

If we decide we want a survey to inform that decision, I don't have an
objection to that, but the end result MUST ;-) be one (1) placeholder
standard. Making it configurable is asking for trouble.

As was discussed earlier in the thread, Drupal, Symfony fullstack,
ezPublish, and CakePHP are not the intended consumers for this
interface. Symfony Components, Guzzle, and Solarium are. Drupal,
ezPublish, and Cake will be providing objects of this interface TO those
libraries, which can then translate those placeholders to whatever
Drupal-specific or Cake-specific format they feel like. But Solarium
shouldn't have to think about such details.

--Larry Garfield

[Evert Pot]

> My statement earlier still stands: I'm happy with "Jordi, pick something", because having something is more important than the bikeshed. (At the time, the only response to that statement was someone else echoing "Jordi, pick something" <g>.)

+1 for Jordi picking one :) My only requirement is to pick a something with a start & end delimiter.

Evert

[Moisa Teodor]

I totally agree on taking string substitution/delimiters etc. in a separate proposal, as the topic is blocking the Logger interface proposal.

On Tue, Dec 11, 2012 at 6:30 PM, Paul Jones <pmjo...@gmail.com> wrote:

As it stands, I don't think placeholder delimiters "MUST" ;-) be part of the LoggerInterface proposal.  I think they may be, and maybe even should be, but only after we do the research.  In fact, in the interest of getting LoggerInterface out the door, I think it's perfectly acceptable to leave delimiters out of the proposal, then move directly to a delimiter proposal that will be the standard for *all* placeholder delimiters in strings, not just for logging.

--

Doru Moisa
web: http;//moisadoru.wordpress.com
tel: +40 720 861 922
Bucharest, Romania

[Paul Jones]

On Dec 11, 2012, at 10:56 AM, Larry Garfield wrote:

> On 12/11/12 10:12 AM, Paul Jones wrote:
>>
>> On Dec 11, 2012, at 10:09 AM, Lukas Kahwe Smith wrote:
>>
>>> But if we say that in case there is no existing majority for stuff like this, we effectively say that the given feature can never be covered by a PSR, that would be a sad affair.
>>
>> I disagree. In my opinion, the mission of the group is to find areas of commonality among member projects, and codify those areas of commonality. Where common practices do not exist, I do *not* think it is our mission to create standards out of whole cloth.
>
> I disagree with that. :-)
>
> The goal of this group is to foster and encourage interoperability between different projects through useful standardization.

... based on what member projects are already doing. Our primary audience is the member projects, not the world at large. The world at large is an important, but still secondary, audience.


-- pmj

[Paul Jones]

On Dec 11, 2012, at 11:04 AM, Larry Garfield wrote:

> On 12/11/12 10:30 AM, Paul Jones wrote:
>>
>> On Dec 11, 2012, at 10:22 AM, Evert Pot wrote:
>>
>>> pmj: if you hypothetically had to provide a wrapper that converts one delimiter-format to another, what would be your main issue with that? And: why do you think that that would be a worse situation than making delimiters configurable?
>>
>> My main issue isn't about the wrappers, per se. It's that we're picking a delimiter "because we need something to standardize on" but not finding out what's actually in most common use. If there is a set of delimiters that are clearly more commonly used than others, then we should use that one. If there's no common agreement, then we need to look at what's being used and pick from those.
>>
>> As it stands, I don't think placeholder delimiters "MUST" ;-) be part of the LoggerInterface proposal. I think they may be, and maybe even should be, but only after we do the research. In fact, in the interest of getting LoggerInterface out the door, I think it's perfectly acceptable to leave delimiters out of the proposal, then move directly to a delimiter proposal that will be the standard for *all* placeholder delimiters in strings, not just for logging.
>>
>>
>> -- pmj
>
> What the delimiter standard is is less important, IMO, than the fact there is one. Without it, the spec is incomplete, insecure, and largely useless.

Disagree -- it's still highly useful in that it defines a common API, one that many or most of the member projects use or plan to use.


-- pmj

[Paul Jones]

I feel like I can't stress this strongly enough. Every PSR thus far, even the contentious ones, has been based not on what we imagine projects *ought to do*, but on what they *are in fact doing* (or are already planning on doing).



-- pmj

[Paul Jones]

On Dec 11, 2012, at 11:07 AM, Evert Pot wrote:

>> My statement earlier still stands: I'm happy with "Jordi, pick something", because having something is more important than the bikeshed. (At the time, the only response to that statement was someone else echoing "Jordi, pick something" <g>.)
>
> +1 for Jordi picking one :) My only requirement is to pick a something with a start & end delimiter.

-1 for "just picking" :-(

If we "just pick" then we're not looking to see what the common practices already are.


-- pmj

[Jordi Boggiano]

Here is what I think:

- Finding commonalities was good for PSR0/1/2 because this was 90%
bikeshed-based. This to me is about ENABLING LIBRARIES TO USE LOGGERS.
Sorry for the caps, but I have said it 20 times, and I keep getting
arguments that it's not good for "me, my framework and I". No framework
will use another framework's logger. Your framework's logger could be
used in the libraries you and your users use though. Most member
projects are *not* who I care about here. The larger ecosystem is.

- There are no commonalities here, the only way to please everyone is to
make placeholders look like: %:foo}

- Placeholders are valuable for some member projects, and do not harm
those that don't want them (the 5 lines of code to handle this are in
the PSR if you don't want to code it yourself in your implementation).

- Multiplying PSRs for the fun of it is counter-productive IMO. It works
for coding standards, it might work for bigger features too. But
placeholders are a little thing that doesn't harm anyone. If you have to
start hinting for PlaceholderableLoggerInterface instead of
LoggerInterface, or maybe hint the latter and then check the former to
see if you can use placeholders or not, well it's simple: nobody will
use them because it's too painful.

I am quite frankly getting tired of reading and answering bikeshedding
emails. It takes a lot of time and energy. I am not targetting Paul in
particular btw. Most emails in the last few days have been about minor
nitpicks and people going back and forth.

I am off for the evening, and unless something relevant is said until
tomorrow I guess I will start a vote. If you think that makes me a bad
person and the PSR worthless, feel free to vote -1. If not I will be
happy to see it pass and I can only hope the future will prove that it
was the right call.

[Paul Jones]

On Dec 11, 2012, at 11:21 AM, Jordi Boggiano wrote:

> - There are no commonalities here

Have you done the research to confirm this? My guess is that, at the very least, we will find that braces (in whatever variation) are more common than percents, but that is only a guess.

> - Multiplying PSRs for the fun of it is counter-productive IMO.

I'm not saying this stuff "for the fun of it." I get that it's frustrating, but it's important in the context of this discussion. Whatever we do should be in relation to the member projects, and as such we need to take them into account.


-- pmj

[Jason Judge]

On Tuesday, 11 December 2012 17:04:11 UTC, Larry Garfield wrote:

As was discussed earlier in the thread, Drupal, Symfony fullstack,
ezPublish, and CakePHP are not the intended consumers for this
interface.  Symfony Components, Guzzle, and Solarium are.  Drupal,
ezPublish, and Cake will be providing objects of this interface TO those
libraries, which can then translate those placeholders to whatever
Drupal-specific or Cake-specific format they feel like.  But Solarium
shouldn't have to think about such details.

--Larry Garfield

This is an important point in my non-voting mind.

If a package is logging its messages to PSR standard, then it will be using the PSR-standard placeholder format (I say format rather than delimiters, because there is much more to the definition of a placeholder, such as letter-case, allowed characters etc). A framework or application using its own placeholder format only needs to provide once converter to transform message strings from PSR placeholders to its own chosen placeholder format. Other non-PSR packages that use other formats are out of scope, and do not need to be worried about here. That was the main thing I was trying to get my head around, but it seems clear now.

So go for it - choose one and run with it :-)

-- Jason

 

[Paul Jones]

On Dec 11, 2012, at 11:21 AM, Jordi Boggiano wrote:

> I am quite frankly getting tired of reading and answering bikeshedding
> emails. It takes a lot of time and energy. I am not targetting Paul in
> particular btw. Most emails in the last few days have been about minor
> nitpicks and people going back and forth.

As an aside, "bikeshedding" may not mean what you (and Larry apparently) think it does here.

The bikeshed metaphor concerns insignificant details unrelated to a larger and more complex plan; the classic example is the color of a bikeshed in relation to the plans for nuclear power plant.

In this case, the delimiters in messages are an important detail in the plan; thus, not bikeshedding.

Presented for your consideration. :-)


-- pmj

[Devon H. O'Dell]

2012/12/11 Paul Jones <pmjo...@gmail.com>:

To bikeshed more, I think his usage is probably fine. You can bikeshed
over important details. Nobody ever said that the color was
unimportant ;)

> Presented for your consideration. :-)

http://www.freebsd.org/cgi/getmsg.cgi?fetch=506636+517178+/usr/local/www/db/text/1999/freebsd-hackers/19991003.freebsd-hackers
is the origin of its use in open source pop culture, for anyone
interested who has not yet read it. In particular, that sleep(1)
needed to handle non-integer arguments was certainly an important
detail. In many cases, it's possible to just pick one and do it. I
think that's accurate for some of the mails I've seen fly by recently.

That said, in a "standards-approving body" of sorts, consensus is very
important.

I choose purple.

> -- pmj

--dho

[Beau Simensen]

As someone who aims to write framework agnostic libraries I am very excited about the idea of having a standard logging interface at my disposal. Thanks for all of the hard work from FIG people (and especially Jordi) for bringing this close to reality!

On the topic of placeholder configurability, I can see consumers of loggers not wanting to use placeholders at all if they are configurable. The work required to generate the messages correctly would add considerable complexity to something that would otherwise be simply sending a fixed string.

As a logger consumer I'd prefer to just know that I can always rely on one placeholder style and call it good.

[Paul Jones]

Hi all,

It appears that Jordi's Monolog LineFormatter already uses and expects %foo% delimiters by default. So it's natural for him to default to that as the delimiter he's been encouraged to "just pick."

Even so, we really ought to do a survey of member projects to see what they use already, and go with whatever is most common.


-- pmj

[Larry Garfield]

Then please do so and see if there is a clear de facto consensus one way
or another. Otherwise, Jordi is "chairing" this discussion so I don't
know who else to nominate to say "dude, we need something, just pick one."

%foo% isn't my preferred format either, but it's a minor enough detail
that I don't care to belabor the point.

--Larry Garfield

[Jordi Boggiano]

The formatter uses that for the log line format description, but does
not replace anything in the message itself. So no need for conspiracy
theories, monolog needs changes to add support for message placeholders
no matter which delimiters are chosen. You can see that in the branch I
linked earlier. Can't find it I'm on my phone. I am also a big symfony
user and use {} for routing there and {{}} in twig, so I don't think im
particularly biased one way or the other.

As I said before, {} is inferior IMO because it's sometimes used with
spaces, sometimes not. Which leads to confusion. I have seen %% used in
many places over the years, not just my little lib. Since I personally
don't care i picked the one that seems the less bad.

Cheers
From: Paul Jones
Sent: 11/12/2012 19:13
To: php...@googlegroups.com
Subject: Re: LoggerInterface - Last Feedback Call

Hi all,

It appears that Jordi's Monolog LineFormatter already uses and expects
%foo% delimiters by default. So it's natural for him to default to
that as the delimiter he's been encouraged to "just pick."

Even so, we really ought to do a survey of member projects to see what
they use already, and go with whatever is most common.


-- pmj

[Paul Jones]

Yeah, I'm doing a survey even now. Downloading all the member projects is time-consuming. And if it's %foo% then so be it. :-)


-- pmj

[Paul Jones]

On Dec 11, 2012, at 12:23 PM, Jordi Boggiano wrote:

> As I said before, {} is inferior IMO because it's sometimes used with
> spaces, sometimes not. Which leads to confusion. I have seen %% used in
> many places over the years, not just my little lib. Since I personally
> don't care i picked the one that seems the less bad.

Man, I hear you. Not making conspiracy accusations. :-)

I'm doing the survey even now. I'll report back as soon as I can.


-- pmj

[Paul Jones]

Hi all,


Survey Results
--------------

agavi : no obvious interpolation mechanism
assetic : no obvious logging mechanism
aws-sdk-1.5.17.1 : no obvious interpolation mechanism
Buzz : uses sprintf()
cakephp : no obvious interpolation mechanism
chisimba : appears to use PEAR_Log
composer : no obvious logging mechanism
doctrine2 : no obvious interpolation mechanism
drupal : various: `%foo`, `@foo`, `!foo`
ezpublish : no obvious interpolation mechanism
FLOW3 : no obvious interpolation mechanism
jackalope : no obvious logging mechanism
joomla-platform : no obvious interpolation mechanism
lithium : no obvious interpolation mechanism
monolog : no obvious interpolation mechanism
packagist : uses monolog
PEAR_Log : no obvious interpolation mechanism
phpbb3 : no obvious interpolation mechanism
ppi2 : no obvious interpolation mechanism
Propel : no obvious interpolation mechanism
pyrocms : no obvious interpolation mechanism
SabreDAV : no obvious logging mechanism
solar : uses `{:foo}` placeholders throughout
sugarcrm_dev : no obvious interpolation mechanism
symfony : uses monolog
zf2 : no obvious interpolation mechanism
zikula : uses Zend_Log

(N.b.: Please correct the survey above as needed.)


Summary
-------

Of the 27 surveyed projects:

- 4 (14.8%) neither provide nor use logging
- 21 (77.8%) provide or use logging, without message interpolation
- 2 ( 7.4%) provide or use logging, with message interpolation

Some of the projects *do* accept an array of data, but they don't interpolate it into the message. They log it along with the message as a var_dump() or something similar. (I failed to keep track of which ones do this.)

So of all the member projects, only two support interpolating variables into placeholders, with the following delimiter sets:

- Solar: `{:foo}`
- Drupal: `%foo`, `@foo`, `!foo`


Analysis
--------

I am surprised, as usual, by the survey results. Even though I personally find interpolation of variables into messages to be useful, it appears that doing so is a relatively uncommon thing. (One wonders how common it is "in the wild" as a result.)

If the above survey is accurate, then I have to argue that specifying the interpolation of variables into message strings is not appropriate for the LoggerInterface proposal.

The passing of a $context array *does* seem appropriate, since some of the projects do so already. But what the logger implementation does with that $context data seems to be something that should be left to implementors.

So my position is modified yet again: delimiters should not be specified, and what to do with the $context array should also not be specified, and should be left to implementors.

To wit:

- Delete the bullet point contents beginning with "The message MAY contain placeholders in %variable% format (e.g. %foo%) ..." through to the sample implementation.

- Replace with language to this effect: "Implementors MAY use the $context array as they wish. Uses include but are not limited to interpolating values into the message, dumping the array into the log, or ignoring it entirely."

This allows for what appears to be the overwhelmingly majority use of sending just a message to the logger, *and* allows for extended usage by what appears to be a minority of projects.


-- pmj