Skip to content

Commit

Permalink
feat(mongodb): support v5 (#1451)
Browse files Browse the repository at this point in the history 
* feat(mongodb): add support for v5

* refactor(mongodb): avoid duplicating code for v4 & v5

* fix: lint

* refactor(mongodb): use range for mongodb version check

* chore(mongodb): make test commands consistent

* refactor(mongodb): adapt code based on main's changes

* chore: revert mongodb bump in devDependencies

* refactor: adapt code based on comments

* chore: re-use old order for tests suites

* refactor: simply metrics tests

* test: fix DB name in v5 tests

* chore: revert MongoDB to 3.6.11 in package.json

* test: fix compilation issue due to different signatures (v3 vs v5)

* fix: lint

---------

Co-authored-by: Haddas Bronfman <85441461+haddasbronfman@users.noreply.github.com>
Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com>
  • Loading branch information
@GuillaumeDeconinck @haddasbronfman @pichlermarc
3 people committed Jul 4, 2023
1 parent 8777cbd commit 05c4e9e
Show file tree
Hide file tree
Showing 7 changed files with 672 additions and 67 deletions.
6 changes: 4 additions & 2 deletions plugins/node/opentelemetry-instrumentation-mongodb/.tav.yml
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
mongodb:
jobs:
- versions: ">=3.3 <4"
commands: npm run test
commands: npm run test-v3
- versions: ">=4 <5"
commands: npm run test-new-versions
commands: npm run test-v4
- versions: ">=5 <6"
commands: npm run test-v5

# Fix missing `contrib-test-utils` package
pretest: npm run --prefix ../../../ lerna:link
10 changes: 6 additions & 4 deletions plugins/node/opentelemetry-instrumentation-mongodb/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
"repository": "open-telemetry/opentelemetry-js-contrib",
"scripts": {
"docker:start": "docker run -e MONGODB_DB=opentelemetry-tests -e MONGODB_PORT=27017 -e MONGODB_HOST=127.0.0.1 -p 27017:27017 --rm mongo",
"test": "nyc ts-mocha -p tsconfig.json --require '@opentelemetry/contrib-test-utils' 'test/**/mongodb-v3.test.ts'",
"test-new-versions": "nyc ts-mocha -p tsconfig.json --require '@opentelemetry/contrib-test-utils' 'test/mongodb-v4**.test.ts'",
"test": "npm run test-v3",
"test-v3": "nyc ts-mocha -p tsconfig.json --require '@opentelemetry/contrib-test-utils' 'test/**/mongodb-v3.test.ts'",
"test-v4": "nyc ts-mocha -p tsconfig.json --require '@opentelemetry/contrib-test-utils' 'test/mongodb-v4-v5.metrics.test.ts' 'test/**/mongodb-v4.test.ts'",
"test-v5": "nyc ts-mocha -p tsconfig.json --require '@opentelemetry/contrib-test-utils' 'test/mongodb-v4-v5.metrics.test.ts' 'test/**/mongodb-v5.test.ts'",
"test-all-versions": "tav",
"tdd": "npm run test -- --watch-extensions ts --watch",
"clean": "rimraf build/*",
Expand Down Expand Up @@ -55,12 +57,12 @@
"@opentelemetry/context-async-hooks": "^1.8.0",
"@opentelemetry/sdk-trace-base": "^1.8.0",
"@opentelemetry/sdk-trace-node": "^1.8.0",
"@types/bson": "4.0.5",
"@types/mocha": "7.0.2",
"@types/mongodb": "3.6.20",
"@types/node": "18.11.7",
"mocha": "7.2.0",
"mongodb": "3.6.11",
"@types/mongodb": "3.6.20",
"@types/bson": "4.0.5",
"nyc": "15.1.0",
"rimraf": "5.0.0",
"test-all-versions": "5.0.1",
Expand Down
8 changes: 4 additions & 4 deletions plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,25 +95,25 @@ export class MongoDBInstrumentation extends InstrumentationBase {
),
new InstrumentationNodeModuleDefinition<any>(
'mongodb',
['4.*'],
['4.*', '5.*'],
undefined,
undefined,
[
new InstrumentationNodeModuleFile<V4Connection>(
'mongodb/lib/cmap/connection.js',
['4.*'],
['4.*', '5.*'],
v4PatchConnection,
v4UnpatchConnection
),
new InstrumentationNodeModuleFile<V4Connect>(
'mongodb/lib/cmap/connect.js',
['4.*'],
['4.*', '5.*'],
v4PatchConnect,
v4UnpatchConnect
),
new InstrumentationNodeModuleFile<V4Session>(
'mongodb/lib/sessions.js',
['4.*'],
['4.*', '5.*'],
v4PatchSessions,
v4UnpatchSessions
),
Expand Down
2 changes: 1 addition & 1 deletion plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v3.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import * as mongodb from 'mongodb';
import { assertSpans, accessCollection, DEFAULT_MONGO_HOST } from './utils';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';

describe('MongoDBInstrumentation', () => {
describe('MongoDBInstrumentation-Tracing-v3', () => {
function create(config: MongoDBInstrumentationConfig = {}) {
instrumentation.setConfig(config);
}
Expand Down
67 changes: 39 additions & 28 deletions ...n-mongodb/test/mongodb-v4.metrics.test.ts → ...ongodb/test/mongodb-v4-v5.metrics.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ import {
ResourceMetrics,
} from '@opentelemetry/sdk-metrics';

import * as mongodb from 'mongodb';

const otelTestingMeterProvider = new MeterProvider();
const inMemoryMetricsExporter = new InMemoryMetricExporter(
AggregationTemporality.CUMULATIVE
Expand All @@ -40,17 +38,13 @@ const metricReader = new PeriodicExportingMetricReader({
exportTimeoutMillis: 100,
});

otelTestingMeterProvider.addMetricReader(metricReader);

import { registerInstrumentationTesting } from '@opentelemetry/contrib-test-utils';
const instrumentation = registerInstrumentationTesting(
new MongoDBInstrumentation()
);

instrumentation.setMeterProvider(otelTestingMeterProvider);

import { accessCollection, DEFAULT_MONGO_HOST } from './utils';

import * as mongodb from 'mongodb';
import * as assert from 'assert';

async function waitForNumberOfExports(
Expand Down Expand Up @@ -86,34 +80,39 @@ describe('MongoDBInstrumentation-Metrics', () => {
const DB_NAME = process.env.MONGODB_DB || 'opentelemetry-tests-metrics';
const COLLECTION_NAME = 'test-metrics';
const URL = `mongodb://${HOST}:${PORT}/${DB_NAME}`;

let client: mongodb.MongoClient;
let collection: mongodb.Collection;

before(() => {
otelTestingMeterProvider.addMetricReader(metricReader);
instrumentation?.setMeterProvider(otelTestingMeterProvider);
});

beforeEach(function mongoBeforeEach(done) {
// Skipping all tests in beforeEach() is a workaround. Mocha does not work
// properly when skipping tests in before() on nested describe() calls.
// https://github.com/mochajs/mocha/issues/2819
if (!shouldTest) {
this.skip();
}

inMemoryMetricsExporter.reset();
done();
});

it('Should add connection usage metrics', async () => {
const result = await accessCollection(URL, DB_NAME, COLLECTION_NAME);
client = result.client;
collection = result.collection;
const collection = result.collection;
const insertData = [{ a: 1 }, { a: 2 }, { a: 3 }];
await collection.insertMany(insertData);
await collection.deleteMany({});
let exportedMetrics = await waitForNumberOfExports(
const exportedMetrics = await waitForNumberOfExports(
inMemoryMetricsExporter,
1
);

assert.strictEqual(exportedMetrics.length, 1);
let metrics = exportedMetrics[0].scopeMetrics[0].metrics;
const metrics = exportedMetrics[0].scopeMetrics[0].metrics;
assert.strictEqual(metrics.length, 1);
assert.strictEqual(metrics[0].dataPointType, DataPointType.SUM);

Expand All @@ -126,43 +125,55 @@ describe('MongoDBInstrumentation-Metrics', () => {
metrics[0].descriptor.name,
'db.client.connections.usage'
);
assert.strictEqual(metrics[0].dataPoints.length, 2);
assert.strictEqual(metrics[0].dataPoints[0].value, 0);
assert.strictEqual(metrics[0].dataPoints[0].attributes['state'], 'used');

// Checking dataPoints
const dataPoints = metrics[0].dataPoints;
assert.strictEqual(dataPoints.length, 2);
assert.strictEqual(dataPoints[0].value, 0);
assert.strictEqual(dataPoints[0].attributes['state'], 'used');
assert.strictEqual(
metrics[0].dataPoints[0].attributes['pool.name'],
dataPoints[0].attributes['pool.name'],
`mongodb://${HOST}:${PORT}/${DB_NAME}`
);

assert.strictEqual(metrics[0].dataPoints[1].value, 1);
assert.strictEqual(metrics[0].dataPoints[1].attributes['state'], 'idle');
assert.strictEqual(dataPoints[1].value, 1);
assert.strictEqual(dataPoints[1].attributes['state'], 'idle');
assert.strictEqual(
metrics[0].dataPoints[1].attributes['pool.name'],
dataPoints[1].attributes['pool.name'],
`mongodb://${HOST}:${PORT}/${DB_NAME}`
);
});

it('Should add disconnection usage metrics', async () => {
await client.close();

exportedMetrics = await waitForNumberOfExports(inMemoryMetricsExporter, 2);
const exportedMetrics = await waitForNumberOfExports(
inMemoryMetricsExporter,
2
);
assert.strictEqual(exportedMetrics.length, 2);
metrics = exportedMetrics[1].scopeMetrics[0].metrics;
const metrics = exportedMetrics[1].scopeMetrics[0].metrics;
assert.strictEqual(metrics.length, 1);
assert.strictEqual(metrics[0].dataPointType, DataPointType.SUM);

assert.strictEqual(
metrics[0].descriptor.description,
'The number of connections that are currently in state described by the state attribute.'
);
assert.strictEqual(metrics[0].dataPoints.length, 2);
assert.strictEqual(metrics[0].dataPoints[0].value, 0);
assert.strictEqual(metrics[0].dataPoints[0].attributes['state'], 'used');

// Checking dataPoints
const dataPoints = metrics[0].dataPoints;
assert.strictEqual(dataPoints.length, 2);
assert.strictEqual(dataPoints[0].value, 0);
assert.strictEqual(dataPoints[0].attributes['state'], 'used');
assert.strictEqual(
metrics[0].dataPoints[0].attributes['pool.name'],
dataPoints[0].attributes['pool.name'],
`mongodb://${HOST}:${PORT}/${DB_NAME}`
);
assert.strictEqual(metrics[0].dataPoints[1].value, 0);
assert.strictEqual(metrics[0].dataPoints[1].attributes['state'], 'idle');
assert.strictEqual(dataPoints[1].value, 0);
assert.strictEqual(dataPoints[1].attributes['state'], 'idle');
assert.strictEqual(
metrics[0].dataPoints[1].attributes['pool.name'],
dataPoints[1].attributes['pool.name'],
`mongodb://${HOST}:${PORT}/${DB_NAME}`
);
});
Expand Down
52 changes: 24 additions & 28 deletions plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v4.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@

import { context, trace, SpanKind, Span } from '@opentelemetry/api';
import * as assert from 'assert';
import { MongoDBInstrumentation, MongoDBInstrumentationConfig } from '../src';
import { MongoResponseHookInformation } from '../src';
import {
MongoDBInstrumentation,
MongoDBInstrumentationConfig,
MongoResponseHookInformation,
} from '../src';
import {
registerInstrumentationTesting,
getTestSpans,
Expand All @@ -34,7 +37,7 @@ import * as mongodb from 'mongodb';
import { assertSpans, accessCollection, DEFAULT_MONGO_HOST } from './utils';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';

describe('MongoDBInstrumentation-Tracing', () => {
describe('MongoDBInstrumentation-Tracing-v4', () => {
function create(config: MongoDBInstrumentationConfig = {}) {
instrumentation.setConfig(config);
}
Expand All @@ -47,12 +50,11 @@ describe('MongoDBInstrumentation-Tracing', () => {
shouldTest = false;
}

const URL = `mongodb://${process.env.MONGODB_HOST || DEFAULT_MONGO_HOST}:${
process.env.MONGODB_PORT || '27017'
}`;
const HOST = process.env.MONGODB_HOST || DEFAULT_MONGO_HOST;
const PORT = process.env.MONGODB_PORT || '27017';
const DB_NAME = process.env.MONGODB_DB || 'opentelemetry-tests-traces';
const COLLECTION_NAME = 'test-traces';
const conn_string = `${URL}/${DB_NAME}`;
const URL = `mongodb://${HOST}:${PORT}/${DB_NAME}`;

let client: mongodb.MongoClient;
let collection: mongodb.Collection;
Expand Down Expand Up @@ -95,9 +97,9 @@ describe('MongoDBInstrumentation-Tracing', () => {
done();
});

after(() => {
after(async () => {
if (client) {
client.close();
await client.close();
}
});

Expand All @@ -116,7 +118,7 @@ describe('MongoDBInstrumentation-Tracing', () => {
'mongodb.insert',
SpanKind.CLIENT,
'insert',
conn_string
URL
);
done();
})
Expand All @@ -138,7 +140,7 @@ describe('MongoDBInstrumentation-Tracing', () => {
'mongodb.update',
SpanKind.CLIENT,
'update',
conn_string
URL
);
done();
})
Expand All @@ -160,7 +162,7 @@ describe('MongoDBInstrumentation-Tracing', () => {
'mongodb.delete',
SpanKind.CLIENT,
'delete',
conn_string
URL
);
done();
})
Expand All @@ -186,7 +188,7 @@ describe('MongoDBInstrumentation-Tracing', () => {
'mongodb.find',
SpanKind.CLIENT,
'find',
conn_string
URL
);
done();
})
Expand Down Expand Up @@ -215,7 +217,7 @@ describe('MongoDBInstrumentation-Tracing', () => {
'mongodb.find',
SpanKind.CLIENT,
'find',
conn_string
URL
);
// assert that we correctly got the first as a find
assertSpans(
Expand All @@ -225,7 +227,7 @@ describe('MongoDBInstrumentation-Tracing', () => {
'mongodb.getMore',
SpanKind.CLIENT,
'getMore',
conn_string
URL
);
done();
})
Expand All @@ -251,7 +253,7 @@ describe('MongoDBInstrumentation-Tracing', () => {
'mongodb.createIndexes',
SpanKind.CLIENT,
'createIndexes',
conn_string
URL
);
done();
})
Expand Down Expand Up @@ -287,7 +289,7 @@ describe('MongoDBInstrumentation-Tracing', () => {
operationName,
SpanKind.CLIENT,
'insert',
conn_string,
URL,
false,
false
);
Expand Down Expand Up @@ -333,7 +335,7 @@ describe('MongoDBInstrumentation-Tracing', () => {
operationName,
SpanKind.CLIENT,
'insert',
conn_string,
URL,
false,
true
);
Expand Down Expand Up @@ -374,7 +376,7 @@ describe('MongoDBInstrumentation-Tracing', () => {
'mongodb.insert',
SpanKind.CLIENT,
'insert',
conn_string
URL
);
done();
})
Expand Down Expand Up @@ -469,13 +471,7 @@ describe('MongoDBInstrumentation-Tracing', () => {
.then(() => {
span.end();
const spans = getTestSpans();
assertSpans(
spans,
'mongodb.find',
SpanKind.CLIENT,
'find',
conn_string
);
assertSpans(spans, 'mongodb.find', SpanKind.CLIENT, 'find', URL);
done();
})
.catch(err => {
Expand All @@ -502,7 +498,7 @@ describe('MongoDBInstrumentation-Tracing', () => {
'mongodb.insert',
SpanKind.CLIENT,
'insert',
conn_string
URL
);
resetMemoryExporter();

Expand All @@ -517,7 +513,7 @@ describe('MongoDBInstrumentation-Tracing', () => {
'mongodb.find',
SpanKind.CLIENT,
'find',
conn_string
URL
);
assert.strictEqual(
mainSpan.spanContext().spanId,
Expand Down
Loading

0 comments on commit 05c4e9e

Please sign in to comment.