Skip to content
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.

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

feat(cassandra-responsehook): added response hook to execute func #1180

Merged
merged 38 commits into from
Oct 28, 2022
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
39a175c
feat(cassandra-responsehook): added response hook in to execute func
samimusallam Sep 18, 2022
0572e3f
Merge branch 'main' into feat/cassandra-responsehook
samimusallam Sep 18, 2022
427836f
Merge branch 'main' into feat/cassandra-responsehook
samimusallam Sep 19, 2022
3f50695
Merge branch 'main' into feat/cassandra-responsehook
samimusallam Sep 20, 2022
8308c42
Merge branch 'main' into feat/cassandra-responsehook
samimusallam Sep 22, 2022
a88b21f
feat(cassandra-responsehook): log error in responsehook call
samimusallam Sep 22, 2022
254c02c
feat(cassandra-responsehook): using ResponseInfo object in the respon…
samimusallam Sep 22, 2022
e9c07eb
feat(cassandra-responsehook): added attribute from response to test
samimusallam Sep 27, 2022
7e4789e
feat(cassandra-responsehook): added exception handling test
samimusallam Sep 27, 2022
5c03bf6
feat(cassandra-responsehook): added responseHook option to README.md
samimusallam Sep 27, 2022
f13f4ff
Merge branch 'main' into feat/cassandra-responsehook
samimusallam Sep 27, 2022
c7fc26a
Merge branch 'main' into feat/cassandra-responsehook
samimusallam Sep 28, 2022
64c8349
Merge branch 'main' into feat/cassandra-responsehook
rauno56 Sep 30, 2022
f00166a
feat(cassandra-responsehook): removed unnecessary log prefix
samimusallam Sep 29, 2022
84251d9
feat(cassandra-responsehook): using Attributes structure in tests
samimusallam Sep 29, 2022
a27452d
feat(cassandra-responsehook): using number type to test instead of co…
samimusallam Sep 29, 2022
7a94547
feat(cassandra-responsehook): changed assertAttributeInSpan to assert…
samimusallam Sep 29, 2022
21c45bb
feat(cassandra-responsehook): removed unused params from assertAttrib…
samimusallam Oct 9, 2022
f0e3bfc
feat(cassandra-responsehook): removed overridden setConfig and getConfig
samimusallam Oct 9, 2022
b213616
Merge branch 'main' into feat/cassandra-responsehook
samimusallam Oct 9, 2022
faa1b30
Merge branch 'main' into feat/cassandra-responsehook
samimusallam Oct 11, 2022
0ec4682
feat(cassandra-responsehook): changed 'any' to it's type T in wrapPro…
samimusallam Oct 11, 2022
a62041c
feat(cassandra-responsehook): fixed line break issue in README.md
samimusallam Oct 18, 2022
ed1ce61
feat(cassandra-responsehook): using type Attributes instead of any
samimusallam Oct 18, 2022
d7aa856
Merge branch 'main' into feat/cassandra-responsehook
samimusallam Oct 18, 2022
584af3a
feat(cassandra-responsehook): using spread syntax and toNumber
samimusallam Oct 19, 2022
9a0a405
feat(cassandra-responsehook): small refactor for better readability
samimusallam Oct 19, 2022
4d0f7e7
Merge branch 'main' into feat/cassandra-responsehook
samimusallam Oct 19, 2022
e78076a
Merge branch 'main' into feat/cassandra-responsehook
samimusallam Oct 20, 2022
12bdecc
Merge branch 'main' into feat/cassandra-responsehook
samimusallam Oct 21, 2022
afa455c
feat(cassandra-responsehook): using result subset in CassandraDriverR…
samimusallam Oct 24, 2022
b33c85f
feat(cassandra-responsehook): updated responseInfo's description
samimusallam Oct 27, 2022
49449e2
feat(cassandra-responsehook): code style changes in types.ts
samimusallam Oct 27, 2022
321afb7
feat(cassandra-responsehook): pass the whole result instead of result…
samimusallam Oct 27, 2022
021c693
Merge branch 'main' into feat/cassandra-responsehook
blumamir Oct 28, 2022
e1c1b13
Merge branch 'main' into feat/cassandra-responsehook
blumamir Oct 28, 2022
41fb581
Merge branch 'main' into feat/cassandra-responsehook
blumamir Oct 28, 2022
2c6a7d6
Merge branch 'main' into feat/cassandra-responsehook
blumamir Oct 28, 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
10 changes: 5 additions & 5 deletions plugins/node/opentelemetry-instrumentation-cassandra/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ await client.execute('select * from foo');

### Instrumentation options

| Option | Type | Default | Description |
| ------ | ---- | ------- | ----------- |
| `enhancedDatabaseReporting` | `boolean` | `false` | Whether to include database queries with spans. These can contain sensitive information when using unescaped parameters - i.e. `insert into persons (name) values ('Bob')` instead of `insert into persons (name) values (?)`. |
| `maxQueryLength` | `number` | `65536` | If `enhancedDatabaseReporting` is enabled, limits the attached query strings
to this length. |
| Option | Type | Default | Description |
|-----------------------------|--------------------------------------------------|-------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `enhancedDatabaseReporting` | `boolean` | `false` | Whether to include database queries with spans. These can contain sensitive information when using unescaped parameters - i.e. `insert into persons (name) values ('Bob')` instead of `insert into persons (name) values (?)`. |
| `responseHook` | `CassandraDriverResponseCustomAttributeFunction` | `undefined` | Hook for adding custom attributes before response is handled |
| `maxQueryLength` | `number` | `65536` | If `enhancedDatabaseReporting` is enabled, limits the attached query strings to this length. |

### Supported versions

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
isWrapped,
safeExecuteInTheMiddle,
} from '@opentelemetry/instrumentation';
import { CassandraDriverInstrumentationConfig } from './types';
import { CassandraDriverInstrumentationConfig, ResultSet } from './types';
import {
SemanticAttributes,
DbSystemValues,
Expand All @@ -41,6 +41,8 @@ import type * as CassandraDriver from 'cassandra-driver';
const supportedVersions = ['>=4.4 <5.0'];

export class CassandraDriverInstrumentation extends InstrumentationBase {
protected override _config!: CassandraDriverInstrumentationConfig;

constructor(config: CassandraDriverInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-cassandra-driver', VERSION, config);
}
Expand Down Expand Up @@ -147,7 +149,13 @@ export class CassandraDriverInstrumentation extends InstrumentationBase {
}
);

const wrappedPromise = wrapPromise(span, execPromise);
const wrappedPromise = wrapPromise(
span,
execPromise,
(span, result) => {
plugin._callResponseHook(span, result);
}
);

return context.bind(execContext, wrappedPromise);
};
Expand Down Expand Up @@ -319,6 +327,22 @@ export class CassandraDriverInstrumentation extends InstrumentationBase {
attributes,
});
}

private _callResponseHook(span: Span, response: ResultSet) {
if (!this._config.responseHook) {
return;
}

safeExecuteInTheMiddle(
() => this._config.responseHook!(span, { response: response }),
e => {
if (e) {
this._diag.error('responseHook error', e);
}
},
true
);
}
}

function failSpan(span: Span, error: Error) {
Expand All @@ -336,10 +360,17 @@ function combineQueries(queries: Array<string | { query: string }>) {
.join('\n');
}

function wrapPromise<T>(span: Span, promise: Promise<T>): Promise<T> {
function wrapPromise<T>(
span: Span,
promise: Promise<T>,
successCallback?: (span: Span, result: T) => void
): Promise<T> {
return promise
.then(result => {
return new Promise<T>(resolve => {
if (successCallback) {
successCallback(span, result);
}
span.end();
resolve(result);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,32 @@
*/

import { InstrumentationConfig } from '@opentelemetry/instrumentation';
import { Span } from '@opentelemetry/api';

export interface Row {
get(columnName: string | number): any;

keys(): string[];

forEach(callback: (row: Row) => void): void;

values(): any[];

[key: string]: any;
}

// https://github.com/datastax/nodejs-driver/blob/d42176e4baa1cfc3df79699cc3b5d575c86e3cec/lib/types/index.d.ts#L323
export interface ResultSet {
rows: Row[];
}

export interface ResponseHookInfo {
response: ResultSet;
}

export interface CassandraDriverResponseCustomAttributeFunction {
(span: Span, responseInfo: ResponseHookInfo): void;
}

export interface CassandraDriverInstrumentationConfig
extends InstrumentationConfig {
Expand All @@ -29,4 +55,10 @@ export interface CassandraDriverInstrumentationConfig
* @default 65536
*/
maxQueryLength?: number;
/**
* Function for adding custom attributes before response is handled.
* @param span the current span
* @param responseInfo array of the resulting rows. This will only return the first page of results
*/
responseHook?: CassandraDriverResponseCustomAttributeFunction;
samimusallam marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import {
ReadableSpan,
} from '@opentelemetry/sdk-trace-base';
import {
Attributes,
context,
Span,
SpanKind,
SpanStatus,
SpanStatusCode,
Expand All @@ -38,6 +40,7 @@ import {
CassandraDriverInstrumentation,
CassandraDriverInstrumentationConfig,
} from '../src';
import { ResponseHookInfo } from '../src/types';

const memoryExporter = new InMemorySpanExporter();
const provider = new NodeTracerProvider();
Expand All @@ -53,11 +56,13 @@ function assertSpan(
span: ReadableSpan,
name: string,
query?: string,
status?: SpanStatus
status?: SpanStatus,
customAttributes?: Attributes
) {
const attributes = {
const attributes: Attributes = {
[SemanticAttributes.DB_SYSTEM]: DbSystemValues.CASSANDRA,
[SemanticAttributes.DB_USER]: 'cassandra',
...customAttributes,
};

if (query !== undefined) {
Expand All @@ -76,6 +81,13 @@ function assertSingleSpan(name: string, query?: string, status?: SpanStatus) {
assertSpan(span, name, query, status);
}

function assertAttributeInSingleSpan(name: string, attributes?: Attributes) {
const spans = memoryExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1);
const [span] = spans;
assertSpan(span, name, undefined, undefined, attributes);
}

function assertErrorSpan(
name: string,
error: Error & { code?: number },
Expand Down Expand Up @@ -131,6 +143,7 @@ describe('CassandraDriverInstrumentation', () => {
}

instrumentation = new CassandraDriverInstrumentation();

instrumentation.setTracerProvider(provider);

const cassandra = require('cassandra-driver');
Expand Down Expand Up @@ -228,6 +241,60 @@ describe('CassandraDriverInstrumentation', () => {
assertSingleSpan('cassandra-driver.execute', query.substr(0, 25));
});
});

describe('responseHook', () => {
samimusallam marked this conversation as resolved.
Show resolved Hide resolved
after(() => {
instrumentation.setConfig({});
});

it('adds custom attributes to span', async () => {
const responseAttributeName = 'response.attribute';
const customAttributeName = 'custom.attribute';
const customAttributeValue = 'custom attribute value';

const config: CassandraDriverInstrumentationConfig = {
responseHook: (span: Span, responseInfo: ResponseHookInfo) => {
const row = responseInfo.response.rows[0];
const responseValue = row.count.toNumber();

span.setAttribute(responseAttributeName, responseValue);
span.setAttribute(customAttributeName, customAttributeValue);
},
};

instrumentation.setConfig(config);

await client.execute(
"SELECT count(*) FROM system_schema.columns WHERE keyspace_name = 'ot' AND table_name = 'test';"
);

assertAttributeInSingleSpan('cassandra-driver.execute', {
[customAttributeName]: customAttributeValue,
[responseAttributeName]: 2,
});
});

it('throws and should not affect user flow or span creation', async () => {
const hookAttributeName = 'hook.attribute';
const hookAttributeValue = 'hook attribute value';

const config: CassandraDriverInstrumentationConfig = {
responseHook: (span: Span, responseInfo: ResponseHookInfo): void => {
span.setAttribute(hookAttributeName, hookAttributeValue);
throw new Error('error inside hook');
},
};

instrumentation.setConfig(config);

const query = 'select * from ot.test';
await client.execute(query);

assertAttributeInSingleSpan('cassandra-driver.execute', {
[hookAttributeName]: hookAttributeValue,
});
});
});
});

describe('batch', () => {
Expand Down