Review/std.logger

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

Initial pre-review discussion

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 thread

Review 2

Result

Voting for inclusion into std.experimental has been started

Description

Review thread

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

Voting thread

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

1. Jakob Ovrum* - No (http://forum.dlang.org/post/rgahginysupbnodumsmy@forum.dlang.org)
2. Rikki Cattermole - Yes (http://forum.dlang.org/post/lr7c0l$2cvn$1@digitalmars.com)
3. Andrei Alexandrescu* - No (http://forum.dlang.org/post/lr7dql$2e9n$1@digitalmars.com)
4. Martin Drašar - Yes (http://forum.dlang.org/post/mailman.138.1406616733.16021.digitalmars-d@puremagic.com)
5. John Colvin - Yes (http://forum.dlang.org/post/clsfzgbrwlwosrgwbboi@forum.dlang.org)
6. Casey - Yes (http://forum.dlang.org/post/tyrgmitxvreylllzbmcn@forum.dlang.org)
7. Francesco Cattoglio - No (http://forum.dlang.org/post/uuwvcxxwhicwcanakzsy@forum.dlang.org)
8. Byron Heads - No (http://forum.dlang.org/post/lr87s9$24d$1@digitalmars.com)
9. ponce - Yes (http://forum.dlang.org/post/stsqbrbrbfxtcjptkwxr@forum.dlang.org)
10. Sean Kelly* - No (http://forum.dlang.org/post/ithprsbaaumbhzmgvmdb@forum.dlang.org)
11. MrSmith - Yes (http://forum.dlang.org/post/idttmoegjegwnbehhvmk@forum.dlang.org)
12. Martin Nowak* - No (http://forum.dlang.org/post/lretfc$p7i$1@digitalmars.com)
13. 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.