search

eclipse-paho / paho.mqtt.golang

Other
2.78k stars 534 forks source link

Feature Request: Add option for structured logging #519

Open Jarema opened 3 years ago

Jarema commented 3 years ago

Current logging solution does not allow for clean structured logs. Not only because all is printed as a string, but also because custom format is used:

[net]      outgoing waiting for an outbound message\n

I would prefer to have:

{
  "message": "outgoing waiting for an outbound message",
  "event.type": "mqtt.net",
  "level": "debug"
}

Without this, using this library where structured logging is used (and it's basically a standard now in Cloud Native solutions) really hurts the readability.

MattBrittan commented 3 years ago

The logging in this library is primarily for debugging; I would expect it to be enabled only in cases where you are attempting to trace a problem within the library itself (a lot of information is produced and much of it is fairly meaningless unless you understand the internals of the paho package). As such the information produced is really intended for use by itself (rather then intermingled with logging from other sources).

The closest thing to a standard for logging in Go is Logger in the standard library and this library uses a compatible (but minimal!) method. This is inline with other similar projects (such as gRPC).

Calls to the logger are fairly standard (this is not enforced but is a convention) e.g.:

DEBUG.Println(NET, "outgoing waiting for an outbound message")
ERROR.Println(CLI, "Connecting to", broker, "CONNACK was not CONN_ACCEPTED, but rather", packets.ConnackReturnCodes[rc])

Which should make it fairly easy to make your implementation of Println log in your preferred manner (use the logger to determine level and first argument to Println to determine the event.type with the remainder being message). While Printf is supported I think its only used in one place so I'd be happy to see it removed; there may be a few logging calls that do not follow the above convention and I'd also welcome a pull request that resolves this.

Note that for more general logging I'd expect the callbacks to be used to raise log entries (e.g. OnConnect, OnConnectionLost, OnConnectAttempt).