patternjavascriptMinor
node.js Syslog server recording to MS-SQL database
Viewed 0 times
syslogsqlnoderecordingdatabaseserver
Problem
This 43 line program is designed to listen for Syslog messages on port 514 and record the messages to an MS SQL Server database table.
I would appreciate feedback from experienced node.js developers.
I am just starting to learn node.js. I'm average at JavaScript, still getting my head around closures. I am really struggling to get my head around the node.js event driven programming paradigm.
If I try to access the SQL server connection outside of the
So for this reason I had to place the receive message and save it in database logic inside the
Anyway the code works (amazingly!) and seems to be quite efficient, saving many dozens of syslog messages per second to the local MS SQL database with negligible CPU impact...
Are there any fundamental flaws in this code, and any changes that should be made to improve it?
I guess I am amazed that this can be done in just one page of code. node.js is very cool...
`var dgram = require("dgram");
var server = dgram.createSocket("udp4");
var sql = require('msnodesql');
var conn_str = "Driver={SQL Server Native Client 11.0}; Server=.; Database=SYSLOG; UID=xxxxxxx; PWD=xxxxxxxx;";
// open the database connection
sql.open(conn_str, function (err, conn) {
if (err) {
console.log("error", err);
return;
}
//database connection succeeded, log a mesage into the syslog.incoming table
conn.queryRaw("insert into Incoming(Source,Message) values('LOCAL','node.js syslog server started')");
//create an event listener for when a syslog message is recieved
server.on("message", function (msg, rinfo) {
//sanitise the data by replacing single quotes with two single-quotes
var message = msg.toString().replace(/'/g, "''")
var src = rinfo.addre
I would appreciate feedback from experienced node.js developers.
I am just starting to learn node.js. I'm average at JavaScript, still getting my head around closures. I am really struggling to get my head around the node.js event driven programming paradigm.
If I try to access the SQL server connection outside of the
sql.open callback function, I get the error that the database connection is closed.So for this reason I had to place the receive message and save it in database logic inside the
sql.open callback function, which seems messy and counter-intuitive, but that is probably due to my not quite fully understanding the event driven model?Anyway the code works (amazingly!) and seems to be quite efficient, saving many dozens of syslog messages per second to the local MS SQL database with negligible CPU impact...
Are there any fundamental flaws in this code, and any changes that should be made to improve it?
I guess I am amazed that this can be done in just one page of code. node.js is very cool...
`var dgram = require("dgram");
var server = dgram.createSocket("udp4");
var sql = require('msnodesql');
var conn_str = "Driver={SQL Server Native Client 11.0}; Server=.; Database=SYSLOG; UID=xxxxxxx; PWD=xxxxxxxx;";
// open the database connection
sql.open(conn_str, function (err, conn) {
if (err) {
console.log("error", err);
return;
}
//database connection succeeded, log a mesage into the syslog.incoming table
conn.queryRaw("insert into Incoming(Source,Message) values('LOCAL','node.js syslog server started')");
//create an event listener for when a syslog message is recieved
server.on("message", function (msg, rinfo) {
//sanitise the data by replacing single quotes with two single-quotes
var message = msg.toString().replace(/'/g, "''")
var src = rinfo.addre
Solution
From a once over:
-
Comments should be worth the space they take up, my favourite offender is this one:
-
In this block, you should either use 1 comma separated
-
In a script of this size I would consider merging some statements, this
might as well be
Still, all in all a solid script.
-
Comments should be worth the space they take up, my favourite offender is this one:
// open the database connection
sql.open(conn_str, function (err, conn) {- I want to point to is msnodesql maintained?
- I also want to point out that storing uid/pwd in your script can be dangerous, I hope this is not a production database, which does not have sensitive data
- As Nelson Menezes mentioned, you should look into using prepared statements
-
In this block, you should either use 1 comma separated
var or semicolons:var message = msg.toString().replace(/'/g, "''")
var src = rinfo.address.toString().replace(/'/g, "''")
var s = "insert into Incoming(Source,Message) values('"+src+"','"+message+"')";-
In a script of this size I would consider merging some statements, this
console.log(s);
console.log(err);
return;might as well be
return console.log(s, '\n', err);});// end of sql.open()
Still, all in all a solid script.
Code Snippets
// open the database connection
sql.open(conn_str, function (err, conn) {var message = msg.toString().replace(/'/g, "''")
var src = rinfo.address.toString().replace(/'/g, "''")
var s = "insert into Incoming(Source,Message) values('"+src+"','"+message+"')";console.log(s);
console.log(err);
return;return console.log(s, '\n', err);Context
StackExchange Code Review Q#38951, answer score: 2
Revisions (0)
No revisions yet.