get host from request instead

aws · May 18, 2021 · 54efc22 · 54efc22
1 parent 287e8d3
commit 54efc22
Show file tree

Hide file tree

Showing 7 changed files with 14 additions and 20 deletions.
diff --git a/packages/core/lib/patchers/http_p.js b/packages/core/lib/patchers/http_p.js
@@ -136,7 +136,7 @@ function enableCapture(module, downstreamXRayEnabled, subsegmentCallback) {
       } else {
         const madeItToDownstream = (e.code !== 'ECONNREFUSED');
 
-        subsegment.addRemoteRequestData(this, null, madeItToDownstream && downstreamXRayEnabled, options);
+        subsegment.addRemoteRequestData(this, null, madeItToDownstream && downstreamXRayEnabled);
         subsegment.close(e);
       }
 
@@ -165,7 +165,7 @@ function enableCapture(module, downstreamXRayEnabled, subsegmentCallback) {
         if (cause)
           subsegment[cause] = true;
 
-        subsegment.addRemoteRequestData(res.req, res, !!downstreamXRayEnabled, options);
+        subsegment.addRemoteRequestData(res.req, res, !!downstreamXRayEnabled);
         subsegment.close();
       });
 

diff --git a/packages/core/lib/segments/attributes/remote_request_data.js b/packages/core/lib/segments/attributes/remote_request_data.js
@@ -7,17 +7,15 @@ var { stripQueryStringFromPath } = require('../../utils');
  * @param {http.ClientRequest|https.ClientRequest} req - The request object from the HTTP/HTTPS call.
  * @param {http.IncomingMessage|https.IncomingMessage} res - The response object from the HTTP/HTTPS call.
  * @param {boolean} downstreamXRayEnabled - when true, adds a "traced": true hint to generated subsegments such that the AWS X-Ray service expects a corresponding segment from the downstream service.
- * @param {object} [options] - Optional HTTP Request options, which will take precedence over header data to populate request metadata.
  */
 
-function RemoteRequestData(req, res, downstreamXRayEnabled, options) {
-  this.init(req, res, downstreamXRayEnabled, options);
+function RemoteRequestData(req, res, downstreamXRayEnabled) {
+  this.init(req, res, downstreamXRayEnabled);
 }
 
-RemoteRequestData.prototype.init = function init(req, res, downstreamXRayEnabled, options) {
-  const useOptionsHost = options && options.hostname;
+RemoteRequestData.prototype.init = function init(req, res, downstreamXRayEnabled) {
   this.request = {
-    url: (req.agent && req.agent.protocol) ? (req.agent.protocol + '//' + (useOptionsHost ? options.hostname : req.getHeader('host')) +  stripQueryStringFromPath(req.path)) : '',
+    url: (req.agent && req.agent.protocol) ? (req.agent.protocol + '//' + (req.host || req.getHeader('host')) +  stripQueryStringFromPath(req.path)) : '',
     method: req.method || '',
   };
 

diff --git a/packages/core/lib/segments/attributes/subsegment.d.ts b/packages/core/lib/segments/attributes/subsegment.d.ts
@@ -30,7 +30,7 @@ declare class Subsegment {
 
   addError(err: Error | string, remote?: boolean): void;
 
-  addRemoteRequestData(req: http.ClientRequest, res: http.IncomingMessage, downstreamXRayEnabled?: boolean, options?: object): void;
+  addRemoteRequestData(req: http.ClientRequest, res: http.IncomingMessage, downstreamXRayEnabled?: boolean): void;
 
   addFaultFlag(): void;
 

diff --git a/packages/core/lib/segments/attributes/subsegment.js b/packages/core/lib/segments/attributes/subsegment.js
@@ -215,11 +215,10 @@ Subsegment.prototype.addError = function addError(err, remote) {
  * @param {http.ClientRequest/https.ClientRequest} req - The request object from the HTTP/HTTPS call.
  * @param {http.IncomingMessage/https.IncomingMessage} res - The response object from the HTTP/HTTPS call.
  * @param {boolean} downstreamXRayEnabled - when true, adds a "traced": true hint to generated subsegments such that the AWS X-Ray service expects a corresponding segment from the downstream service.
- * @param {object} [options] - Optional HTTP Request options, which will take precedence over header data to populate request metadata.
  */
 
-Subsegment.prototype.addRemoteRequestData = function addRemoteRequestData(req, res, downstreamXRayEnabled, options) {
-  this.http = new RemoteRequestData(req, res, downstreamXRayEnabled, options);
+Subsegment.prototype.addRemoteRequestData = function addRemoteRequestData(req, res, downstreamXRayEnabled) {
+  this.http = new RemoteRequestData(req, res, downstreamXRayEnabled);
   if ('traced' in this.http.request) {
     this.traced = this.http.request.traced;
     delete this.http.request.traced;

diff --git a/packages/core/test-d/index.test-d.ts b/packages/core/test-d/index.test-d.ts
@@ -219,7 +219,6 @@ expectType<void>(subsegment.addSqlData({}));
 const clientRequest = new http.ClientRequest('http://localhost');
 expectType<void>(subsegment.addRemoteRequestData(clientRequest, incomingMessage));
 expectType<void>(subsegment.addRemoteRequestData(clientRequest, incomingMessage, true));
-expectType<void>(subsegment.addRemoteRequestData(clientRequest, incomingMessage, false, {}));
 expectType<true | undefined>(subsegment.streamSubsegments());
 expectType<{ [key: string]: any }>(subsegment.toJSON());
 

diff --git a/packages/core/test/unit/patchers/http_p.test.js b/packages/core/test/unit/patchers/http_p.test.js
@@ -282,7 +282,7 @@ describe('HTTP/S', function() {
           capturedHttp.request(httpOptions);
 
           setTimeout(function() {
-            addRemoteDataStub.should.have.been.calledWithExactly(fakeRequest, fakeResponse, false, httpOptions);
+            addRemoteDataStub.should.have.been.calledWithExactly(fakeRequest, fakeResponse, false);
             done();
           }, 50);
         });
@@ -293,7 +293,7 @@ describe('HTTP/S', function() {
           capturedHttp.request(httpOptions);
 
           setTimeout(function() {
-            addRemoteDataStub.should.have.been.calledWithExactly(fakeRequest, fakeResponse, true, httpOptions);
+            addRemoteDataStub.should.have.been.calledWithExactly(fakeRequest, fakeResponse, true);
             done();
           }, 50);
         });

diff --git a/packages/core/test/unit/segments/attributes/remote_request_data.test.js b/packages/core/test/unit/segments/attributes/remote_request_data.test.js
@@ -50,13 +50,11 @@ describe('RemoteRequestData', function() {
         ''
       );
     });
-    it('should use the host from the options', () => {
-      const options = {
-        hostname: 'different-site.com'
-      };
+    it('should use the host from the request object over headers', () => {
+      const requestWithHost = Object.assign(request, { host: 'different-site.com' });
 
       assert.propertyVal(
-        new RemoteRequestData(request, response, true, options).request,
+        new RemoteRequestData(requestWithHost, response, true).request,
         'url',
         'https://different-site.com/path/to/resource'
       );