On Aug 11, 2013, at 11:37 AM, Igor Galić <i.ga...@brainsware.org> wrote:
> > > I squashed the stuff from the branch and pushed it into a new one: > > reviews are very welcome: > > > https://git-wip-us.apache.org/repos/asf?p=trafficserver.git;a=shortlog;h=refs/heads/consistent-errors Thanks Igor, I'll review on Monday > > i > > ----- Original Message ----- >> >> >> ----- Original Message ----- >>> 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. >> >> I'm perfectly fine with delivering this non-API for our in-tree plugins then. >> >> For compelteness: Can someone please enlighten me as to how >> TSTextLogObjectWrite() fits into the this whole picture? >> >> >>> 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. >> >> >> +1, go for it. >> >>> J >>> >>> >> >> -- >> Igor Galić >> >> Tel: +43 (0) 664 886 22 883 >> Mail: i.ga...@brainsware.org >> URL: http://brainsware.org/ >> GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE >> >> > > -- > Igor Galić > > Tel: +43 (0) 664 886 22 883 > Mail: i.ga...@brainsware.org > URL: http://brainsware.org/ > GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE
