SNOW-1801434 add GUID to request in node.js driver

index.d.ts

@@ -615,6 +615,14 @@ declare module 'snowflake-sdk' {
          */
         requestId?: string;
 
+        /**
+         * The request GUID is a unique identifier of an HTTP request issued to Snowflake.
+         * Unlike the requestId, it is regenerated even when the request is resend with the retry mechanism.
+         * If not specified, request GUIDs are attached to all requests to Snowflake for better traceability.
+         * In the majority of cases it should not be set or filled with false value.
+         */
+        excludeGuid?: string;
+
         /**
          * Use different rest endpoints based on whether the query id is available.
          */

lib/constants/sf_params.js

@@ -0,0 +1,2 @@
+exports.paramsNames = {};
+exports.paramsNames.SF_REQUEST_GUID = 'request_guid';

lib/http/request_util.js

@@ -1,3 +1,4 @@
+const sfParams = require('../constants/sf_params');
 
 /**
  * Should work with not yet parsed options as well (before calling prepareRequestOptions method).
@@ -6,7 +7,7 @@
  * @returns {string}
  */
 exports.describeRequestFromOptions = function (requestOptions) {
-  return exports.describeRequestData({ method: requestOptions.method, url: requestOptions.url });
+  return exports.describeRequestData({ method: requestOptions.method, url: requestOptions.url, guid: requestOptions.params?.[sfParams.paramsNames.SF_REQUEST_GUID] });
 };
 
 /**
@@ -16,11 +17,21 @@ exports.describeRequestFromOptions = function (requestOptions) {
  * @returns {string}
  */
 exports.describeRequestFromResponse = function (response) {
-  // Uppercase is used to allow finding logs corresponding to the same request - response pair,
-  // by matching identical options' descriptions.
-  return exports.describeRequestData({ method: response.config?.method.toUpperCase(), url: response.config?.url });
+  let methodUppercase = undefined;
+  let url = undefined;
+  let guid = undefined;
+  const responseConfig = response.config;
+  if (responseConfig) {
+    // Uppercase is used to allow finding logs corresponding to the same request - response pair,
+    // by matching identical options' descriptions.
+    methodUppercase = responseConfig.method.toUpperCase();
+    url = responseConfig.url;
+    guid = responseConfig.params?.[sfParams.paramsNames.SF_REQUEST_GUID];
+  }
+  return exports.describeRequestData({ method: methodUppercase, url: url, guid: guid });
 };
 
-exports.describeRequestData = function (requestData) {
-  return `[method: ${requestData.method}, url: ${requestData.url}]`;
+exports.describeRequestData = function ({ method, url, guid } = {}) {
+  const guidDescription = guid ? `, guid: ${guid}` : '';
+  return `[method: ${method}, url: ${url}` + guidDescription + ']';
 };

lib/services/sf.js

@@ -60,6 +60,7 @@ const AuthenticationTypes  = require('../authentication/authentication_types');
 const AuthOkta = require('../authentication/auth_okta');
 const AuthKeypair = require('../authentication/auth_keypair');
 const Authenticator = require('../authentication/authentication');
+const sfParams = require('../constants/sf_params');
 
 function isRetryableNetworkError(err) {
   // anything other than REVOKED error can be retryable.
@@ -578,17 +579,24 @@ function StateAbstract(options) {
    *
    * @param {Object} requestOptions
    * @param {Object} httpClient
-   *
+   * @param {Object} auth
    * @returns {Object} the http request object.
    */
   function sendHttpRequest(requestOptions, httpClient, auth) {
+
+    const params = requestOptions.params || {};
+    if (!requestOptions.excludeGuid) {
+      addGuidToParams(params);
+    }
+
     const realRequestOptions =
       {
         method: requestOptions.method,
         headers: requestOptions.headers,
         url: requestOptions.absoluteUrl,
         gzip: requestOptions.gzip,
         json: requestOptions.json,
+        params: params,
         callback: async function (err, response, body) {
           // if we got an error, wrap it into a network error
           if (err) {
@@ -679,8 +687,8 @@ function StateAbstract(options) {
       };
 
     if (requestOptions.retry > 2) {
-      const includeParam = requestOptions.url.includes('?');
-      realRequestOptions.url += (includeParam ? '&' : '?');
+      const includesParam = requestOptions.url.includes('?');
+      realRequestOptions.url += (includesParam ? '&' : '?');
       realRequestOptions.url +=
         ('clientStartTime=' + requestOptions.startTime
           + '&' + 'retryCount=' + (requestOptions.retry - 1));
@@ -703,7 +711,7 @@ function StateAbstract(options) {
   ///////////////////////////////////////////////////////////////////////////
 
   /**
-   * Creates a new Request.
+   * Creates a new Request to Snowflake.
    *
    * @param {Object} requestOptions
    * @constructor
@@ -721,18 +729,31 @@ function StateAbstract(options) {
     // pre-process the request options
     this.preprocessOptions(this.requestOptions);
 
+    const params = this.requestOptions.params || {};
+    if (!this.requestOptions.excludeGuid) {
+      addGuidToParams(params);
+    }
     const options =
     {
       method: this.requestOptions.method,
       headers: this.requestOptions.headers,
       url: this.requestOptions.absoluteUrl,
-      json: this.requestOptions.json
+      json: this.requestOptions.json,
+      params: params
     };
 
     // issue the async http request
+    //TODO: this should be wrapped with the same operations, as in the synchronous send method's callback.
     return await httpClient.requestAsync(options);
   };
 
+  function addGuidToParams(params) {
+    // In case of repeated requests for the same request ID,
+    // the Global UID is added for better traceability.
+    const guid = uuidv4();
+    params[sfParams.paramsNames.SF_REQUEST_GUID] = guid;
+  }
+
   /**
    * Sends out the request.
    *
@@ -765,6 +786,8 @@ function StateAbstract(options) {
 
     // augment the options with the absolute url
     requestOptions.absoluteUrl = this.buildFullUrl(requestOptions.url);
+
+    requestOptions.excludeGuid = !Util.exists(requestOptions.excludeGuid) ? false : requestOptions.excludeGuid;
   };
 
   /**

test/integration/testRequestParams.js

@@ -0,0 +1,72 @@
+/*
+ * Copyright (c) 2015-2024 Snowflake Computing Inc. All rights reserved.
+ */
+
+const testUtil = require('./testUtil');
+const assert = require('assert');
+const httpInterceptorUtils = require('./test_utils/httpInterceptorUtils');
+const Core = require('../../lib/core');
+const Util = require('../../lib/util');
+
+describe('SF service tests', function () {
+  const selectPiTxt = 'select PI();';
+  let interceptors;
+  let coreInstance;
+
+  before(async function () {
+    interceptors = new httpInterceptorUtils.Interceptors();
+    const HttpClientClassWithInterceptors = httpInterceptorUtils.getHttpClientWithInterceptorsClass(interceptors);
+    coreInstance = Core({
+      httpClientClass: HttpClientClassWithInterceptors,
+      loggerClass: require('./../../lib/logger/node'),
+      client: {
+        version: Util.driverVersion,
+        name: Util.driverName,
+        environment: process.versions,
+      },
+    });
+  });
+
+  it('GUID called for all', async function () {
+    let guidAddedWhenExpected = true;
+    let totalCallsWithGUIDCount = 0;
+    let expectedCallsWithGUIDCount = 0;
+    const pathsExpectedToIncludeGuid = [
+      'session?delete=true&requestId',
+      'queries/v1/query-request',
+      'session/v1/login-request'
+    ];
+
+    function countCallsWithGuid(requestOptions) {
+      pathsExpectedToIncludeGuid.forEach((value) => {
+        if (requestOptions.url.includes(value)) {
+          // Counting is done instead of assertions, because we do not want to interrupt
+          // the flow of operations inside the HttpClient. Retries and other custom exception handling could be triggered.
+          if (!testUtil.isGuidInRequestOptions(requestOptions)) {
+            guidAddedWhenExpected = false;
+          }
+          expectedCallsWithGUIDCount++;
+        }
+      });
+
+      if (testUtil.isGuidInRequestOptions(requestOptions)) {
+        totalCallsWithGUIDCount++;
+      }
+    }
+    interceptors.add('request', httpInterceptorUtils.HOOK_TYPE.FOR_ARGS, countCallsWithGuid);
+
+    const connection = testUtil.createConnection({}, coreInstance);
+    await testUtil.connectAsync(connection);
+    await testUtil.executeCmdAsync(connection, selectPiTxt);
+
+    const guidCallsOccurred = totalCallsWithGUIDCount > 0;
+    assert.strictEqual(guidCallsOccurred, true, 'No GUID calls occurred');
+    assert.strictEqual(guidAddedWhenExpected, true, `GUID not found in all requests with paths: ${pathsExpectedToIncludeGuid}`);
+    assert.strictEqual(expectedCallsWithGUIDCount === totalCallsWithGUIDCount, true, `GUID was added to requests not included in the expected paths: ${pathsExpectedToIncludeGuid}.` +
+      `Total calls with guid: ${totalCallsWithGUIDCount}. Expected calls with guid: ${expectedCallsWithGUIDCount}.`
+    );
+
+    await testUtil.destroyConnectionAsync(connection);
+    interceptors.clear();
+  });
+});

test/integration/testUtil.js

@@ -12,19 +12,31 @@ const Logger = require('../../lib/logger');
 const path = require('path');
 const os = require('os');
 
-module.exports.createConnection = function (validConnectionOptionsOverride = {}) {
-  return snowflake.createConnection({
+module.exports.createConnection = function (validConnectionOptionsOverride = {}, coreInstance) {
+  coreInstance = coreInstance ? coreInstance : snowflake;
+
+  return coreInstance.createConnection({
     ...connOptions.valid,
     ...validConnectionOptionsOverride,
   });
 };
 
-module.exports.createProxyConnection = function () {
-  return snowflake.createConnection(connOptions.connectionWithProxy);
+module.exports.createProxyConnection = function (validConnectionOptionsOverride, coreInstance) {
+  coreInstance = coreInstance ? coreInstance : snowflake;
+
+  return coreInstance.createConnection({
+    ...connOptions.connectionWithProxy,
+    ...validConnectionOptionsOverride
+  });
 };
 
-module.exports.createConnectionPool = function () {
-  return snowflake.createPool(connOptions.valid, { max: 10, min: 0, testOnBorrow: true });
+module.exports.createConnectionPool = function (validConnectionOptionsOverride, coreInstance) {
+  coreInstance = coreInstance ? coreInstance : snowflake;
+
+  return coreInstance.createPool({
+    ...connOptions.valid,
+    ...validConnectionOptionsOverride
+  }, { max: 10, min: 0, testOnBorrow: true });
 };
 
 module.exports.connect = function (connection, callback) {
@@ -355,6 +367,9 @@ module.exports.assertActiveConnectionDestroyedCorrectlyAsync = async function (c
   module.exports.assertConnectionInactive(connection);
 };
 
-
 module.exports.normalizeRowObject = normalizeRowObject;
-module.exports.normalizeValue = normalizeValue;
+module.exports.normalizeValue = normalizeValue;
+
+module.exports.isGuidInRequestOptions = function (requestOptions) {
+  return requestOptions.url.includes('request_guid') || 'request_guid' in requestOptions.params;
+};

test/integration/test_utils/httpInterceptorUtils.js

@@ -0,0 +1,112 @@
+const Logger = require('../../../lib/logger');
+const { NodeHttpClient } = require('../../../lib/http/node');
+const Util = require('../../../lib/util');
+
+const HOOK_TYPE = {
+  FOR_ARGS: 'args',
+  FOR_RETURNED_VALUE: 'returned',
+};
+
+module.exports.HOOK_TYPE = HOOK_TYPE;
+
+class Interceptor {
+  constructor(methodName, hookType, callback) {
+    this.methodName = methodName;
+    this.hookType = hookType || HOOK_TYPE.FOR_ARGS;
+    this.callback = callback;
+  }
+
+  execute(...args) {
+    this.callback(...args);
+  }
+}
+
+class Interceptors {
+  constructor(initialInterceptors) {
+    this.interceptorsMap = this.createInterceptorsMap(initialInterceptors);
+  }
+
+  add(methodName, hookType, callback, interceptor = undefined) {
+    if (!interceptor) {
+      interceptor = new Interceptor(methodName, hookType, callback);
+    }
+    this.interceptorsMap[interceptor.methodName][interceptor.hookType] = interceptor;
+  }
+
+  get(methodName, hookType) {
+    return this.interceptorsMap[methodName][hookType];
+  }
+
+  intercept(methodName, hookType, ...args) {
+    // When no interceptor registered - ignores and does not raise any error
+    try {
+      return this.get(methodName, hookType)?.execute(...args);
+    } catch (e) {
+      throw 'Unable to execute interceptor method in tests.  Error: ' + e;
+    }
+  }
+
+  clear() {
+    this.interceptorsMap = this.createInterceptorsMap();
+  }
+
+  createInterceptorsMap(initialInterceptors = {}) {
+    if (initialInterceptors instanceof Interceptors) {
+      return initialInterceptors.interceptorsMap;
+    }
+    // Map creating another map for each accessed key not present in the map
+    // (analogy - DefaultDict from Python).
+    return new Proxy(initialInterceptors, {
+      get: (target, prop) => {
+        if (prop in target) {
+          return target[prop];
+        } else {
+          // Create an empty object, store it in target, and return it
+          const newObj = {};
+          target[prop] = newObj;
+          return newObj;
+        }
+      }
+    });
+  }
+}
+
+module.exports.Interceptors = Interceptors;
+
+function HttpClientWithInterceptors(connectionConfig, initialInterceptors) {
+  Logger.getInstance().trace('Initializing HttpClientWithInterceptors with Connection Config[%s]',
+    connectionConfig.describeIdentityAttributes());
+  this.interceptors = new Interceptors(initialInterceptors);
+  NodeHttpClient.apply(this, [connectionConfig]);
+}
+
+Util.inherits(HttpClientWithInterceptors, NodeHttpClient);
+
+
+HttpClientWithInterceptors.prototype.requestAsync = async function (url, options) {
+  this.interceptors.intercept('requestAsync', HOOK_TYPE.FOR_ARGS, url, options);
+  const response = await NodeHttpClient.prototype.requestAsync.call(this, url, options);
+  this.interceptors.intercept('requestAsync', HOOK_TYPE.FOR_RETURNED_VALUE, response);
+  return response;
+};
+
+HttpClientWithInterceptors.prototype.request = function (url, options) {
+  this.interceptors.intercept('request', HOOK_TYPE.FOR_ARGS, url, options);
+  const response = NodeHttpClient.prototype.request.call(this, url, options);
+  this.interceptors.intercept('request', HOOK_TYPE.FOR_RETURNED_VALUE, response);
+  return response;
+};
+
+// Factory method for HttpClientWithInterceptors to be able to partially initialize class
+// with interceptors used in fully instantiated object.
+function getHttpClientWithInterceptorsClass(interceptors) {
+  function HttpClientWithInterceptorsWrapper(connectionConfig) {
+    HttpClientWithInterceptors.apply(this, [connectionConfig, interceptors]);
+  }
+  Util.inherits(HttpClientWithInterceptorsWrapper, HttpClientWithInterceptors);
+
+  return HttpClientWithInterceptorsWrapper;
+}
+
+
+module.exports.getHttpClientWithInterceptorsClass = getHttpClientWithInterceptorsClass;