Repository: honeycombio/beeline-nodejs
Pull request #210 (2020-06-15 22:45:21)

http(s) instrumentation should output urls with path/pathname/query correctly
If you call `axios.get("https://google.com/search?q=honeycomb.io")` then honeycomb will record it as `https://google.com/search%3Fq=honeycomb.io?q=honeycomb.io`.

axios parses the url and calls http.get with `{ path, pathname, search }`. The http/https instrumentation should respect the incoming pathname if it exists.

toshok: let me take a crack at fixing the conflicts

Patch:
diff --git a/lib/instrumentation/http.js b/lib/instrumentation/http.js
index 7d3cb08..367c563 100644
--- a/lib/instrumentation/http.js
+++ b/lib/instrumentation/http.js
@@ -50,7 +50,7 @@ let instrumentHTTP = (http, opts = {}) => {
       // make sure options is populated enough for url.format to give us reasonable results.  these are all defaults from nodejs docs.
       combinedOptions.protocol = combinedOptions.protocol || "http:";
       combinedOptions.port = combinedOptions.port !== 80 ? combinedOptions.port : null;
-      combinedOptions.pathname = combinedOptions.path || "/";
+      combinedOptions.pathname = combinedOptions.pathname || combinedOptions.path || "/";
       if (!combinedOptions.hostname && !combinedOptions.host) {
         combinedOptions.hostname = "localhost";
       }
diff --git a/lib/instrumentation/https.js b/lib/instrumentation/https.js
index eb0e0e0..ee9da7d 100644
--- a/lib/instrumentation/https.js
+++ b/lib/instrumentation/https.js
@@ -51,7 +51,7 @@ let instrumentHTTPS = (https, opts = {}) => {
       // make sure options is populated enough for url.format to give us reasonable results.  these are all defaults from nodejs docs.
       combinedOptions.protocol = combinedOptions.protocol || "https:";
       combinedOptions.port = combinedOptions.port !== 443 ? combinedOptions.port : null;
-      combinedOptions.pathname = combinedOptions.path || "/";
+      combinedOptions.pathname = combinedOptions.pathname || combinedOptions.path || "/";
       if (!combinedOptions.hostname && !combinedOptions.host) {
         combinedOptions.hostname = "localhost";
       }


Test patch:
diff --git a/lib/instrumentation/http.test.js b/lib/instrumentation/http.test.js
index f6efeb8..6255b84 100644
--- a/lib/instrumentation/http.test.js
+++ b/lib/instrumentation/http.test.js
@@ -82,6 +82,33 @@ test("url as options", (done) => {
   );
 });
 
+test("url as options with pathname and query", done => {
+  tracker.setTracked(newMockContext());
+
+  http.get(
+    {
+      hostname: "localhost",
+      port: 9009,
+      path: "/test?something=true",
+      pathname: "/test",
+      search: "?something=true",
+    },
+    res => {
+      expect(res.headers[api.TRACE_HTTP_HEADER.toLowerCase()]).toBe(
+        "1;trace_id=0,parent_id=51001,context=e30="
+      );
+      expect(api._apiForTesting().sentEvents).toEqual([
+        expect.objectContaining({
+          [schema.EVENT_TYPE]: "http",
+          name: "GET",
+          url: "http://localhost:9009/test?something=true",
+        }),
+      ]);
+      done();
+    }
+  );
+});
+
 test("correct response context", (done) => {
   tracker.setTracked(newMockContext());
 
diff --git a/lib/instrumentation/https.test.js b/lib/instrumentation/https.test.js
index 5b85512..2619dab 100644
--- a/lib/instrumentation/https.test.js
+++ b/lib/instrumentation/https.test.js
@@ -89,6 +89,44 @@ test("url as options", (done) => {
   );
 });
 
+test("url as options with pathname and query", done => {
+  tracker.setTracked(newMockContext());
+
+  https.get(
+    {
+      hostname: "google.com",
+      path: "/search?q=honeycomb.io",
+      pathname: "/search",
+      search: "?q=honeycomb.io",
+    },
+    _res => {
+      expect(api._apiForTesting().sentEvents).toMatchObject(
+        semver.lt(process.version, "9.0.0")
+          ? [
+              {
+                [schema.EVENT_TYPE]: "http",
+                [schema.TRACE_SPAN_NAME]: "GET",
+                url: "http://google.com:443/search?q=honeycomb.io",
+              },
+              {
+                [schema.EVENT_TYPE]: "https",
+                [schema.TRACE_SPAN_NAME]: "GET",
+                url: "https://google.com/search?q=honeycomb.io",
+              },
+            ]
+          : [
+              {
+                [schema.EVENT_TYPE]: "https",
+                [schema.TRACE_SPAN_NAME]: "GET",
+                url: "https://google.com/search?q=honeycomb.io",
+              },
+            ]
+      );
+      done();
+    }
+  );
+});
+
 test("correct response context", (done) => {
   tracker.setTracked(newMockContext());