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

cleanup todos #395

Merged
merged 1 commit into from
Feb 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

}
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
*/

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

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');

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

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