-
Notifications
You must be signed in to change notification settings - Fork 136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add option to log all websocket messages in and out #81
base: master
Are you sure you want to change the base?
Conversation
@@ -20,6 +20,31 @@ import ( | |||
"github.com/gorilla/websocket" | |||
) | |||
|
|||
type Logger interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was about to suggest using the pre-existing Logger
in ocpp/types.go
, but then I realised this would add a dependency to the ws
module, which should be completely agnostic.
What do you think about moving the existing Logger
interface from ocpp/types.go
to a separate module, to be re-used by the websockets as well? Mostly for easier/unified integration with existing loggers. Yes, you would lose that comfy (id string, data []byte)
interface, so that's why I'm asking if you agree with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the current interface since it allows customized behavior when logging the raw ocpp-j messages
@@ -371,6 +396,7 @@ func (server *Server) Write(webSocketId string, data []byte) error { | |||
if !ok { | |||
return fmt.Errorf("couldn't write to websocket. No socket with id %v is open", webSocketId) | |||
} | |||
log.SendMessage(webSocketId, data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep in mind, that this is logged before any network operation actually occurred. It may potentially be followed by an error/reconnect issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I wanted to log basically which OCPP-J messages the server/client "think" they're sending/receiving so this is fine for my use case, since at this point we have the []byte
s
I'm not sure if you'd consider this generally useful, so feel free to close, but it's been useful for us.