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 70c2dd9
Show file tree
Hide file tree
Showing 4 changed files with 191 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;
}
164 changes: 164 additions & 0 deletions plugins/node/instrumentation-fs/test/parent.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
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 70c2dd9

Please sign in to comment.