Skip to content

Commit

Permalink
feat(instrumentation-fs): require parent span
Browse files Browse the repository at this point in the history
  • Loading branch information
unflxw committed Dec 27, 2022
1 parent 378f130 commit e408510
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 14 deletions.
5 changes: 3 additions & 2 deletions plugins/node/instrumentation-fs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ You can set the following:

| Options | Type | Description |
| ------- | ---- | ----------- |
| `createHook` | `(functionName: FMember | FPMember, info: { args: ArrayLike<unknown> }) => boolean` | Hook called before creating the span. If `false` is returned this and all the sibling calls will not be traced. |
| `endHook` | `( functionName: FMember | FPMember, info: { args: ArrayLike<unknown>; span: api.Span } ) => void` | Function called just before the span is ended. Useful for adding attributes. |
| `createHook` | `(functionName: FMember \| FPMember, info: { args: ArrayLike<unknown> }) => boolean` | Hook called before creating the span. If `false` is returned this and all the sibling calls will not be traced. |
| `endHook` | `( functionName: FMember \| FPMember, info: { args: ArrayLike<unknown>; span: api.Span } ) => void` | Function called just before the span is ended. Useful for adding attributes. |
| `requireParentSpan` | `boolean` | Require parent to create fs span, default when unset is `false`. |

## Useful links

Expand Down
35 changes: 23 additions & 12 deletions plugins/node/instrumentation-fs/src/instrumentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,7 @@ export default class FsInstrumentation extends InstrumentationBase<FS> {
): T {
const instrumentation = this;
return <any>function (this: any, ...args: any[]) {
if (isTracingSuppressed(api.context.active())) {
// Performance optimization. Avoid creating additional contexts and spans
// if we already know that the tracing is being suppressed.
if (!instrumentation._shouldTrace()) {
return original.apply(this, args);
}
if (
Expand Down Expand Up @@ -180,9 +178,7 @@ export default class FsInstrumentation extends InstrumentationBase<FS> {
): T {
const instrumentation = this;
return <any>function (this: any, ...args: any[]) {
if (isTracingSuppressed(api.context.active())) {
// Performance optimization. Avoid creating additional contexts and spans
// if we already know that the tracing is being suppressed.
if (!instrumentation._shouldTrace()) {
return original.apply(this, args);
}
if (
Expand Down Expand Up @@ -260,9 +256,7 @@ export default class FsInstrumentation extends InstrumentationBase<FS> {
>(functionName: FMember, original: T): T {
const instrumentation = this;
const patchedFunction = <any>function (this: any, ...args: any[]) {
if (isTracingSuppressed(api.context.active())) {
// Performance optimization. Avoid creating additional contexts and spans
// if we already know that the tracing is being suppressed.
if (!instrumentation._shouldTrace()) {
return original.apply(this, args);
}
if (
Expand Down Expand Up @@ -345,9 +339,7 @@ export default class FsInstrumentation extends InstrumentationBase<FS> {
): T {
const instrumentation = this;
return <any>async function (this: any, ...args: any[]) {
if (isTracingSuppressed(api.context.active())) {
// Performance optimization. Avoid creating additional contexts and spans
// if we already know that the tracing is being suppressed.
if (!instrumentation._shouldTrace()) {
return original.apply(this, args);
}
if (
Expand Down Expand Up @@ -415,4 +407,23 @@ export default class FsInstrumentation extends InstrumentationBase<FS> {
}
}
}

protected _shouldTrace(): boolean {
const activeContext = api.context.active();
if (isTracingSuppressed(activeContext)) {
// Performance optimization. Avoid creating additional contexts and spans
// if we already know that the tracing is being suppressed.
return false;
}

const { requireParentSpan } = this.getConfig() as FsInstrumentationConfig;
if (requireParentSpan) {
const parentSpan = api.trace.getSpan(activeContext);
if (parentSpan === undefined) {
return false;
}
}

return true;
}
}
1 change: 1 addition & 0 deletions plugins/node/instrumentation-fs/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,5 @@ export type EndHook = (
export interface FsInstrumentationConfig extends InstrumentationConfig {
createHook?: CreateHook;
endHook?: EndHook;
requireParentSpan?: boolean;
}
137 changes: 137 additions & 0 deletions plugins/node/instrumentation-fs/test/parent.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
import {
BasicTracerProvider,
InMemorySpanExporter,
SimpleSpanProcessor,
} from '@opentelemetry/sdk-trace-base';
import Instrumentation from '../src';
import * as assert from 'assert';
import type * as FSType from 'fs';
import type { FsInstrumentationConfig } from '../src/types';
import * as api from '@opentelemetry/api';
import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';

const supportsPromises = parseInt(process.versions.node.split('.')[0], 10) > 8;

const provider = new BasicTracerProvider();
const memoryExporter = new InMemorySpanExporter();
provider.addSpanProcessor(new SimpleSpanProcessor(memoryExporter));

const tracer = provider.getTracer('default');

describe('fs instrumentation: requireParentSpan', () => {
let plugin: Instrumentation;
let fs: typeof FSType;
let context: api.Context;
let endRootSpan: () => void;
let additionalSpans: number;

const initializePlugin = (pluginConfig: FsInstrumentationConfig) => {
plugin = new Instrumentation();
plugin.setTracerProvider(provider);
plugin.setConfig(pluginConfig);
plugin.enable();
fs = require('fs');
assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
}

beforeEach(() => {
const contextManager = new AsyncHooksContextManager();
api.context.setGlobalContextManager(contextManager.enable());
})

afterEach(() => {
plugin.disable();
memoryExporter.reset();
});

const createSpanTests = (shouldCreateSpan: boolean) => {
const prefix = shouldCreateSpan ? "creates" : "does not create"
const expectedSpans = () => (shouldCreateSpan ? 1 : 0) + additionalSpans

it(`${prefix} a span with the callback API`, async () => {
await new Promise<void>((resolve) => {
api.context.with(context, () => {
fs.access('./test/fixtures/readtest', fs.constants.R_OK, () => resolve())
})
}).then(() => endRootSpan())

assert.deepEqual(memoryExporter.getFinishedSpans().length, expectedSpans());
})

it(`${prefix} a span with the synchronous API`, () => {
api.context.with(context, () => {
fs.accessSync('./test/fixtures/readtest', fs.constants.R_OK)
endRootSpan()
})

assert.deepEqual(memoryExporter.getFinishedSpans().length, expectedSpans());
})

if (supportsPromises) {
it(`${prefix} a span with the promises API`, async () => {
await new Promise<void>((resolve) => {
api.context.with(context, () => {
fs.promises.access('./test/fixtures/readtest', fs.constants.R_OK)
.finally(() => resolve(endRootSpan()))
})
})

assert.deepEqual(memoryExporter.getFinishedSpans().length, expectedSpans());
})
}
}

const withRootSpan = (fn: () => void) => {
describe('with a root span', () => {
beforeEach(() => {
const rootSpan = tracer.startSpan("root span")

context = api.trace.setSpan(api.context.active(), rootSpan)
endRootSpan = () => rootSpan.end()
additionalSpans = 1
})

fn()
})
}

const withoutRootSpan = (fn: () => void) => {
describe('without a root span', () => {
beforeEach(() => {
context = api.context.active()
endRootSpan = () => {}
additionalSpans = 0
})

fn()
})
}

describe('requireParentSpan enabled', () => {
beforeEach(() => {
initializePlugin({requireParentSpan: true})
})

withRootSpan(() => {
createSpanTests(true)
})

withoutRootSpan(() => {
createSpanTests(false)
})
})

describe('requireParentSpan disabled', () => {
beforeEach(() => {
initializePlugin({requireParentSpan: false})
})

withRootSpan(() => {
createSpanTests(true)
})

withoutRootSpan(() => {
createSpanTests(true)
})
})
})

0 comments on commit e408510

Please sign in to comment.