From 8bfe3a5a94b5dea7692856509803b6a54ce072ee Mon Sep 17 00:00:00 2001
From: Lloyd Hilaiel <lloyd@hilaiel.com>
Date: Wed, 17 Aug 2011 17:51:37 +0300
Subject: [PATCH] rather than imposing restrictions on structure of logged
objects we should make all required fields proper parameters that are obvious
upon inspection of the signature of the log function. issue #168
---
browserid/app.js | 2 +-
browserid/lib/db_mysql.js | 4 ++--
libs/metrics.js | 18 ++++++++++--------
verifier/app.js | 27 ++++++++++++---------------
4 files changed, 25 insertions(+), 26 deletions(-)
diff --git a/browserid/app.js b/browserid/app.js
index 8adc439ed..e31733579 100644
--- a/browserid/app.js
+++ b/browserid/app.js
@@ -81,7 +81,7 @@ function router(app) {
// this should probably be an internal redirect
// as soon as relative paths are figured out.
app.get('/sign_in', function(req, res, next ) {
- metrics.userEntry('browserid', req);
+ metrics.userEntry(req);
res.render('dialog.ejs', {
title: 'A Better Way to Sign In',
layout: false,
diff --git a/browserid/lib/db_mysql.js b/browserid/lib/db_mysql.js
index 964281d49..d81395526 100644
--- a/browserid/lib/db_mysql.js
+++ b/browserid/lib/db_mysql.js
@@ -83,7 +83,7 @@ function logUnexpectedError(detail) {
var where;
try { dne; } catch (e) { where = e.stack.split('\n')[2].trim(); };
// now log it!
- metrics.report('db', { type: "unexpected", message: "unexpected database failure", detail: detail, where: where });
+ metrics.report('unexpected', { message: "unexpected database failure", detail: detail, where: where });
}
// open & create the mysql database
@@ -426,7 +426,7 @@ exports.pubkeysForEmail = function(email, cb) {
exports.removeEmail = function(authenticated_email, email, cb) {
exports.emailsBelongToSameAccount(authenticated_email, email, function(ok) {
if (!ok) {
- metrics.report('security', { type: 'security', msg: authenticated_email + ' attempted to delete an email that doesn\'t belong to her: ' + email });
+ metrics.report('security', { msg: authenticated_email + ' attempted to delete an email that doesn\'t belong to her: ' + email });
cb("authenticated user doesn't have permission to remove specified email " + email);
return;
}
diff --git a/libs/metrics.js b/libs/metrics.js
index f05328226..ae0aaf354 100644
--- a/libs/metrics.js
+++ b/libs/metrics.js
@@ -88,15 +88,18 @@ function setupLogger() {
}
// entry is an object that will get JSON'ified
-exports.report = function(category, entry) {
- // entry must have at least a type
- if (!entry.type)
- throw new Error("every log entry needs a type");
-
+exports.report = function(type, entry) {
// setup the logger if need be
setupLogger();
+ // allow convenient reporting of atoms by converting
+ // atoms into objects
+ if (entry === null || typeof entry !== 'object') entry = { msg: entry };
+ if (entry.type) throw "reported metrics may not have a `type` property, that's reserved";
+ entry.type = type;
+
// timestamp
+ if (entry.at) throw "reported metrics may not have an `at` property, that's reserved";
entry.at = new Date().toUTCString();
// if no logger, go to console (FIXME: do we really want to log to console?)
@@ -104,9 +107,8 @@ exports.report = function(category, entry) {
};
// utility function to log a bunch of stuff at user entry point
-exports.userEntry = function(category, req) {
- exports.report(category, {
- type: 'signin',
+exports.userEntry = function(req) {
+ exports.report('signin', {
browser: req.headers['user-agent'],
rp: req.headers['referer'],
// IP address (this probably needs to be replaced with the X-forwarded-for value
diff --git a/verifier/app.js b/verifier/app.js
index 1116b881f..e2afb4fb2 100644
--- a/verifier/app.js
+++ b/verifier/app.js
@@ -72,11 +72,10 @@ function doVerify(req, resp, next) {
audience,
function(payload) {
// log it!
- metrics.report('verifier', {
- type: 'verify',
- result: 'success',
- rp: payload.audience
- });
+ metrics.report('verify', {
+ result: 'success',
+ rp: payload.audience
+ });
result = {
status : "okay",
@@ -88,21 +87,19 @@ function doVerify(req, resp, next) {
resp.json(result);
},
function(errorObj) {
- metrics.report('verifier', {
- type: 'verify',
- result: 'failure',
- rp: audience
- });
+ metrics.report('verify', {
+ result: 'failure',
+ rp: audience
+ });
resp.json({ status: "failure", reason: errorObj });
}
);
} catch (e) {
console.log(e.stack);
- metrics.report('verifier', {
- type: 'verify',
- result: 'failure',
- rp: audience
- });
+ metrics.report('verify', {
+ result: 'failure',
+ rp: audience
+ });
resp.json({ status: "failure", reason: e.toString() });
}
}
--
GitLab