From a5bbf25ddcd54d8ffd735127f3f3112d1d5057dd Mon Sep 17 00:00:00 2001 From: Unitech Date: Wed, 29 Mar 2017 19:04:57 +0200 Subject: [PATCH] (stackparser/cache) harden test + fixes --- lib/Interactor/TransactionAggregator.js | 7 +- lib/Interactor/Utility.js | 16 ++- test/interface/aggregator.mocha.js | 13 --- test/interface/cache.mocha.js | 61 +++++++++++ test/interface/misc/trace_factory.js | 1 + test/interface/stacktrace.mocha.js | 128 ++++++++++++++++++++++++ test/pm2_programmatic_tests.sh | 4 + 7 files changed, 213 insertions(+), 17 deletions(-) create mode 100644 test/interface/cache.mocha.js create mode 100644 test/interface/stacktrace.mocha.js diff --git a/lib/Interactor/TransactionAggregator.js b/lib/Interactor/TransactionAggregator.js index 25594244..1585a8ff 100644 --- a/lib/Interactor/TransactionAggregator.js +++ b/lib/Interactor/TransactionAggregator.js @@ -624,7 +624,10 @@ var TransactionAggregator = module.exports = function (pushInteractor) { spans.forEach(function (span) { // if empty make sure that it doesnt exist - if (!span.labels.stacktrace || typeof(span.labels.stacktrace) !== 'string') + if (!span || + !span.labels || + !span.labels.stacktrace || + typeof(span.labels.stacktrace) !== 'string') return; // you never know what come through that door @@ -644,6 +647,8 @@ var TransactionAggregator = module.exports = function (pushInteractor) { }); spans.forEach(function (span) { + if (!span || !span.labels) + return; delete span.labels.stacktrace; }) } diff --git a/lib/Interactor/Utility.js b/lib/Interactor/Utility.js index c64da7ef..e8c8816d 100644 --- a/lib/Interactor/Utility.js +++ b/lib/Interactor/Utility.js @@ -61,9 +61,11 @@ Cache.prototype.get = function (key) { var value = this._cache[key] if (value) return value - var value = this._miss(key) + value = this._miss(key) + if (value) this.set(key, value) + return value } @@ -100,12 +102,20 @@ StackTraceParser.prototype.parse = function (stack) { for (var i = 0, len = stack.length; i < len; i++) { var callsite = stack[i]; + // avoid null values - if (!callsite.file_name || !callsite.line_number) continue; + if (!callsite || + !callsite.file_name || + !callsite.line_number) + continue; + var type = (!path.isAbsolute(callsite.file_name) && callsite.file_name[0] !== '.') ? 'core' : 'user' // only use the callsite if its inside user space - if (!callsite || type === 'core' || callsite.file_name.indexOf('node_modules') > -1 || callsite.file_name.indexOf('vxx') > -1) + if (!callsite || + type === 'core' || + callsite.file_name.indexOf('node_modules') > -1 || + callsite.file_name.indexOf('vxx') > -1) continue; // get the whole context (all lines) and cache them if necessary diff --git a/test/interface/aggregator.mocha.js b/test/interface/aggregator.mocha.js index 5d42cf7b..6644adac 100644 --- a/test/interface/aggregator.mocha.js +++ b/test/interface/aggregator.mocha.js @@ -189,17 +189,4 @@ describe('Transactions Aggregator', function() { }); }); - describe('.parseStacktrace', function() { - it('should parse stacktrace and get context', function(done) { - var obj = [{ - labels: { - stacktrace: JSON.stringify(TraceFactory.stacktrace) - } }] - aggregator.parseStacktrace(obj); - obj[0].labels['source/file'].indexOf('test/interface/misc/trace_factory.js:10').should.be.above(0); - should(obj[0].labels['source/context']).eql("var random_routes = [\n '/api/bucket',\n>>'/api/bucket/users',\n '/api/bucket/chameau',\n '/backo/testo'") - done(); - }); - }); - }); diff --git a/test/interface/cache.mocha.js b/test/interface/cache.mocha.js new file mode 100644 index 00000000..7467ae61 --- /dev/null +++ b/test/interface/cache.mocha.js @@ -0,0 +1,61 @@ + +var should = require('should'); +var Utility = require('../../lib/Interactor/Utility.js'); +var path = require('path'); +var fs = require('fs'); + +describe('Cache Utility', function() { + var aggregator; + var stackParser; + var cache; + + it('should instanciate context cache', function() { + cache = new Utility.Cache({ + miss: function (key) { + try { + var content = fs.readFileSync(path.resolve(key)); + return content.toString().split(/\r?\n/); + } catch (err) { + return null; + } + } + }) + }); + + it('should get null without key', function() { + should(cache.get()).be.null(); + }); + + it('should get null with unknow value', function() { + should(cache.get('toto')).be.null(); + }); + + it('should get null', function() { + should(cache.get()).be.null(); + }); + + it('should set null', function() { + should(cache.set()).be.false(); + }); + + it('should not set key without value', function() { + should(cache.set('toto')).be.false(); + }); + + it('should set value', function() { + should(cache.set('toto', 'val')).be.true(); + }); + + it('should get value', function() { + should(cache.get('toto')).eql('val'); + }); + + it('should reset', function() { + cache.reset(); + }); + + it('should get null with unknow value', function() { + should(cache.get('toto')).be.null(); + }); + +}); diff --git a/test/interface/misc/trace_factory.js b/test/interface/misc/trace_factory.js index 1346d437..b2ee85b5 100644 --- a/test/interface/misc/trace_factory.js +++ b/test/interface/misc/trace_factory.js @@ -140,6 +140,7 @@ exports.stacktrace = { ] } + if (require.main === module) { console.log(generateTrace()); } diff --git a/test/interface/stacktrace.mocha.js b/test/interface/stacktrace.mocha.js new file mode 100644 index 00000000..940d3e0d --- /dev/null +++ b/test/interface/stacktrace.mocha.js @@ -0,0 +1,128 @@ + +var should = require('should'); +var Aggregator = require('../../lib/Interactor/TransactionAggregator.js'); +var Utility = require('../../lib/Interactor/Utility.js'); +var TraceFactory = require('./misc/trace_factory.js'); +var path = require('path'); +var fs = require('fs'); + +describe('StackTrace Utility', function() { + var aggregator; + var stackParser; + + it('should instanciate context cache', function() { + var cache = new Utility.Cache({ + miss: function (key) { + try { + var content = fs.readFileSync(path.resolve(key)); + return content.toString().split(/\r?\n/); + } catch (err) { + return undefined; + } + } + }) + + stackParser = new Utility.StackTraceParser({ cache: cache, context: 2}); + }); + + it('should instanciate aggregator', function() { + aggregator = new Aggregator({ stackParser: stackParser}); + }); + + describe('.parseStacktrace', function() { + it('should parse stacktrace and get context', function(done) { + var obj = [{ + labels: { + stacktrace: JSON.stringify(TraceFactory.stacktrace) + } + }]; + + aggregator.parseStacktrace(obj); + obj[0].labels['source/file'].indexOf('test/interface/misc/trace_factory.js:10').should.be.above(0); + should(obj[0].labels['source/context']).eql("var random_routes = [\n '/api/bucket',\n>>'/api/bucket/users',\n '/api/bucket/chameau',\n '/backo/testo'"); + done(); + }); + + it('should handle malformated stacktraces', function() { + aggregator.parseStacktrace([{ + labels: { + stacktrace: JSON.stringify({ + stack_frame: [{ + line_number: 10, + column_number: 10, + method_name: '' + }, { + file_name: 'node_modules/express.js', + column_number: 10, + method_name: '' + }, { + file_name: path.resolve(__dirname, 'trace_factory.js'), + line_number: 10, + column_number: 10, + method_name: '' + }] + }) + } + }]); + }); + + it('should handle malformated stacktrace v1', function() { + aggregator.parseStacktrace([{ + labels: { + stacktrace: JSON.stringify({ + stack_frame: [{ + file_name: 'events.js' + },{ + file_name: 'node_modules/express.js' + },{ + file_name: path.resolve(__dirname, 'trace_factory.js') + }] + }) + } + }]); + }); + + it('should handle malformated stacktrace v2', function() { + aggregator.parseStacktrace([{ + labels: { + stacktrace: JSON.stringify({ + stack_frame: [{ + file_name: 'events.js', + column_number: 10, + method_name: '' + },{ + file_name: 'node_modules/express.js', + column_number: 10, + method_name: '' + },{ + file_name: path.resolve(__dirname, 'trace_factory.js'), + line_number: 10, + column_number: 10, + method_name: '' + }] + }) + } + }]); + }); + + it('should handle malformated stacktrace v3', function() { + aggregator.parseStacktrace([{ + labels: {} + }]); + }); + + it('should handle malformated stacktrace v4', function() { + aggregator.parseStacktrace([{ + }]); + }); + + it('should handle malformated stacktrace v5', function() { + aggregator.parseStacktrace([]); + }); + + it('should handle malformated stacktrace v5', function() { + aggregator.parseStacktrace(); + }); + + }); +}); diff --git a/test/pm2_programmatic_tests.sh b/test/pm2_programmatic_tests.sh index 5a4cb768..aeff1767 100644 --- a/test/pm2_programmatic_tests.sh +++ b/test/pm2_programmatic_tests.sh @@ -105,5 +105,9 @@ mocha --opts ./mocha.opts ./request.mocha.js spec "Protocol communication test" mocha --opts ./mocha.opts ./aggregator.mocha.js spec "Transaction trace aggregator test" +mocha --opts ./mocha.opts ./stacktrace.mocha.js +spec "Stacktrace Utility" +mocha --opts ./mocha.opts ./cache.mocha.js +spec "Cache Utility" mocha --opts ./mocha.opts ./pm2.link.check.mocha.js spec "Transaction option enablement"