Change code to use url.URL instead of url.parse

[watson]

url.parse is a deprecated core API. We should update our code to use url.URL instead.

[yuxizhe]

@watson I'm happy to take this issue.

[watson]

@yuxizhe that would be really helpful. But it might be a bit tricky though... before I opened this issue, I checked if I could easily do it myself. But it's unfortunately not a 1-to-1 replacement due to the differences in how the two functions behave. But we can discuss how to best implement it if you still want to take it on

[yuxizhe]

@watson Yes, I reread the docs https://nodejs.org/api/url.html#url_url_strings_and_url_objects.

I search in this repo find there are a few places where we use url.parse, most of them are just use .pathname propertie. Because URL and UrlObject both have .pathname propertie, so I think in these places they can be easily replaced.

Except for the code in

apm-agent-nodejs/lib/instrumentation/http-shared.js

Lines 83 to 95 in 248c945

	// NOTE: This will also stringify and parse URL instances
	// to a format which can be mixed into the options object.
	function ensureUrl (v) {
	if (typeof v === 'string') {
	return url.parse(v)
	} else if (v instanceof url.Url) {
	return v
	} else if (url.URL && v instanceof url.URL) { // check for url.URL because it wasn't added until Node.js 6.13.0
	return url.parse(v.toString())
	} else {
	return v
	}
	}

apm-agent-nodejs/lib/instrumentation/http-shared.js

Line 116 in 248c945

Object.assign(options, ensureUrl(arg))

Object.assign(options, URL) mix options with URL in a different format

{ [Symbol(context)]:
   URLContext {
     flags: 912,
     scheme: 'https:',
     username: '',
     password: '',
     host: 'aaa.com',
     port: null,
     path: [ 'a', 'b' ],
     query: 'a=1',
     fragment: null },
  [Symbol(query)]: URLSearchParams { 'a' => '1' } }

So in this place I won't simply replace it with URL,
what about the code below:

function formatURL (item) {
  return {
    href: item.href,
    pathname: item.pathname,
    protocol: item.protocol,
    host: item.host,
    port: item.port,
    hostname: item.hostname,
    hash: item.hash,
    search: item.search,
  }
}
// NOTE: This will also stringify and parse URL instances
// to a format which can be mixed into the options object.
function ensureUrl (v) {
  if (url.URL && (v instanceof url.URL || typeof v === 'string')) {
    const urlItem = new URL(v.toString())
    return formatURL(urlItem)
  } else if (typeof v === 'string') {
    return url.parse(v)
  } else {
    return v
  }
}

[yuxizhe]

Oh, there is another problem. Relative URLs in WHATWG URL API nodejs/node#12682

I prefer to use this one new URL('/a/b', 'relative:///');

[watson]

Oh nice, I didn't know you could put in relative:/// - I just thought we would put in http://localhost or something like that. But I like relative:/// more 👍

[watson]

I think your approach in the code you posted above is good. If you can throw together a PR we can take it from there 😃