src: patch https in Node 8.9 and 9.0 #591
Conversation
googlebot
added
the
cla: yes
| @@ -164,6 +164,7 @@ export const defaultConfig = { | |||
| 'grpc': path.join(__dirname, 'src/plugins/plugin-grpc.js'), | |||
| 'hapi': path.join(__dirname, 'src/plugins/plugin-hapi.js'), | |||
| 'http': path.join(__dirname, 'src/plugins/plugin-http.js'), | |||
| 'https': path.join(__dirname, 'src/plugins/plugin-https.js'), | |||
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
@ofrobots @GoogleCloudPlatform/node-team PTAL when AppVeyor CI is green. |
src/plugins/plugin-https.ts
Outdated
|
|
||
| module.exports = []; | ||
|
|
||
| export default {}; |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/plugins/plugin-https.ts
Outdated
| // https depends on http anyway, so this shouldn't cause an unnecessary load. | ||
| require('http'); | ||
|
|
||
| module.exports = []; |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| !!options.headers[api.constants.TRACE_AGENT_REQUEST_HEADER]; | ||
| } | ||
|
|
||
| function makeRequestTrace(request, api) { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
kjin
changed the title
src/plugins/plugin-http.ts
Outdated
| return request.apply(this, arguments); | ||
| } | ||
| function patchHttps(https, api) { | ||
| console.log('patch https'); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/plugins/plugin-http.ts
Outdated
| shimmer.wrap(https, 'request', function requestWrap(request) { | ||
| return makeRequestTrace(request, api); | ||
| }); | ||
| shimmer.wrap(https, 'get', function requestWrap(get) { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Fixes #589
Loading either
httporhttpswill cause both to be patched for tracing. This is in line with behavior in <8.9, ashttps.requestused to depend onhttp.request.The only new user-facing change is in the addition of the
httpsconfig field: disabling causes the specific behavior of not patchinghttpsonly if it's loaded beforehttpin user-space code. It's a niche use-case so I think this is justifiable, but please let me know your thoughts.By nature of our loader system this is uncharted territory, so feel free to give suggestions if you think there's a better approach.