lorenzodonini / ocpp-go

Open Charge Point Protocol implementation in Go
MIT License
262 stars 125 forks source link

Logging requests/responses #150

Closed andig closed 1 year ago

andig commented 1 year ago

It would be nice, if there was a facility for logging all requests/responses sent or received by server/client as seen on the websocket. Such logging could be on the websocket (plain) or on the dispatcher level (parsed).

andig commented 1 year ago

@lorenzodonini is there any simple way for doing so? Maybe I'm overlooking something?

lorenzodonini commented 1 year ago

Do you mean debug logs? Those can be enabled passing your own logger like this:

ws.SetLogger(<your-logger>)
ocppj.SetLogger(<your-logger>)

Or do you expect the full raw message to be visible in the logs? I didn't add that since it can be pretty large, depending on the message type. Not something you typically want to see in logs 😅

andig commented 1 year ago

Or do you expect the full raw message to be visible in the logs? I didn't add that since it can be pretty large, depending on the message type. Not something you typically want to see in logs 😅

Exactly. For our application (https://evcc.io) ww have optional logging for each physical transport, be it http, modbus, signalr or whatever. This has proven invaluable for diagnosing issues.

One current problem is https://github.com/evcc-io/evcc/discussions/3806#discussioncomment-3884592 where different boxen all seem to fail in different ways, not responding to GetConfigurationKeys, doing so if/if not list of keys is specified, doing so if BootNotification is requested or not. I've put logging in all handlers I could find but I can never be sure of the sent messages or if I even plugged all places.

Begin able to log JSON messages (occpj logger), preferably per client, would be a huge help.

lorenzodonini commented 1 year ago

I can add it as an additional debug log for incoming/outgoing requests. It will be just be a huge string as the payload of the log message though. Would that work?

andig commented 1 year ago

Yeah, that would be perfect! Being able to differentiate the messages by chargepoint would be a bonus.

andig commented 1 year ago

@lorenzodonini any chance getting the logging in? We're talking to a lot of different charger types and find it very hard to analyse the behaviour without being able to read all traffic.

lorenzodonini commented 1 year ago

Hey @andig, have a look at https://github.com/lorenzodonini/ocpp-go/pull/167, specifically whether the format of the log line works for you.

I would love to enforce structured logging and make the JSON message and client ID two dedicated fields. This might however break some existing implementations, since the logging interface doesn't include With or WithField yet, and afaict logging libraries cannot agree on a common interface.

Let me know what you think.

andig commented 1 year ago

LGTM. As this is really only for debugging I don't think structured logging (or some other solution apart from logging) is not necessary.

@tarelda what do you think?