Skip to content

Commit

Permalink
fix(instrumentation-fs): allow realpath.native and realpathSync.native (
Browse files Browse the repository at this point in the history 
#1332)

* test(instrumentation-fs): add failing test for bug

* fix(instrumentation-fs): allow .native fs funcs

* refactor(instrumentation-fs): indexFs

change indexFs return types and object properties of return

* refactor(instr-fs): use string for two level fns

previously ['realpath', 'native'] now 'realpath.native'

also moved exported yet private functions to utils file

fixed tests to follow these changes

refactored patching code to function with added comment

* refactor(instrumentation-fs): remove memberToDisplayName

* refactor(instrumentation-fs): simplify splitTwoLevels signature

* fix(instrumentation-fs): lint

* fix(instrumentation-fs): lint (typeof fs)

* fix(instrumentation-fs): test types

* chore: fix compile and lint

* fix(fs): exclude undefined in generic type

* test(fs): skip fs functions in node14 which were added later

---------

Co-authored-by: Amir Blum <[email protected]>
Co-authored-by: Amir Blum <[email protected]>
  • Loading branch information
@JakubKoralewski @blumamir
3 people authored Feb 16, 2023
1 parent 3484b75 commit ee0a59a
Show file tree
Hide file tree
Showing 6 changed files with 230 additions and 41 deletions.
2 changes: 2 additions & 0 deletions plugins/node/instrumentation-fs/src/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export const CALLBACK_FUNCTIONS: FMember[] = [
'readFile',
'readlink',
'realpath',
'realpath.native',
'rename',
'rm', // added in v14
'rmdir',
Expand Down Expand Up @@ -111,6 +112,7 @@ export const SYNC_FUNCTIONS: FMember[] = [
'readFileSync',
'readlinkSync',
'realpathSync',
'realpathSync.native',
'renameSync',
'rmdirSync',
'rmSync', // added in v14
Expand Down
65 changes: 44 additions & 21 deletions plugins/node/instrumentation-fs/src/instrumentation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,20 @@ import type {
FsInstrumentationConfig,
} from './types';
import { promisify } from 'util';
import { indexFs } from './utils';

type FS = typeof fs;

/**
* This is important for 2-level functions like `realpath.native` to retain the 2nd-level
* when patching the 1st-level.
*/
function patchedFunctionWithOriginalProperties<
T extends (...args: any[]) => ReturnType<T>
>(patchedFunction: T, original: T): T {
return Object.assign(patchedFunction, original);
}

export default class FsInstrumentation extends InstrumentationBase<FS> {
constructor(config?: FsInstrumentationConfig) {
super('@opentelemetry/instrumentation-fs', VERSION, config);
Expand All @@ -52,32 +63,35 @@ export default class FsInstrumentation extends InstrumentationBase<FS> {
(fs: FS) => {
this._diag.debug('Applying patch for fs');
for (const fName of SYNC_FUNCTIONS) {
if (isWrapped(fs[fName])) {
this._unwrap(fs, fName);
const { objectToPatch, functionNameToPatch } = indexFs(fs, fName);

if (isWrapped(objectToPatch[functionNameToPatch])) {
this._unwrap(objectToPatch, functionNameToPatch);
}
this._wrap(
fs,
fName,
objectToPatch,
functionNameToPatch,
<any>this._patchSyncFunction.bind(this, fName)
);
}
for (const fName of CALLBACK_FUNCTIONS) {
if (isWrapped(fs[fName])) {
this._unwrap(fs, fName);
const { objectToPatch, functionNameToPatch } = indexFs(fs, fName);
if (isWrapped(objectToPatch[functionNameToPatch])) {
this._unwrap(objectToPatch, functionNameToPatch);
}
if (fName === 'exists') {
// handling separately because of the inconsistent cb style:
// `exists` doesn't have error as the first argument, but the result
this._wrap(
fs,
fName,
objectToPatch,
functionNameToPatch,
<any>this._patchExistsCallbackFunction.bind(this, fName)
);
continue;
}
this._wrap(
fs,
fName,
objectToPatch,
functionNameToPatch,
<any>this._patchCallbackFunction.bind(this, fName)
);
}
Expand All @@ -97,13 +111,15 @@ export default class FsInstrumentation extends InstrumentationBase<FS> {
if (fs === undefined) return;
this._diag.debug('Removing patch for fs');
for (const fName of SYNC_FUNCTIONS) {
if (isWrapped(fs[fName])) {
this._unwrap(fs, fName);
const { objectToPatch, functionNameToPatch } = indexFs(fs, fName);
if (isWrapped(objectToPatch[functionNameToPatch])) {
this._unwrap(objectToPatch, functionNameToPatch);
}
}
for (const fName of CALLBACK_FUNCTIONS) {
if (isWrapped(fs[fName])) {
this._unwrap(fs, fName);
const { objectToPatch, functionNameToPatch } = indexFs(fs, fName);
if (isWrapped(objectToPatch[functionNameToPatch])) {
this._unwrap(objectToPatch, functionNameToPatch);
}
}
for (const fName of PROMISE_FUNCTIONS) {
Expand All @@ -121,7 +137,7 @@ export default class FsInstrumentation extends InstrumentationBase<FS> {
original: T
): T {
const instrumentation = this;
return <any>function (this: any, ...args: any[]) {
const patchedFunction = <any>function (this: any, ...args: any[]) {
const activeContext = api.context.active();

if (!instrumentation._shouldTrace(activeContext)) {
Expand Down Expand Up @@ -166,14 +182,15 @@ export default class FsInstrumentation extends InstrumentationBase<FS> {
span.end();
}
};
return patchedFunctionWithOriginalProperties(patchedFunction, original);
}

protected _patchCallbackFunction<T extends (...args: any[]) => ReturnType<T>>(
functionName: FMember,
original: T
): T {
const instrumentation = this;
return <any>function (this: any, ...args: any[]) {
const patchedFunction = <any>function (this: any, ...args: any[]) {
const activeContext = api.context.active();

if (!instrumentation._shouldTrace(activeContext)) {
Expand Down Expand Up @@ -247,11 +264,12 @@ export default class FsInstrumentation extends InstrumentationBase<FS> {
return original.apply(this, args);
}
};
return patchedFunctionWithOriginalProperties(patchedFunction, original);
}

protected _patchExistsCallbackFunction<
T extends (...args: any[]) => ReturnType<T>
>(functionName: FMember, original: T): T {
>(functionName: 'exists', original: T): T {
const instrumentation = this;
const patchedFunction = <any>function (this: any, ...args: any[]) {
const activeContext = api.context.active();
Expand Down Expand Up @@ -319,26 +337,30 @@ export default class FsInstrumentation extends InstrumentationBase<FS> {
return original.apply(this, args);
}
};
const functionWithOriginalProperties =
patchedFunctionWithOriginalProperties(patchedFunction, original);

// `exists` has a custom promisify function because of the inconsistent signature
// replicating that on the patched function
const promisified = function (path: unknown) {
return new Promise(resolve => patchedFunction(path, resolve));
return new Promise(resolve =>
functionWithOriginalProperties(path, resolve)
);
};
Object.defineProperty(promisified, 'name', { value: functionName });
Object.defineProperty(patchedFunction, promisify.custom, {
Object.defineProperty(functionWithOriginalProperties, promisify.custom, {
value: promisified,
});

return patchedFunction;
return functionWithOriginalProperties;
}

protected _patchPromiseFunction<T extends (...args: any[]) => ReturnType<T>>(
functionName: FPMember,
original: T
): T {
const instrumentation = this;
return <any>async function (this: any, ...args: any[]) {
const patchedFunction = <any>async function (this: any, ...args: any[]) {
const activeContext = api.context.active();

if (!instrumentation._shouldTrace(activeContext)) {
Expand Down Expand Up @@ -383,6 +405,7 @@ export default class FsInstrumentation extends InstrumentationBase<FS> {
span.end();
}
};
return patchedFunctionWithOriginalProperties(patchedFunction, original);
}

protected _runCreateHook(
Expand Down
37 changes: 32 additions & 5 deletions plugins/node/instrumentation-fs/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,40 @@ import type * as fs from 'fs';
import type * as api from '@opentelemetry/api';
import type { InstrumentationConfig } from '@opentelemetry/instrumentation';

export type FunctionPropertyNames<T> = {
[K in keyof T]: T[K] extends Function ? K : never;
}[keyof T];
export type FunctionPropertyNames<T> = Exclude<
{
[K in keyof T]: T[K] extends Function ? K : never;
}[keyof T],
undefined
>;
export type FunctionProperties<T> = Pick<T, FunctionPropertyNames<T>>;

export type FMember = FunctionPropertyNames<typeof fs>;
export type FPMember = FunctionPropertyNames<(typeof fs)['promises']>;
export type FunctionPropertyNamesTwoLevels<T> = Exclude<
{
[K in keyof T]: {
[L in keyof T[K]]: L extends string
? T[K][L] extends Function
? K extends string
? L extends string
? `${K}.${L}`
: never
: never
: never
: never;
}[keyof T[K]];
}[keyof T],
undefined
>;

export type Member<F> =
| FunctionPropertyNames<F>
| FunctionPropertyNamesTwoLevels<F>;
export type FMember =
| FunctionPropertyNames<typeof fs>
| FunctionPropertyNamesTwoLevels<typeof fs>;
export type FPMember =
| FunctionPropertyNames<(typeof fs)['promises']>
| FunctionPropertyNamesTwoLevels<(typeof fs)['promises']>;

export type CreateHook = (
functionName: FMember | FPMember,
Expand Down
53 changes: 53 additions & 0 deletions plugins/node/instrumentation-fs/src/utils.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* 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 type { FunctionPropertyNames, FMember } from './types';
import type * as fs from 'fs';
type FS = typeof fs;

export function splitTwoLevels<FSObject>(
functionName: FMember
):
| [FunctionPropertyNames<FSObject> & string]
| [FunctionPropertyNames<FSObject> & string, string] {
const memberParts = functionName.split('.');
if (memberParts.length > 1) {
if (memberParts.length !== 2)
throw Error(`Invalid member function name ${functionName}`);
return memberParts as [FunctionPropertyNames<FSObject> & string, string];
} else {
return [functionName as FunctionPropertyNames<FSObject> & string];
}
}

export function indexFs<FSObject extends FS | FS['promises']>(
fs: FSObject,
member: FMember
): { objectToPatch: any; functionNameToPatch: string } {
if (!member) throw new Error(JSON.stringify({ member }));
const splitResult = splitTwoLevels<FSObject>(member);
const [functionName1, functionName2] = splitResult;
if (functionName2) {
return {
objectToPatch: fs[functionName1],
functionNameToPatch: functionName2,
};
} else {
return {
objectToPatch: fs,
functionNameToPatch: functionName1,
};
}
}
20 changes: 16 additions & 4 deletions plugins/node/instrumentation-fs/test/definitions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,21 @@
import { FMember, FPMember } from '../src/types';
import * as fs from 'fs';

export type FsFunction = FPMember & FMember;
export type FsFunction = FMember;
export type Opts = {
sync?: boolean;
callback?: boolean;
promise?: boolean;
};
export type Result = { error?: RegExp; result?: any; resultAsError?: any };
export type Result = {
error?: RegExp;
result?: any;
resultAsError?: any;
hasPromiseVersion?: boolean;
};
export type TestCase = [FsFunction, any[], Result, any[], Opts?];
export type TestCreator = (
name: FsFunction,
export type TestCreator<Member extends FMember | FPMember> = (
name: Member,
args: any[],
result: Result,
spans: any[]
Expand Down Expand Up @@ -77,6 +82,13 @@ const tests: TestCase[] = [
{ resultAsError: true },
[{ name: 'fs %NAME' }],
],
['realpath', ['/./'], { result: '/' }, [{ name: 'fs %NAME' }]],
[
'realpath.native',
['/./'],
{ result: '/', hasPromiseVersion: false },
[{ name: 'fs %NAME' }],
],
];

export default tests;
Loading

0 comments on commit ee0a59a

Please sign in to comment.