Closed ghost closed 12 years ago
I'm picking this one up.
To elaborate a bit on what Keith explained:
The crux of the issue is that my original change added a field to the server probes' structures but not the client probes' structures. As a result, the C type that gets passed into these probes was split from node_dtrace_http_request_t into node_dtrace_http_client_request_t and node_dtrace_http_server_request_t. However, the names of the corresponding structures in the library file (node.d) were not changed, so when dtrace(1M) goes to translate a script that looks like this:
node57257:::http-client-request { trace(args[0]->url); }
the binary is providing node_dtrace_http_client_request_t, but dtrace(1M) doesn't know anything about that (because it's not in the node.d library file), and it spits out:
dtrace: invalid probe specifier node57257:::http-client-request { trace(args[0]->url); }: in action list: failed to resolve native type for args[0]
The suggested fix is to split the structures in the provider file and define four translators, one for each of the following cases:
The one consolation is that the only changes necessary are to the provider file, which can be overridden with the "-L" argument to dtrace(1M). Existing binaries will continue to work.
The test plan must include testing both client and server probes using both translated arguments without using "xlate", for each of:
The changes to the provider file will break existing D scripts. It would be possible to preserve compatibility with old scripts by defining translators from each of server_request and client_request to request_t, but It's unlikely that anyone is using this except Joyent's own Cloud Analytics, which will deliver the new provider file with a new version of the software that generates updated scripts.
I forgot to mention that the workaround is to use the xlate operator directly, casting the argument to the appropriate type so that dtrace(1M) doesn't need to know what kind it is. This explains why Cloud Analytics works fine after this change, and may explain why I didn't see this during testing.
This was a bit more subtle than we'd realized in a few ways:
A working fix is available here: https://github.com/davepacheco/node/commit/4a92541710af150795afcee271fa25457136783f
but it's still waiting for review.
Here are the scripts I used to test the fix: https://gist.github.com/1d99633ce16952cd4267
and here's what I tested:
Node v0.4.10:
Node v0.6.8:
Both:
All of the FAILs were because of an unrelated issue with the HTTP client probes, which I verified already existed (and also shouldn't be affected by my change). All of my tests were 32-bit, as I'm not sure 64-bit SmartOS is supported yet. (I also didn't change anything that should affect this.)
I managed to test both the client probes and that 64-bit (mostly) works using a fake node binary that just fires these probes with known valid input. This is available at https://github.com/davepacheco/fakenode.
Final fix is https://github.com/davepacheco/node/commit/815419e9d23779c5e7efbec903fab035638a8337 (same as previous but with minor style fixes).
I hit the same issue with node 0.8.9, and it was due to another reason: I had no /usr/lib/dtrace/node.d library on my SmartOS. While some SmartOS versions may have it included, others do not. If you miss the file, download latest version of node.d from https://raw.github.com/joyent/node/master/src/node.d, put it in /var/lib/dtrace/ and run dtrace using -L option: dtrace -L /var/lib/dtrace/ -n 'node*:::http-client-request { trace(args[0]->url); }'
Following the change in e142fe2b, the node provider declares two separate types for HTTP client and server requests. However, the node.d library contains only a single translator, for the now obsolete
node_dtrace_http_request_t
type. This allows it to work correctly with older node binaries, but will generate errors of the formwhen (and only when) one of the fields of a relevant translated structure is referenced in an enabling within a newer node binary.
The correct fix is to have four translators in the library: from the obsolete
node_dtrace_http_request_t
to each of the newnode_http_request_{client,server}_t
, and two "identity" translators fromnode_dtrace_http_{client,server}_request_t
tonode_http_{client_server}_request_t
. This will also allow for future changes to the format of the internal client structure.