Skip to content

Tracing: Add lttng support for tracing on Linux. #702

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.

Sign up for GitHub

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

Closed
GlenTiki wants to merge 5 commits into from
Closed

Tracing: Add lttng support for tracing on Linux. #702

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6ab942b
Tracing: Add lttng support for tracing on Linux.
GlenTiki Jan 22, 2015
71c0ab7
Fixing nits
GlenTiki Feb 6, 2015
28c2e2e
fix nits for review
GlenTiki Feb 6, 2015
b8a72b5
fix missing cast nit
GlenTiki Feb 6, 2015
fde51f1
fix missing cast nit
GlenTiki Feb 7, 2015
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
Prev Previous commit
Next Next commit
Fixing nits
  • Loading branch information
@GlenTiki
GlenTiki committed Feb 6, 2015
commit 71c0ab7c600b0138ede66380cb2c58723cb84a7b
10 changes: 6 additions & 4 deletions src/node_lttng.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,10 @@ void LTTNG_HTTP_SERVER_REQUEST(const FunctionCallbackInfo<Value>& args) {
if (!NODE_HTTP_SERVER_REQUEST_ENABLED())
return;

if (!args[0]->IsObject())
return;

Environment* env = Environment::GetCurrent(args);
HandleScope scope(env->isolate());
Local<Object> arg0 = Local<Object>::Cast(args[0]);
Copy link
Copy Markdown
Member

@bnoordhuis bnoordhuis Feb 5, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caveat emptor: Cast() does the wrong thing when someone does this.connection = null. There should be a if (!args[0]->IsObject()) return guard a few lines up. You probably want to do it after the NODE_HTTP_SERVER_REQUEST_ENABLED check because that check is presumably cheaper than calling args[0]->IsObject().

Side note: you don't need the HandleScope. JS -> C++ callbacks have an implicit HandleScope.

Local<Object> headers;

Expand All @@ -155,8 +157,9 @@ void LTTNG_HTTP_SERVER_REQUEST(const FunctionCallbackInfo<Value>& args) {

Local<Value> strfwdfor = headers->Get(env->x_forwarded_string());
node::Utf8Value fwdfor(env->isolate(), strfwdfor);
req.forwardedFor = *fwdfor;

if (!strfwdfor->IsString() || (req.forwardedFor = *fwdfor) == nullptr)
if (!strfwdfor->IsString() || req.forwardedFor == nullptr)
req.forwardedFor = const_cast<char*>("");
Copy link
Copy Markdown
Member

@bnoordhuis bnoordhuis Feb 5, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you make forwardedFor const?

Style note: io.js uses snake_case for variables and members, not camelCase.

Copy link
Copy Markdown
Contributor Author

@GlenTiki GlenTiki Feb 6, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I took this code from here, node_dtrace.cc and it is nearly the exact same - can you advise on the change you want to see?

Copy link
Copy Markdown
Member

@bnoordhuis bnoordhuis Feb 6, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The struct member should (and can) be const char* because you're not mutating it after the fact.

Rule of thumb: if something can be const, it probably should.


SLURP_CONNECTION(args[1], conn);
Expand All @@ -176,13 +179,12 @@ void LTTNG_HTTP_SERVER_RESPONSE(const FunctionCallbackInfo<Value>& args) {

void LTTNG_HTTP_CLIENT_REQUEST(const FunctionCallbackInfo<Value>& args) {
node_lttng_http_client_request_t req;
char *header;
char* header;

if (!NODE_HTTP_CLIENT_REQUEST_ENABLED())
return;

Environment* env = Environment::GetCurrent(args);
HandleScope scope(env->isolate());

/*
* For the method and URL, we're going to dig them out of the header. This
Expand Down
64 changes: 32 additions & 32 deletions src/node_lttng_provider.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,56 +8,58 @@
namespace node {

void NODE_HTTP_SERVER_REQUEST(node_lttng_http_server_request_t* req,
node_lttng_connection_t* conn, const char *remote, int port,
const char *method, const char *url, int fd) {
tracepoint(node, http_server_request,
req->url, req->method, req->forwardedFor);
node_lttng_connection_t* conn,
const char *remote, int port,
const char *method, const char *url,
int fd) {
tracepoint(node, http_server_request, req->url, req->method, \
req->forwardedFor);
}

void NODE_HTTP_SERVER_RESPONSE(node_lttng_connection_t* conn,
const char *remote, int port, int fd) {
const char *remote, int port, int fd) {
tracepoint(node, http_server_response, port, conn->remote, fd);
}

void NODE_HTTP_CLIENT_REQUEST(node_lttng_http_client_request_t* req,
node_lttng_connection_t* conn, const char *remote, int port,
const char *method, const char *url, int fd) {
node_lttng_connection_t* conn,
const char *remote, int port,
const char *method, const char *url,
int fd) {
tracepoint(node, http_client_request, req->url, req->method);
}

void NODE_HTTP_CLIENT_RESPONSE(node_lttng_connection_t* conn,
const char *remote, int port, int fd) {
const char *remote, int port, int fd) {
tracepoint(node, http_client_response, port, conn->remote, fd);
}

void NODE_NET_SERVER_CONNECTION(node_lttng_connection_t* conn,
const char *remote, int port, int fd) {
tracepoint(node, net_server_connection,
conn->remote, port, fd, conn->buffered);
const char *remote, int port, int fd) {
tracepoint(node, net_server_connection, conn->remote, port, fd, \
conn->buffered);
}

void NODE_NET_STREAM_END(node_lttng_connection_t* conn,
const char *remote, int port, int fd) {
const char *remote, int port, int fd) {
tracepoint(node, net_stream_end, conn->remote, port, fd);
}

void NODE_GC_START(v8::GCType type,
v8::GCCallbackFlags flags,
v8::Isolate* isolate) {
int opt1 = 1 << 0;
int opt2 = 1 << 1;
char* typeStr = "";
char* flagsStr = "";
if (type == opt1) {
typeStr = "kGCTypeScavenge";
} else if (type == opt2) {
const char* typeStr = "";
const char* flagsStr = "";
if (type == v8::GCType::kGCTypeScavenge) {
typeStr = "kGCTypeScavenge";
} else if (type == v8::GCType::kGCTypeMarkSweepCompact) {
typeStr = "kGCTypeMarkSweepCompact";
} else {
} else if (type == v8::GCType::kGCTypeAll) {
typeStr = "kGCTypeAll";
}
if (flags == opt1) {
if (flags == v8::GCCallbackFlags::kNoGCCallbackFlags) {
flagsStr = "kNoGCCallbackFlags";
} else {
} else if (flags == v8::GCCallbackFlags::kGCCallbackFlagCompacted) {
flagsStr = "kGCCallbackFlagCompacted";
}

Expand All @@ -68,20 +70,18 @@ void NODE_GC_START(v8::GCType type,
void NODE_GC_DONE(v8::GCType type,
v8::GCCallbackFlags flags,
v8::Isolate* isolate) {
int opt1 = 1 << 0;
int opt2 = 1 << 1;
char* typeStr = "";
char* flagsStr = "";
if (type == opt1) {
typeStr = "kGCTypeScavenge";
} else if (type == opt2) {
const char* typeStr = "";
const char* flagsStr = "";
if (type == v8::GCType::kGCTypeScavenge) {
typeStr = "kGCTypeScavenge";
} else if (type == v8::GCType::kGCTypeMarkSweepCompact) {
typeStr = "kGCTypeMarkSweepCompact";
} else {
} else if (type == v8::GCType::kGCTypeAll) {
typeStr = "kGCTypeAll";
}
if (flags == opt1) {
if (flags == v8::GCCallbackFlags::kNoGCCallbackFlags) {
flagsStr = "kNoGCCallbackFlags";
} else {
} else if (flags == v8::GCCallbackFlags::kGCCallbackFlagCompacted) {
flagsStr = "kGCCallbackFlagCompacted";
}

Expand Down
8 changes: 4 additions & 4 deletions src/node_lttng_tp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ TRACEPOINT_EVENT(
node,
gc_start,
TP_ARGS(
char*, gctype,
char*, gcflags
const char*, gctype,
const char*, gcflags
),
TP_FIELDS(
ctf_string(gctype, gctype)
Expand All @@ -117,8 +117,8 @@ TRACEPOINT_EVENT(
node,
gc_done,
TP_ARGS(
char*, gctype,
char*, gcflags
const char*, gctype,
const char*, gcflags
),
TP_FIELDS(
ctf_string(gctype, gctype)
Expand Down
5 changes: 5 additions & 0 deletions test/sequential/test-util-debug.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ function test(environ, shouldWrite) {

var spawn = require('child_process').spawn;
var child = spawn(process.execPath, [__filename, 'child'], {
// Lttng requires the HOME env variable or it prints to stderr,
// This is not really ideal, as it breaks this test, so the HOME
// env variable is passed to the child to make the test pass.
// this is fixed in the next version of lttng (2.7+), so we can
// remove it at sometime in the future.
env: { NODE_DEBUG: environ, HOME: process.env.HOME }
});

Expand Down