net: Supply host argument in the Socket lookup event callback.

[entertainyou]

Pull Request check-list

Please make sure to review and check all of these items:

• Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
  this change (including linting)?
• Is the commit message formatted according to [CONTRIBUTING.md][0]?
• If this change fixes a bug (or a performance problem), is a regression
  test (or a benchmark) included?
• Is a documentation update included (if this change modifies
  existing APIs, or introduces new ones)?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Affected core subsystem(s)

net

Description of change

Supply host argument in Socket lookup event callback

Also removes _host field in Socket.

[entertainyou]

Reverted the _host remove part, as _host field is used in lib/_tls_wrap.js.

Remove _host makes test/internet/test-tls-reuse-host-from-socket.js fail, but make test does not run that testcase.

[cjihrig]

Just out of curiosity, what is your use case for this?

[entertainyou]

@cjihrig I want to record my app dns resolve time metric(the host should be part of it).

[cjihrig]

Do you not already have access to that data when you setup the socket connection?

[entertainyou]

@cjihrig I did not use Socket directly, I use the request library, following is a sample code:

var request = require('request');

var req = request('http://httpbin.org/redirect-to?url=http%3A%2F%2Fexample.com%2F', (res) => {
    console.log('Got response', res);
}).on('socket', socket => {
    socket.on('lookup', (err, ip, type) => {
    console.log('LOOKUP', err, ip);
    });
});

The url requested may have redirects, and I want to record each dns resolution(NOTE that I can not control the url requested).

[cjihrig]

I'd like to hear if anyone else has opinions on this. The code changes LGTM though.

[evanlucas]

with the addition of being able to specify a lookup function in net.createConnection, this would also need to update those docs for the function signature. I'm on the fence here. Is this not something that could be done with using a custom lookup function?

[entertainyou]

@evanlucas , I don't quite understand, why need to update lookup function signature? The host variable is which we passed to the lookup function, this patch does not modify lookup related code.

[jasnell]

@cjihrig ... this LGTM but I don't think it's a high priority.

[evanlucas]

@entertainyou ah disregard my previous comment. Sorry about that.

LGTM

[entertainyou]

ping, @evanlucas , it's this PR good to be merge?

[r-52]

CI https://ci.nodejs.org/job/node-test-pull-request/1954/

[evanlucas]

One of the build bots went offline. New CI: https://ci.nodejs.org/job/node-test-pull-request/1955/

I'll land it today if the CI is happy. Thanks!

[evanlucas]

Landed in 545b8fd. Thanks for the contribution!