Review/std.logger
Contents
Description
std.logger is a module authored by Robert Schadek. It is relatively minimalistic module aiming to serve as a standard API base for any common logging functionality one may need in the application. It does not aim to provide many actual "batteries" right now.
Related links
Current state
After final review and voting stage it is undergoing API/implementation polishing and will be moved to std.experimental once all outstanding issues mentioned during voting are taken care of.
Review 1
Proposed extensions (not needed for merging, can be implemented later on)
structured logging
http://forum.dlang.org/post/lpquji$prm$1@digitalmars.com
I think we can provide structured logging support as a non-breaking API extension, so we should not make this part of this review. But here's how I'd imagine such an API to work:
Frontend
- log_ get new overloads which accept (T...) as the last parameter (or if T... is already the last parameter that's fine).
- Add a new struct to logger.core: struct MsgID which is just a strong typedef for UUID
- Add a templated type, KeyValue, which can be used like this:
KeyValue("user", "nobody") //string key / string value
KeyValue("age", 42); //string key / T value
KeyValue("msg", "Hello %s! %s", "World", 42); //string key/fmt val
- KeyValue stores it's parameters, no string processing yet
- Multivalue parameters handled by many KeyValue with same key? Might complicate backend. Or don't support multivalue at all? Or KeyValue("key", MultiValue(a, ,b, c)) (MultiValue == Tuple?)
- Structured loggers do not use msg, instead they use a KeyValue with "msg" key. This is cause you usually want different messages with structured loggers. We still keep everything in one function, so the user doesn't have to do "if(structuredlogger) logstruct() else log()" for every log message.
- MsgID marks the end of normal format parameters. See example below. This is also the reason why we can't use UUID directly
Usage:
string error;
logf("Something bad happened: %s", error,
MsgID("abcd-valid-uuid"), //MsgID--> end of fmt params
KeyValue("msg", "Something bad happend"),
KeyValue("error-code", error));
output: normal backend:
test.d:42 Something bad happened: out of memory
structured backend: (only example, exact format backend specific)
{
"uuid": "abcd-valid-uuid",
"msg": "Something bad happened",
"error": "out of memory",
"file": "test.d",
"line": 42
}
The next part is an efficient Backend Layer:
class StructuredLogger : Logger
{
logHeader;
writeLogMsg; //Not used
finishLogMsg;
void logKey(string key);
void valuePart(const(char)[] part);
void finishValue(bool last); //Last only if we support multivalue
}
Usage:
auto slog = new StructuredLogger();
slog.logHeader(...);
foreach(KeyValue kv; T...)
{
slog.logKey(kv.key);
//Need slog -> outputrange adapter: map put<>valuePart
//see https://github.com/burner/logger/pull/9
formattedWrite(wrap(slog), kv.formatstring, kv.args);
slog.finishValue(true);
}
finishLogMsg();
Result
No formal result expected for early review. Opinions expressed, arguments made.
Description
Review 2
Result
Voting for inclusion into std.experimental has been started
Description
Pre-voting review at some point turned into discussion about different API options and necessary features. Lot of changes have been done based on feedback. There is still no full agreement about target API (and will probably never be) but package has been confirmed to be used by several real-world users and should be ready for inclusion to std.experimental
Voting for std.experimental
Votes
Votes for inclusion into std.experimental. As original description did not match actual expectations of the process from Andrei I adjusted some of votes based on more detailed explanations. Thus "yes" votes that list clear critical API requirements that not yet met has been turned into "No".
- Jakob Ovrum* - No (http://forum.dlang.org/post/rgahginysupbnodumsmy@forum.dlang.org)
- Rikki Cattermole - Yes (http://forum.dlang.org/post/lr7c0l$2cvn$1@digitalmars.com)
- Andrei Alexandrescu* - No (http://forum.dlang.org/post/lr7dql$2e9n$1@digitalmars.com)
- Martin Drašar - Yes (http://forum.dlang.org/post/mailman.138.1406616733.16021.digitalmars-d@puremagic.com)
- John Colvin - Yes (http://forum.dlang.org/post/clsfzgbrwlwosrgwbboi@forum.dlang.org)
- Casey - Yes (http://forum.dlang.org/post/tyrgmitxvreylllzbmcn@forum.dlang.org)
- Francesco Cattoglio - No (http://forum.dlang.org/post/uuwvcxxwhicwcanakzsy@forum.dlang.org)
- Byron Heads - No (http://forum.dlang.org/post/lr87s9$24d$1@digitalmars.com)
- ponce - Yes (http://forum.dlang.org/post/stsqbrbrbfxtcjptkwxr@forum.dlang.org)
- Sean Kelly* - No (http://forum.dlang.org/post/ithprsbaaumbhzmgvmdb@forum.dlang.org)
- MrSmith - Yes (http://forum.dlang.org/post/idttmoegjegwnbehhvmk@forum.dlang.org)
- Martin Nowak* - No (http://forum.dlang.org/post/lretfc$p7i$1@digitalmars.com)
- David Nadlinger* - No (http://forum.dlang.org/post/anzeespykooeiaqisfjg@forum.dlang.org)
6 Yes, 7 No
Members of https://github.com/orgs/D-Programming-Language/teams/team-phobos are marked with *
Results
While there were many objections against specific design choices overall attitude to the proposal is positive and utility of standard logging module is not questioned. Discussion has naturally moved to GitHub PR which is indeed most practical course of actions - once Phobos reviewers make sure all concerns listed are taken care of, it can go striaght for another approval voting without any additional formal review overheead.
Effective outcome is thus : postponed until blockers are taken care of.
Review 3
Review 3 has effectivelty never ended - comments kept coming to http://forum.dlang.org/post/deltfarxxzxtswuayksy@forum.dlang.org in a slow flow with various issues fixed over a long period of time.
At some point it was decided that it could take forever and more practical approach would be to continue development in std.experimental
