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 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
