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

production vs development vs debug #29

Closed
Martii opened this issue Nov 8, 2014 · 11 comments
Closed

production vs development vs debug #29

Martii opened this issue Nov 8, 2014 · 11 comments
Assignees
Milestone

Comments

@Martii
Copy link

Martii commented Nov 8, 2014

Firstly I just wanted to say this is one of the more clever packages for node projects that I've seen to date. Thank you for your continued effort with this.

Secondly to the questions/explanation for a feature enhancement.

Currently we would like to use your package in our site but it needs a little tweaking that seems to be best served in this package so we don't collide with your replaceConsole.

What we are looking for is a config setting that encompasses production, development and native node debug mode and a "null" mode. e.g. send console method messages to the equivalent of /dev/nul output.

I've done a little preliminary research into this and the following conditionals can achieve testing the environment:

// Usually invoked by deployment
var isProduction = process.env.NODE_ENV === 'production';
// Usually invoked by `$ node index.js`
var isDevelopment = process.env.NODE_ENV !== 'production';
// Usually invoked by `$ node debug index.js`
var isDebug = typeof v8debug === 'object';

What I am currently missing is a solid understanding of this project to commit a pull request at this time (this will most likely change hopefully in the near future) and matching some of your styling/naming conventions.

What I'd like to know is if you would be willing to add some variant of these to your options that encompasses all of these? I read the statement you have in the README.md of:

NodeMonkey is primarily designed for debugging and for now should only be used for such.

... which is one of our goals however since you said:

I haven't implemented any sort of authorization to prevent anyone from gaining access to the data that is dumped out.

... this feature request might be a good minimum start.

Please let me know and if I can assist in any additional way within my means. Thank you for the consideration... once again very "kewl" package.

@jwarkentin
Copy link
Owner

I want to make sure I understand what you're trying to accomplish. If I understood everything right you want:

  1. The ability to completely silence all output to both the terminal and web console
  2. The ability to determine settings based on the NODE_ENV environment variable in a similar manner to express

Did I get that right?

I think those are both valuable things. I should mention, however, that the v0.3.0 branch (though not quite production ready) is a complete rewrite and does implement security for production along with some other exciting features. The initial work on that was done a while ago though and there's been some valuable input this year with use cases. I need to put some more work into it to make sure it has the right architecture moving forward to accommodate the way people would like to use it.

In the meantime I would be happy to make some modifications to support your use case if I can understand it properly. It's probably about time I start tracking them more officially. Any feedback or pull requests are appreciated.

@Martii
Copy link
Author

Martii commented Nov 8, 2014

The ability to completely silence all output to both the terminal and web console

Yes (console.log, console.warn, etc. wise)... it would be awesome to reenable on the fly too for a mixture of environments like the replaceConsole and revertConsole methods.

The ability to determine settings based on the NODE_ENV environment variable in a similar manner to express

Thought this was a node thing with the process obect but if it's express package extended then that is what we use.

Basically this part of the feature request is currently for development e.g. we want to optionally increase logging (console messages) to the terminal/console for debugging purposes... with your package we could use the browser console to minimize the "clog" ;) ... with better security in place it might be useful on production too but we don't want just anyone listening in. We have an auth routine in place where we can filter that but basic out-of-the-box support for pro/dev/debug node modes would be very useful.

Any feedback or pull requests are appreciated.

I've only been tinkering with node-monkey for a couple of weeks testing but today is when I started digging into the current master HEAD code... and tried a few alterations and it gave me some unexpected results... so I need to spend some more quality time examining what is going on with the dependencies, your root code and some of your history... this will take a while... so I'd like to do some PRs for you but it's a bit beyond my reach today.

@Martii
Copy link
Author

Martii commented Nov 8, 2014

Brainstorming I thought of:

doProduction: true,
doDevelopment: true,
doDebug: true

for the config options... e.g. be able to set/clear any of those... if all are false then no console.log, console.warn, etc. output would happen in the terminal/console and the web browser console... but that's just some random thoughts... don't know if that is the direction 0.3.x is taking you.

@jwarkentin
Copy link
Owner

I will definitely increase control over logging through the config as well as add a setOption method to the object to change individual config settings at runtime. There is currently a setConfig method to completely replace the config options passed in when initializing node-monkey.

As it stands, you can control what options you choose to pass in within your own application based on an environment variable or command line option. For example:

var env = process.env.NODE_ENV,
    config = {};

if(env == 'production') {
  config = {
    suppressOutput: true,
    silent: true
  };
}

var nomo = require('node-monkey').start(config);

// The rest of your app

@jwarkentin jwarkentin self-assigned this Nov 9, 2014
@Martii
Copy link
Author

Martii commented Nov 9, 2014

Will:

{
   suppressOutput: true,
   silent: true
}

... these actually shut down/not initialize and reestablish the socket emitter when changed on the fly?

I will definitely increase control over logging through the config as well as add a setOption method to the object to change individual config settings at runtime. There is currently a setConfig method to completely replace the config options passed in when initializing node-monkey.

As it stands, you can control what options you choose to pass in within your own application based on an environment variable or command line option.

Part of the issue here is we are using a MVC style system common to express apps in which we would need to pass a reference to nomo around or define it on the global Object (to avoid multiple instances and global objects are accessible to all modules)... neither is appealing to most of our collaborators and other owners... which is why the environment checks would be useful to be handled within nomo itself... another alternative I think could be a callback option for these options e.g. not just setting a constant value but allowing a function that would continue to be in scope even when the web application is at a "restful state".

@jwarkentin
Copy link
Owner

these actually shut down/not initialize and reestablish the socket emitter when changed on the fly?

No. There is no need to do so that I am aware of.

As for your second point, that's not the case. You can require('node-monkey') as many times as you want and you will always get the original object however it was initialized since modules are cached after being required. You should only call start() one time. For example:

app.js

require('node-monkey').start({
  suppressOutput: true,
  silent: true
});

require('some-module.js');

some-module.js

// This is the same object you initialized in `app.js` - no need to call `start()`
var nomo = require('node-monkey');

@Martii
Copy link
Author

Martii commented Nov 9, 2014

Ahhh well that explains a bit then... I was calling .start multiple times... okay... well as long as it's a single instance that will be awesome to be able to change the settings on the fly with setOption. Thank you for that explanation... still have quite a bit to learn here. :)

No. There is no need to do so that I am aware of.

So my concern if a anonymous user starts listening in to that port on production that is initialized, but not sending anything out (because it's supressed and silent), and somehow manages to hack their way into the socket stream and cause some sort of exploit elevating their privileges either in our app or worse the server/client... wouldn't that be covered by stopping/starting the emitter on the fly?

I really appreciate you spending the time here btw. :)

@jwarkentin
Copy link
Owner

Ah, security is a fun topic which I have a relatively extensive amount of knowledge about :) While there will be a lot more security with v0.3.0 there is still a couple of things that you should do to ensure security.

In the current version of node-monkey that you are using you should absolutely avoid making that port accessible on the public internet. It should be behind a firewall and a NAT ideally. Also, as soon as I implement #26 you should ideally run node-monkey attached to a server with the https module instead of http so long as you have a valid cert. If you are just using it internally you could even do a self-signed cert.

I know there has been a lot of interest in the features and security with v0.3.0. I hope to get some time to work on finishing it and getting it delivered. I definitely will with the holidays coming up if not sooner. In the meantime, I'm still very interested in understanding exactly what you are hoping to get out of node-monkey.

My goal with v0.3.0 is to make it so it can be always running as a part of any app. You will be able to connect with a username and password at any time (different user accounts for different people with different access levels). You will be able to easily run commands from the web browser, or actually open up an SSH connection to your application and run commands (awesome, eh?!). It's built initially with the websocket and SSH interfaces, but many more can be added if they are useful.

My goal has always been primarily to provide useful insight into what's happening within your application while it's running. The ability to run commands that can dump out current state data as well as run commands to modify state and enable or disable debugging in a running application is exactly what I want to provide. Though, the command interface is very useful for other admin type functions as well. However, debugging many node applications can be a pain and my goal is to make it much less so.

@Martii
Copy link
Author

Martii commented Nov 9, 2014

However, debugging many node applications can be a pain and my goal is to make it much less so.

This is why I'm very interested in this project. :) Basically for pro I'm probably not going to initialize nomo at all but I also don't want to close doors forever. It sounds like 0.3.x is along similar lines to the security aspect of this last question I asked... We currently use OAuth to validate users and I'm not entirely sure we would want an additional validation with user/pass on top of that but that is down the line for the time being in our project and is negotiable I think.

Well thank you again for explaining a bit of this projects goals and aspirations... hopefully others can gain some additional insight too that are lurking about. ;)

@jwarkentin
Copy link
Owner

It would be interesting to be able to integrate with other auth systems. I've added issue #30 to track that as a feature with possible implemenation methods. Thanks for sharing your use case and making node-monkey better!

@jwarkentin jwarkentin modified the milestone: v1.0.0 Jul 22, 2016
@jwarkentin
Copy link
Owner

As far as I can tell, all the issues discussed here should be resolved with the new release. Review the documentation and let me know if you don't think this is resolved.

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

No branches or pull requests

2 participants