Skip to content
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 request logging to worker #27

Open
jenschelkopf opened this issue Nov 7, 2016 · 5 comments
Open

Add request logging to worker #27

jenschelkopf opened this issue Nov 7, 2016 · 5 comments

Comments

@jenschelkopf
Copy link

jenschelkopf commented Nov 7, 2016

It would be a lot easier to pinpoint problems using remotedev-server if it logged requests. For example, if you send a request to a server and get a 502 response you'll want to know if the request ever made it to the server.

It would be sweet to pass logging options to remotedev-server that are then passed to the logging system so I can control the format and verbosity.

I've had good luck with morgan.

@jenschelkopf jenschelkopf changed the title Add request logging Add request logging to worker Nov 7, 2016
@zalmoxisus zalmoxisus mentioned this issue Nov 9, 2016
Merged
10 tasks
@zalmoxisus
Copy link
Owner

While morgan is a good option for express, we still need to log socket requests.

@jondubois, is there an option to have this for socketcluster? We could use middlewares, but I think it would be rather useful to have it out of the box for debugging.

@jondubois
Copy link

@zalmoxisus you can control the log level in SC, but currently, it doesn't go as far as showing each individual request/response but yes it sounds reasonable to do that if the log level is high.
I'll add this to my TODOs.

zalmoxisus added a commit that referenced this issue Nov 9, 2016
@jondubois
Copy link

@zalmoxisus After looking into morgan, I think that there are too many different possible ways to log HTTP requests to let SC to decide. Also some users of SC use it purely as a WebSockets microservice (so they don't use HTTP at all) - So I don't want to force this dependency on all users. Also express is not a core part of SC itself (it's only part of the default boilerplate app that comes with SC).

I ended up adding morgan as an express middleware in the sample SC boilerplate (for new projects) - But for existing projects like remotedev-server, I would recommend manually adding morgan (or whatever logging tool/format you want to use) somewhere inside worker.js like I did in the sample boilerplate app here: https://github.com/SocketCluster/socketcluster/blob/ff2610aaf3d273a660105cba5d96b5095dc10a85/sample/worker.js#L20

For logging individual WebSocket events, I will see if there are existing npm modules which do that and if not, I might write one for SC - But I would like to keep this module optional (like morgan) for flexibility reasons so it will need to be added manually in user-code (E.g. inside worker.js somewhere).

So the idea is that right now SC handles the 'error logging' but the 'access logging' is the responsibility of the user. I'm happy to hear feedback though about all this.

@zalmoxisus
Copy link
Owner

@jondubois, totally agree that logging HTTP (express) requests shouldn't be part of socketcluster, moreover that it's very simple to add. However, I'd like to see an example on how to log socket requests.

@jondubois
Copy link

jondubois commented Nov 14, 2016

@zalmoxisus Regarding WebSocket messages - I will write an optional add-on module for SC to make it easy.

To decide whether or not to integrate this module as part of SC's core logging, I'll need to think about it a bit more (and get some feedback). The are a lot of ways to implement this type of 'access logging' (depending on the company and environment) so that's the argument for keeping it as a separate optional module (in userland).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants