src: fix build with --with-lttng

[simark]

Building with --with-lttng hits a few issues.

• There are missing parentheses in node_lttng_tp.h since

  src: lint node_lttng_tp.h
  f1d2792

  This patch adds them.

• The arraysize function in node_lttng.cc is not found. I included
  node_internals.h and added the node namespace (node::arraysize).

• The -llttng-ust flag is passed when building the static library libnode.a,
  but it's actually when linking a final executable that it's required.
  I see that node can be built as a shared library as well, so maybe the
  flag is relevant in that case. But in the case where the intermediate
  library is a static one, it doesn't work. I am not familiar with
  node's build system, so I did not fix this one, I am fine with
  changing the generated Makefile by hand for now. But if somebody
  wants to take a look, it's of course very appreciated :).

[addaleax]

@simark Is there some way in which the functionality of these things can be tested automatically? Node comes with support for a couple tracing systems, but I don’t think core contributors are actively making sure that they function. At least in my case, I’m not sure how exactly they are actually used in practice.

/cc @nodejs/diagnostics

[simark]

Is there a CI system that checks regression in the node code regularly? If so, you could make sure to have --with-lttng when building. On common distros, it only requires a few packages to be installed (e.g. liblttng-ust-dev and maybe others on Debian).

[addaleax]

@simark Yes. 😄 And we do actually do that kind of thing for some build flags – but that would still only make sure that things compile, right?

[simark]

@simark Yes. smile And we do actually do that kind of thing for some build flags – but that would still only make sure that things compile, right?

Exactly, but it's already better than nothing!

It would be easy enough to make a test that would look like:

1. lttng create && lttng enable-event -u -a && lttng start
2. Run node
3. lttng stop
4. Check that the command lttng view | grep <some event name we know should be there> works
5. lttng destroy

[AndreasMadsen]

@simark The diagnostic WG have discussed reworking our tracing point implementation. Could you comment in nodejs/diagnostics#163 what your use case is?

The missing tests is a problem we have discussed and would like to solve in our refactor.

[GlenTiki]

No one has touched these in nearly 2 years, they've been broken for a while.. I was about to open a PR to deprecate them. Good to see someone using it! 😆

[BridgeAR]

If I am not mistaken this is now obsolete and should be closed? Ping @thekemkid

[GlenTiki]

Yes, we merged the PR to remove lttng support yesterday. I'm closing this.