On Aug 8, 2013, at 6:40 AM, Igor Galić <i.ga...@brainsware.org> wrote:
>
>
> ----- Original Message -----
>>
>>
>> Hi folks,
>>
>> right now many of plugins send error messages without prefixing the plugin
>> they come from, this bad practice is also continued in our examples.
>>
>> many error messages also close with `\n`, although this is not necessary.
>>
>> We should fix this by "normalizing" the way we send those error messages.
>>
>> My proposal is either:
>>
>> TSError("%s: cannot parse file %s", PLUGIN_NAME, filename);
>>
>> xor:
>>
>> TSError("[%s] cannot parse file %s", PLUGIN_NAME, filename);
>>
>> I have no strong leanings towards either. BUT: We should have one consistent
>> format between examples and plugins, as well as newly imported code.
>>
>> The word "error" should not be repeated, as it is already in the "severity
>> (`TSError()`). There should be *no* `\n` at the end of the message.
>>
>> Normally I would just grep the source and see which use-case is the
>> prevailing one, but I'm too tired and I'm flying to Ireland tomorrow,
>> so I'm leaving this as "an exercise to the student" ;)
>>
>> reference issue: https://issues.apache.org/jira/browse/TS-2106
>
>
>
> I've been looking at this for the past two days and I must say, so far
> the combination I like best are the macros in the gzip plugin:
>
>
>
> #define debug(fmt, args...) do { \
> TSDebug(PLUGIN_NAME, "DEBUG: [%s:%d] [%s] " fmt, __FILE__, __LINE__,
> __FUNCTION__ , ##args ); \
> } while (0)
>
> #define info(fmt, args...) do { \
> TSDebug(PLUGIN_NAME, "INFO: " fmt, ##args ); \
> } while (0)
>
> #define warning(fmt, args...) do { \
> TSDebug(PLUGIN_NAME, "WARNING: " fmt, ##args ); \
> } while (0)
>
> #define error(fmt, args...) do { \
> TSError("[%s], [%s:%d] [%s] ERROR: " fmt, PLUGIN_NAME, __FILE__, __LINE__,
> __FUNCTION__ , ##args ); \
> TSDebug(PLUGIN_NAME, "[%s:%d] [%s] ERROR: " fmt, __FILE__, __LINE__,
> __FUNCTION__ , ##args ); \
> } while (0)
>
> #define fatal(fmt, args...) do { \
> error(fmt, ##args ); \
> exit(-1); \
> } while (0)
These are not terrible, though I'd tend to do something like:
#define debug(fmt, ...) do { \
if (unlikely(TSIsDebugTagSet(PLUGIN_NAME))) { \
TSDebug(PLUGIN_NAME, fmt, ##__VA_ARGS__); \
} \
} while(0)
so that format string arguments are not evaluated unless we are going to print
something.
>
>
> What'd you think we make this a default API?
I'm happy to make this change for in-tree plugins. I don't think this is ready
for external API status. I don't much like the dependency on PLUGIN_NAME, and
there are plugins (eg. spdy) that use the debug tag differently.
I'm definitely in favour of having the in-tree plugins log consistently though.
For an external debug logging API, I'd like to see something more similar to
the internal Debug API, with real suport for log levels and
proxy.config.diags.show_location.
J
