[trace] How to add a trace point?

[hhellyer]

(My apologies if this is the wrong place to ask, I know the trace work is still ongoing but I was trying it out and hit an issue so it seemed best to raise it here. @ofrobots - you might be the right person to ask!)

I was looking at how the v8 trace mechanism is intended to be used from Node.js by trying to add a simple trace point to Start() in node.cc and running node --trace-events-enabled helloworld.js

The trace point I added was just to show node had started with:

TRACE_EVENT_INSTANT0("node", "node::Start", TRACE_EVENT_SCOPE_PROCESS);

This produced a lot of trace points for v8 but the new trace point was missing. I ran with node --trace-events-enabled --trace-event-categories node helloworld.js and got no output at all.

After a bit of investigation I found that when node reached the v8::platform::DefaultPlatform class it was calling a version of AddTraceEvent that had not been overridden from the base v8::platform::Platform class and had an empty implementation. Implementing that call (by copying the implemented version of AddTraceEvent) caused my trace point to start appearing.

I’m trying to understand if I did something wrong by just calling TRACE_EVENT_INSTANT0 and if so what I should have done or if there’s a missing piece of code in v8 that the Node.js trace implementation is going to require. (I think the v8 trace points take the other path using the other version of AddTraceEvent.)

I’ve put my demonstration code up on a branch if anyone has a chance to look:
nodejs/node@master...hhellyer:trace_bug

[matthewloring]

The root cause of this issue was that the trace event macros found in src/tracing/trace_event.h fell out of date with the version of V8 in Node. The problem was not detected because there are not yet any uses of the macros in core. I have a PR ready that will bring the macros back up to date that I can open as soon as v8 5.7 lands (nodejs/node#11752). Once we get some trace points added to node core we can add tests to catch these issues when v8 is upgraded. Longer term, we're looking into ways to avoid copying these macros from V8 so that they do not need to be upgraded independently. Sorry for the confusion.

[hhellyer]

np - My main concern was that if it was a problem in v8 it would take a (relatively) long time to round trip any fix back to Node.js so better to mention sooner rather than later. I'll close this issue. Thanks!

[joshgav]

@matthewloring will you land the fix soon now that 5.7 has landed in master? Do you know what the last known good version of Node would be? Thanks!

[matthewloring]

I'm hoping to open the fix today or tomorrow. I'm not exactly sure about the last known good version. This isn't an observable issue unless you try to manually add your own trace points which were not present in old versions of node.