Skip to content

Commit

Permalink
SNOW-1801434 add GUID to request in node.js driver (#965)
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-fpawlowski authored Dec 6, 2024
1 parent bf3b4c2 commit 6d27e7d
Show file tree
Hide file tree
Showing 8 changed files with 297 additions and 27 deletions.
8 changes: 8 additions & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
2 changes: 2 additions & 0 deletions lib/constants/sf_params.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
exports.paramsNames = {};
exports.paramsNames.SF_REQUEST_GUID = 'request_guid';
23 changes: 17 additions & 6 deletions lib/http/request_util.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const sfParams = require('../constants/sf_params');

/**
* Should work with not yet parsed options as well (before calling prepareRequestOptions method).
Expand All @@ -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] });
};

/**
Expand All @@ -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 + ']';
};
33 changes: 28 additions & 5 deletions lib/services/sf.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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));
Expand All @@ -703,7 +711,7 @@ function StateAbstract(options) {
///////////////////////////////////////////////////////////////////////////

/**
* Creates a new Request.
* Creates a new Request to Snowflake.
*
* @param {Object} requestOptions
* @constructor
Expand All @@ -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.
*
Expand Down Expand Up @@ -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;
};

/**
Expand Down
72 changes: 72 additions & 0 deletions test/integration/testRequestParams.js
Original file line number Diff line number Diff line change
@@ -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();
});
});
31 changes: 23 additions & 8 deletions test/integration/testUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
};
112 changes: 112 additions & 0 deletions test/integration/test_utils/httpInterceptorUtils.js
Original file line number Diff line number Diff line change
@@ -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;
Loading

0 comments on commit 6d27e7d

Please sign in to comment.