Skip to content

Nodejs knex: Remove opencensus functionalities #146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Thiyagu55 merged 3 commits into from
Aug 29, 2022
Merged

Nodejs knex: Remove opencensus functionalities #146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
236198c
Decouple opencensus
Thiyagu55 Aug 17, 2022
4a7f808
Remove opencensus from package.json
Thiyagu55 Aug 17, 2022
6c79795
Merge branch 'master' into nodejs-knex-depricate-knex
Thiyagu55 Aug 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5,816 changes: 5,134 additions & 682 deletions nodejs/sqlcommenter-nodejs/packages/sqlcommenter-knex/package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 1 addition & 5 deletions nodejs/sqlcommenter-nodejs/packages/sqlcommenter-knex/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
"test": "test"
},
"devDependencies": {
"@opencensus/nodejs": "^0.0.22",
"@opencensus/propagation-tracecontext": "^0.0.22",
"@opentelemetry/api": "^1.0.3",
"@opentelemetry/core": "^0.24.0",
"@opentelemetry/node": "^0.24.0",
Expand All @@ -23,8 +21,6 @@
"sinon-chai": "^3.5.0"
},
"peerDependencies": {
"@opencensus/nodejs": "^0.0.22",
"@opencensus/propagation-tracecontext": "^0.0.22",
"@opentelemetry/core": "^0.24.0"
},
"engines": {
Expand All @@ -39,4 +35,4 @@
},
"author": "Google, LLC.",
"license": "Apache-2.0"
}
}
6 changes: 2 additions & 4 deletions nodejs/sqlcommenter-nodejs/packages/sqlcommenter-knex/provider/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,17 @@
// See the License for the specific language governing permissions and
// limitations under the License.

const OpenCensus = require('./opencensus');
const OpenTelemetry = require('./opentelemetry');

const providers = {
'opentelemetry': OpenTelemetry,
'opencensus': OpenCensus,
'opentelemetry': OpenTelemetry
}

exports.attachComments = function attachComments(providerName, comments) {
// Verify we have a comments object to modify
if (!comments || typeof comments !== 'object') return;

// Lookup the provider by name, or use the default.
let provider = providers[String(providerName).toLowerCase()] || OpenCensus;
let provider = providers[String(providerName).toLowerCase()] || OpenTelemetry;
provider.addW3CTraceContext(comments);
}
22 changes: 0 additions & 22 deletions nodejs/sqlcommenter-nodejs/packages/sqlcommenter-knex/provider/opencensus.js
View file
Open in desktop

This file was deleted.

85 changes: 21 additions & 64 deletions nodejs/sqlcommenter-nodejs/packages/sqlcommenter-knex/test/comment.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,12 @@

"use strict";

const {wrapMainKnex} = require('../index');
const opencensus_tracing = require('@opencensus/nodejs');
const { wrapMainKnex } = require('../index');
const chai = require("chai");
const {fields} = require('../util');
const {context, trace} = require('@opentelemetry/api');
const {NodeTracerProvider} = require('@opentelemetry/node');
const {AsyncHooksContextManager} = require('@opentelemetry/context-async-hooks');
const {InMemorySpanExporter, SimpleSpanProcessor} = require('@opentelemetry/tracing');
const { context, trace } = require('@opentelemetry/api');
const { NodeTracerProvider } = require('@opentelemetry/node');
const { AsyncHooksContextManager } = require('@opentelemetry/context-async-hooks');
const { InMemorySpanExporter, SimpleSpanProcessor } = require('@opentelemetry/tracing');

const expect = chai.expect;

Expand All @@ -30,7 +28,7 @@ describe("Comments for Knex", () => {
let fakeKnex = {
Client: {
prototype: {
config: { connection: { database: 'fake'}, client: 'fakesql'},
config: { connection: { database: 'fake' }, client: 'fakesql' },
version: 'fake-server:0.0.X',
query: (conn, obj) => {
return Promise.resolve(obj); // simply returns a resolved promise for inspection.
Expand All @@ -43,24 +41,24 @@ describe("Comments for Knex", () => {
};

before(() => {
wrapMainKnex(fakeKnex, {db_driver: true})
wrapMainKnex(fakeKnex, { db_driver: true })
});

describe("Cases", () => {

it("should add comment to generated sql", (done) => {

const want = "SELECT CURRENT_TIMESTAMP /*db_driver='knex%3Afake%3A0.0.1'*/";
const obj = {sql: 'SELECT CURRENT_TIMESTAMP'};
const obj = { sql: 'SELECT CURRENT_TIMESTAMP' };

fakeKnex.Client.prototype.query(null, obj).then(({sql}) => {
fakeKnex.Client.prototype.query(null, obj).then(({ sql }) => {
expect(sql).equals(want);
});
done();
});

it("should NOT affix comments to statements with existing comments", (done) => {

const queries = [
'SELECT * FROM people /* existing */',
'SELECT * FROM people -- existing'
Expand All @@ -80,7 +78,7 @@ describe("Comments for Knex", () => {
const want = [
"db_driver",
];
fakeKnex.Client.prototype.query(null, 'SELECT * from foo').then(({sql}) => {
fakeKnex.Client.prototype.query(null, 'SELECT * from foo').then(({ sql }) => {
want.forEach((key) => {
expect(sql.indexOf(key)).to.be.gt(-1);
});
Expand All @@ -90,17 +88,17 @@ describe("Comments for Knex", () => {

it("should deterministically sort keys alphabetically", (done) => {
const want = "SELECT * from foo /*db_driver='knex%3Afake%3A0.0.1'*/";
fakeKnex.Client.prototype.query(null, {sql: 'SELECT * from foo'}).then(({sql}) => {
fakeKnex.Client.prototype.query(null, { sql: 'SELECT * from foo' }).then(({ sql }) => {
expect(sql).equals(want);
});
done();
});

it("chaining and repeated calls should NOT indefinitely chain SQL", (done) => {

const want = "SELECT * from foo /*db_driver='knex%3Afake%3A0.0.1'*/";
const obj = {sql: 'SELECT * from foo'};

const obj = { sql: 'SELECT * from foo' };

fakeKnex.Client.prototype.query(null, obj)
.then((a) => fakeKnex.Client.prototype.query(null, a))
Expand All @@ -109,61 +107,19 @@ describe("Comments for Knex", () => {
.then((d) => {
expect(d.sql).equals(want);
});

done();
});
});
});


describe("With OpenCensus tracing", () => {

let fakeKnex = {
Client: {
prototype: {
config: { connection: { database: 'fake'}, client: 'fakesql'},
version: 'fake-server:0.0.X',
query: (conn, obj) => {
return Promise.resolve(obj); // simply returns a resolved promise for inspection.
}
}
},
VERSION: () => {
return 'fake:0.0.1';
}
};

before(() => {
wrapMainKnex(fakeKnex, {db_driver: true, traceparent: true, tracestate: true}, {TraceProvider: "OpenCensus"});
});

it('Starting an OpenCensus trace should produce `traceparent`', (done) => {
// Let's remember https://github.com/census-instrumentation/opencensus-node/issues/580

const traceOptions = {
samplingRate: 1, // Always sample
};
const tracer = opencensus_tracing.start(traceOptions).tracer;

tracer.startRootSpan({ name: 'with-tracing' }, rootSpan => {
const obj = {sql: 'SELECT * FROM foo'};
fakeKnex.Client.prototype.query(null, obj).then((got) => {
const augmentedSQL = got.sql;
const wantSQL = `SELECT * FROM foo /*db_driver='knex%3Afake%3A0.0.1',traceparent='00-${rootSpan.traceId}-${rootSpan.id}-01'*/`;
expect(augmentedSQL).equals(wantSQL);
opencensus_tracing.tracer.stop();
done();
});
});
});
});

describe("With OpenTelemetry tracing", () => {

let fakeKnex = {
Client: {
prototype: {
config: { connection: { database: 'fake'}, client: 'fakesql'},
config: { connection: { database: 'fake' }, client: 'fakesql' },
version: 'fake-server:0.0.X',
query: (conn, obj) => {
return Promise.resolve(obj); // simply returns a resolved promise for inspection.
Expand All @@ -187,7 +143,7 @@ describe("With OpenTelemetry tracing", () => {
before(() => {
contextManager = new AsyncHooksContextManager();
context.setGlobalContextManager(contextManager.enable());
wrapMainKnex(fakeKnex, {db_driver: true, traceparent: true, tracestate: true}, {TraceProvider: "OpenTelemetry"});
wrapMainKnex(fakeKnex, { db_driver: true, traceparent: true, tracestate: true }, { TraceProvider: "" });
});

after(() => {
Expand All @@ -198,10 +154,11 @@ describe("With OpenTelemetry tracing", () => {
it('Starting an OpenTelemetry trace should produce `traceparent`', (done) => {
const rootSpan = tracer.startSpan('rootSpan');
context.with(trace.setSpan(context.active(), rootSpan), async () => {
const obj = {sql: 'SELECT * FROM foo'};
const obj = { sql: 'SELECT * FROM foo' };
fakeKnex.Client.prototype.query(null, obj).then((got) => {
const augmentedSQL = got.sql;
const wantSQL = `SELECT * FROM foo /*db_driver='knex%3Afake%3A0.0.1',traceparent='00-${rootSpan.spanContext().traceId}-${rootSpan.spanContext().spanId}-01'*/`;
console.log(augmentedSQL);
expect(augmentedSQL).equals(wantSQL);
rootSpan.end();
done();
Expand Down
33 changes: 8 additions & 25 deletions nodejs/sqlcommenter-nodejs/packages/sqlcommenter-knex/test/unit/provider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,14 @@ const openCensusMock = sinon.spy();
const openTelemetryMock = sinon.spy();

// Mock the dependencies of provider to make use of faked methods for adding trace context
rewiremock('../../provider/opencensus').with({addW3CTraceContext: openCensusMock});
rewiremock('../../provider/opentelemetry').with({addW3CTraceContext: openTelemetryMock});
rewiremock('../../provider/opentelemetry').with({ addW3CTraceContext: openTelemetryMock });

// Load the provider module with the appropriate mocks
rewiremock.enable();
const provider = require('../../provider');

// A helper method to test which provider was called
const verifyProviderUsed = function(traceProvider, spy, used) {
const verifyProviderUsed = function (traceProvider, spy, used) {
provider.attachComments(traceProvider, {});
expect(spy.called).to.equal(used);
};
Expand All @@ -43,42 +42,26 @@ describe("Provider", () => {
describe("attachComment", () => {

beforeEach(() => {
openCensusMock.resetHistory();
openTelemetryMock.resetHistory();
})

it("should default to OpenCensus when no options are provided", () => {
verifyProviderUsed(undefined, openCensusMock, true);
verifyProviderUsed(undefined, openTelemetryMock, false);
it("should default to OpenTelemetry when no options are provided", () => {
verifyProviderUsed(undefined, openTelemetryMock, true);
});

it("should default to OpenCensus when null is provided", () => {
verifyProviderUsed(null, openCensusMock, true);
verifyProviderUsed(null, openTelemetryMock, false);
it("should default to OpenTelemetry when null is provided", () => {
verifyProviderUsed(null, openTelemetryMock, true);
});

it("should default to OpenCensus when invalid options are provided", () => {
verifyProviderUsed("bad trace library name", openCensusMock, true);
verifyProviderUsed("bad trace library name", openTelemetryMock, false);
});

it("should use OpenCensus when the name is provided", () => {
verifyProviderUsed("opencensus", openCensusMock, true);
verifyProviderUsed("opencensus", openTelemetryMock, false);
});

it("should accept an arbitrary capitalization of OpenCensus", () => {
verifyProviderUsed("oPeNceNSus", openCensusMock, true);
verifyProviderUsed("oPeNceNSus", openTelemetryMock, false);
it("should default to OpenTelemetry when invalid options are provided", () => {
verifyProviderUsed("bad trace library name", openTelemetryMock, true);
});

it("should use OpenTelemetry when the name is provided", () => {
verifyProviderUsed("opentelemetry", openCensusMock, false);
verifyProviderUsed("opentelemetry", openTelemetryMock, true);
});

it("should accept an arbitrary capitalization of OpenTelemetry", () => {
verifyProviderUsed("OpenTeleMetRY", openCensusMock, false);
verifyProviderUsed("OpenTeleMetRY", openTelemetryMock, true);
});

Expand Down
Loading