Skip to content

Commit

Permalink
feat(instr-knex): implement requireParentSpan config flag (#2288)
Browse files Browse the repository at this point in the history
Co-authored-by: Amir Blum <amirgiraffe@gmail.com>
  • Loading branch information
reberhardt7 and blumamir committed Jul 23, 2024
1 parent 6dfe93c commit 6e8989d
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 4 deletions.
1 change: 1 addition & 0 deletions plugins/node/opentelemetry-instrumentation-knex/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ registerInstrumentations({
| Options | Type | Example | Description |
| ------- | ---- | ------- | ----------- |
| `maxQueryLength` | `number` | `100` | Truncate `db.statement` attribute to a maximum length. If the statement is truncated `'..'` is added to it's end. Default `1022`. `-1` leaves `db.statement` untouched. |
| `requireParentSpan` | `boolean` | `false` | Don't create spans unless they are part of an existing trace. Default is `false`. |

## Semantic Conventions

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import { KnexInstrumentationConfig } from './types';
const contextSymbol = Symbol('opentelemetry.instrumentation-knex.context');
const DEFAULT_CONFIG: KnexInstrumentationConfig = {
maxQueryLength: 1022,
requireParentSpan: false,
};

export class KnexInstrumentation extends InstrumentationBase<KnexInstrumentationConfig> {
Expand Down Expand Up @@ -120,7 +121,7 @@ export class KnexInstrumentation extends InstrumentationBase<KnexInstrumentation

private createQueryWrapper(moduleVersion?: string) {
const instrumentation = this;
return function wrapQuery(original: () => any) {
return function wrapQuery(original: (...args: any[]) => any) {
return function wrapped_logging_method(this: any, query: any) {
const config = this.client.config;

Expand Down Expand Up @@ -152,14 +153,22 @@ export class KnexInstrumentation extends InstrumentationBase<KnexInstrumentation
);
}

const parent = this.builder[contextSymbol];
const parentContext =
this.builder[contextSymbol] || api.context.active();
const parentSpan = api.trace.getSpan(parentContext);
const hasActiveParent =
parentSpan && api.trace.isSpanContextValid(parentSpan.spanContext());
if (instrumentation._config.requireParentSpan && !hasActiveParent) {
return original.bind(this)(...arguments);
}

const span = instrumentation.tracer.startSpan(
utils.getName(name, operation, table),
{
kind: api.SpanKind.CLIENT,
attributes,
},
parent
parentContext
);
const spanContext = api.trace.setSpan(api.context.active(), span);

Expand Down
2 changes: 2 additions & 0 deletions plugins/node/opentelemetry-instrumentation-knex/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,6 @@ import { InstrumentationConfig } from '@opentelemetry/instrumentation';
export interface KnexInstrumentationConfig extends InstrumentationConfig {
/** max query length in db.statement attribute ".." is added to the end when query is truncated */
maxQueryLength?: number;
/** only create spans if part of an existing trace */
requireParentSpan?: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@
* limitations under the License.
*/

import { SpanKind, context, trace } from '@opentelemetry/api';
import {
INVALID_SPAN_CONTEXT,
SpanKind,
context,
trace,
} from '@opentelemetry/api';
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
import {
Expand Down Expand Up @@ -452,6 +457,73 @@ describe('Knex instrumentation', () => {
);
});
});

describe('Setting requireParentSpan=true', () => {
beforeEach(() => {
plugin.disable();
plugin.setConfig({ requireParentSpan: true });
plugin.enable();
});

it('should not create new spans when there is no parent', async () => {
await client.schema.createTable('testTable1', (table: any) => {
table.string('title');
});
assert.deepEqual(await client('testTable1').select('*'), []);
assert.deepEqual(await client.raw('select 1 as result'), [{ result: 1 }]);
assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
});

it('should not create new spans when there is an INVALID_SPAN_CONTEXT parent', async () => {
const parentSpan = trace.wrapSpanContext(INVALID_SPAN_CONTEXT);
await context.with(
trace.setSpan(context.active(), parentSpan),
async () => {
await client.schema.createTable('testTable1', (table: any) => {
table.string('title');
});
assert.deepEqual(await client('testTable1').select('*'), []);
assert.deepEqual(await client.raw('select 1 as result'), [
{ result: 1 },
]);
}
);
assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
});

it('should create new spans when there is a parent', async () => {
await tracer.startActiveSpan('parentSpan', async parentSpan => {
await client.schema.createTable('testTable1', (table: any) => {
table.string('title');
});
assert.deepEqual(await client('testTable1').select('*'), []);
assert.deepEqual(await client.raw('select 1 as result'), [
{ result: 1 },
]);
parentSpan.end();
});
assert.strictEqual(memoryExporter.getFinishedSpans().length, 4);
const instrumentationSpans = memoryExporter.getFinishedSpans();
const last = instrumentationSpans.pop() as any;
assertSpans(instrumentationSpans, [
{
statement: 'create table `testTable1` (`title` varchar(255))',
parentSpan: last,
},
{
op: 'select',
table: 'testTable1',
statement: 'select * from `testTable1`',
parentSpan: last,
},
{
op: 'raw',
statement: 'select 1 as result',
parentSpan: last,
},
]);
});
});
});

const assertSpans = (actualSpans: any[], expectedSpans: any[]) => {
Expand Down

0 comments on commit 6e8989d

Please sign in to comment.