Skip to content

Commit

Permalink
cleanup todos (#395)
Browse files Browse the repository at this point in the history
PR-URL: #395
  • Loading branch information
matthewloring authored Feb 24, 2017
1 parent a8a6e58 commit f5662e0
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 10 deletions.
5 changes: 3 additions & 2 deletions src/plugins/plugin-redis.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,13 @@ function startSpanFromArguments(api, cmd, args, cb, send_command) {

function createInternalSendCommandWrap(api) {
return function internalSendCommandWrap(internal_send_command) {
// TODO: Document and simplify this code.
return function internal_send_command_trace(cmd, args, cb) {
if (arguments.length === 1 && typeof cmd === 'object') {
// New versions of redis (2.4+) use a single options object instead
// of separate named arguments.
var span = setupSpan(api, cmd.command, cmd.args, 0);
if (!span) {
return internal_send_command.call(this, cmd, args, cb);
return internal_send_command.call(this, cmd);
}
cmd.callback = wrapCallback(api, span, cmd.callback);
return internal_send_command.call(this, cmd);
Expand Down
4 changes: 1 addition & 3 deletions src/plugins/plugin-restify.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ function patchRestify(restify, api) {
skipFrames: 3
};

// TODO(ofrobots): can this be moved to the inner scope near the req.end
// clobber?
var originalEnd = res.end;
api.runInRootSpan(options, function(rootSpan) {
if (!rootSpan) {
return next();
Expand All @@ -67,6 +64,7 @@ function patchRestify(restify, api) {
rootSpan.addLabel(api.labels.HTTP_SOURCE_IP,
req.connection.remoteAddress);

var originalEnd = res.end;
res.end = function(chunk, encoding) {
res.end = originalEnd;
var returned = res.end(chunk, encoding);
Expand Down
1 change: 0 additions & 1 deletion src/trace-plugin-interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ var nullSpan = {
/**
* PluginAPI constructor. Don't call directly - a plugin object will be passed to
* plugin themselves
* TODO(kjin): Should be called something else
*/
function PluginAPI(agent) {
this.agent_ = agent;
Expand Down
4 changes: 0 additions & 4 deletions test/test-trace-agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,13 @@ var assert = require('assert');
var config = require('../config.js');
var file = require('../src/trace-agent.js');
var SpanData = require('../src/span-data.js');
// TODO: This line will silently fail if a logger is not supplied as
// the second argument.
var agent = file.get(config, emptyLogger);
var constants = require('../src/constants.js');
var cls = require('../src/cls.js');

describe('Trace Agent', function() {

it('should return the same object on repeated application', function() {
// TODO: This line will silently fail if a logger is not supplied as
// the second argument.
var agent1 = file.get(config, emptyLogger);
var agent2 = file.get(config, emptyLogger);
assert.strictEqual(agent1, agent2);
Expand Down
2 changes: 2 additions & 0 deletions test/test-trace-header-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ describe('test-trace-header-context', function() {
});

afterEach(function() {
// On node 0.12, mocha may run multiple tests in the same
// cls context, we need to manually clean out the context.
common.clearNamespace(agent);
});

Expand Down

0 comments on commit f5662e0

Please sign in to comment.