fix(instrumentation-fs): allow realpath.native and realpathSync.native

plugins/node/instrumentation-fs/src/constants.ts

@@ -67,6 +67,7 @@ export const CALLBACK_FUNCTIONS: FMember[] = [
   'readFile',
   'readlink',
   'realpath',
+  ['realpath', 'native'],
   'rename',
   'rm', // added in v14
   'rmdir',
@@ -111,6 +112,7 @@ export const SYNC_FUNCTIONS: FMember[] = [
   'readFileSync',
   'readlinkSync',
   'realpathSync',
+  ['realpathSync', 'native'],
   'renameSync',
   'rmdirSync',
   'rmSync', // added in v14

plugins/node/instrumentation-fs/src/instrumentation.ts

@@ -41,6 +41,23 @@ type FS = typeof fs;
 
 const supportsPromises = parseInt(process.versions.node.split('.')[0], 10) > 8;
 
+export function indexFs(fs: FS, member: FMember): { fs: FS | any; fName: any } {
+  if (Array.isArray(member)) {
+    const [K, L] = member;
+    return { fs: fs[K], fName: L };
+  } else {
+    return { fs, fName: member };
+  }
+}
+export function memberToDisplayName(member: FMember): string {
+  if (Array.isArray(member)) {
+    const [K, L] = member;
+    return `${K}.${L}`;
+  } else {
+    return member;
+  }
+}
+
 export default class FsInstrumentation extends InstrumentationBase<FS> {
   constructor(config?: FsInstrumentationConfig) {
     super('@opentelemetry/instrumentation-fs', VERSION, config);
@@ -54,32 +71,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 { fs: fsToIndex, fName: fNameToIndex } = indexFs(fs, fName);
+
+            if (isWrapped(fsToIndex[fNameToIndex])) {
+              this._unwrap(fsToIndex, fNameToIndex);
             }
             this._wrap(
-              fs,
-              fName,
+              fsToIndex,
+              fNameToIndex,
               <any>this._patchSyncFunction.bind(this, fName)
             );
           }
           for (const fName of CALLBACK_FUNCTIONS) {
-            if (isWrapped(fs[fName])) {
-              this._unwrap(fs, fName);
+            const { fs: fsToIndex, fName: fNameToIndex } = indexFs(fs, fName);
+            if (isWrapped(fsToIndex[fNameToIndex])) {
+              this._unwrap(fsToIndex, fNameToIndex);
             }
             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,
+                fsToIndex,
+                fNameToIndex,
                 <any>this._patchExistsCallbackFunction.bind(this, fName)
               );
               continue;
             }
             this._wrap(
-              fs,
-              fName,
+              fsToIndex,
+              fNameToIndex,
               <any>this._patchCallbackFunction.bind(this, fName)
             );
           }
@@ -101,13 +121,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 { fs: fsToIndex, fName: fNameToIndex } = indexFs(fs, fName);
+            if (isWrapped(fsToIndex[fNameToIndex])) {
+              this._unwrap(fsToIndex, fNameToIndex);
             }
           }
           for (const fName of CALLBACK_FUNCTIONS) {
-            if (isWrapped(fs[fName])) {
-              this._unwrap(fs, fName);
+            const { fs: fsToIndex, fName: fNameToIndex } = indexFs(fs, fName);
+            if (isWrapped(fsToIndex[fNameToIndex])) {
+              this._unwrap(fsToIndex, fNameToIndex);
             }
           }
           if (supportsPromises) {
@@ -127,7 +149,8 @@ export default class FsInstrumentation extends InstrumentationBase<FS> {
     original: T
   ): T {
     const instrumentation = this;
-    return <any>function (this: any, ...args: any[]) {
+    functionName = memberToDisplayName(functionName) as FMember;
+    const rv = <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.
@@ -172,14 +195,18 @@ export default class FsInstrumentation extends InstrumentationBase<FS> {
         span.end();
       }
     };
+    Object.assign(rv, original);
+
+    return rv;
   }
 
   protected _patchCallbackFunction<T extends (...args: any[]) => ReturnType<T>>(
     functionName: FMember,
     original: T
   ): T {
     const instrumentation = this;
-    return <any>function (this: any, ...args: any[]) {
+    functionName = memberToDisplayName(functionName) as FMember;
+    const rv = <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.
@@ -253,6 +280,8 @@ export default class FsInstrumentation extends InstrumentationBase<FS> {
         return original.apply(this, args);
       }
     };
+    Object.assign(rv, original);
+    return rv;
   }
 
   protected _patchExistsCallbackFunction<

plugins/node/instrumentation-fs/src/types.ts

@@ -23,7 +23,19 @@ export type FunctionPropertyNames<T> = {
 }[keyof T];
 export type FunctionProperties<T> = Pick<T, FunctionPropertyNames<T>>;
 
-export type FMember = FunctionPropertyNames<typeof fs>;
+export type FunctionPropertyNamesTwoLevels<T> = {
+  [K in keyof T]: {
+    [L in keyof T[K]]: L extends string
+      ? T[K][L] extends Function
+        ? [K, L]
+        : never
+      : never;
+  }[keyof T[K]];
+}[keyof T];
+
+export type FMember =
+  | FunctionPropertyNames<typeof fs>
+  | FunctionPropertyNamesTwoLevels<typeof fs>;
 export type FPMember = FunctionPropertyNames<typeof fs['promises']>;
 
 export type CreateHook = (

plugins/node/instrumentation-fs/test/definitions.ts

@@ -14,16 +14,21 @@
  * limitations under the License.
  */
 
-import { FMember, FPMember } from '../src/types';
+import { FMember } 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,
@@ -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;

plugins/node/instrumentation-fs/test/index.test.ts

@@ -23,11 +23,12 @@ import {
 } from '@opentelemetry/sdk-trace-base';
 import * as assert from 'assert';
 import { promisify } from 'util';
-import Instrumentation from '../src';
+import Instrumentation, { indexFs, memberToDisplayName } from '../src';
 import * as sinon from 'sinon';
 import type * as FSType from 'fs';
 import tests, { TestCase, TestCreator } from './definitions';
 import type { FMember, FPMember, CreateHook, EndHook } from '../src/types';
+import { SYNC_FUNCTIONS } from '../src/constants';
 
 const supportsPromises = parseInt(process.versions.node.split('.')[0], 10) > 8;
 
@@ -38,7 +39,7 @@ const createHook = <CreateHook>sinon.spy(
   (fnName: FMember | FPMember, { args, span }) => {
     // `ts-node`, which we use via `ts-mocha` also patches module loading and creates
     // a lot of unrelated spans. Filter those out.
-    if (['readFileSync', 'existsSync'].includes(fnName)) {
+    if (['readFileSync', 'existsSync'].includes(fnName as string)) {
       const filename = args[0];
       if (!/test\/fixtures/.test(filename)) {
         return false;
@@ -93,18 +94,30 @@ describe('fs instrumentation', () => {
     { error, result, resultAsError = null },
     spans
   ) => {
-    const syncName: FMember = `${name}Sync` as FMember;
-    const rootSpanName = `${syncName} test span`;
-    it(`${syncName} ${error ? 'error' : 'success'}`, () => {
+    let syncName: FMember;
+    let syncDisplayName: string;
+    if (Array.isArray(name)) {
+      syncName = [`${name[0]}Sync`, name[1]] as any as FMember;
+      syncDisplayName = `${syncName[0] as string}.${name[1]}`;
+    } else {
+      syncName = `${name}Sync` as FMember;
+      syncDisplayName = syncName as string;
+    }
+    const rootSpanName = `${syncDisplayName} test span`;
+    it(`${syncDisplayName} ${error ? 'error' : 'success'}`, () => {
+      const { fs: fsToIndex, fName: fNameToIndex } = indexFs(fs, syncName);
       const rootSpan = tracer.startSpan(rootSpanName);
 
       assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
       context.with(trace.setSpan(context.active(), rootSpan), () => {
         if (error) {
-          assert.throws(() => Reflect.apply(fs[syncName], fs, args), error);
+          assert.throws(
+            () => Reflect.apply(fsToIndex[fNameToIndex], fsToIndex, args),
+            error
+          );
         } else {
           assert.deepEqual(
-            Reflect.apply(fs[syncName], fs, args),
+            Reflect.apply(fsToIndex[fNameToIndex], fsToIndex, args),
             result ?? resultAsError
           );
         }
@@ -113,7 +126,7 @@ describe('fs instrumentation', () => {
 
       assertSpans(memoryExporter.getFinishedSpans(), [
         ...spans.map((s: any) => {
-          const spanName = s.name.replace(/%NAME/, syncName);
+          const spanName = s.name.replace(/%NAME/, syncDisplayName);
           const attributes = {
             ...(s.attributes ?? {}),
           };
@@ -128,21 +141,33 @@ describe('fs instrumentation', () => {
       ]);
     });
   };
+  const makeRootSpanName = (name: FMember): string => {
+    let rsn: string;
+    if (Array.isArray(name)) {
+      rsn = `${name[0]}.${name[1]}`;
+    } else {
+      rsn = `${name}`;
+    }
+    rsn = `${rsn} test span`;
+    return rsn;
+  };
 
   const callbackTest: TestCreator = (
     name: FMember,
     args,
     { error, result, resultAsError = null },
     spans
   ) => {
-    const rootSpanName = `${name} test span`;
-    it(`${name} ${error ? 'error' : 'success'}`, done => {
+    const rootSpanName = makeRootSpanName(name);
+    const displayName = memberToDisplayName(name) as FMember;
+    it(`${displayName} ${error ? 'error' : 'success'}`, done => {
+      const { fs: fsToIndex, fName: fNameToIndex } = indexFs(fs, name);
       const rootSpan = tracer.startSpan(rootSpanName);
 
       assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
 
       context.with(trace.setSpan(context.active(), rootSpan), () => {
-        (fs[name] as Function)(
+        (fsToIndex[fNameToIndex] as Function)(
           ...args,
           (actualError: any | undefined, actualResult: any) => {
             assert.strictEqual(trace.getSpan(context.active()), rootSpan);
@@ -176,7 +201,7 @@ describe('fs instrumentation', () => {
               }
               assertSpans(memoryExporter.getFinishedSpans(), [
                 ...spans.map((s: any) => {
-                  const spanName = s.name.replace(/%NAME/, name);
+                  const spanName = s.name.replace(/%NAME/, displayName);
                   const attributes = {
                     ...(s.attributes ?? {}),
                   };
@@ -202,10 +227,12 @@ describe('fs instrumentation', () => {
   const promiseTest: TestCreator = (
     name: FPMember,
     args,
-    { error, result, resultAsError = null },
+    { error, result, resultAsError = null, hasPromiseVersion = true },
     spans
   ) => {
-    const rootSpanName = `${name} test span`;
+    if (!hasPromiseVersion) return;
+    const rootSpanName = makeRootSpanName(name);
+    name = memberToDisplayName(name) as FPMember;
     it(`promises.${name} ${error ? 'error' : 'success'}`, async () => {
       const rootSpan = tracer.startSpan(rootSpanName);
 
@@ -260,6 +287,30 @@ describe('fs instrumentation', () => {
     });
   };
 
+  describe('Syncronous API native', () => {
+    beforeEach(() => {
+      plugin.enable();
+    });
+    it('should not remove fs functions', () => {
+      for (const fname of SYNC_FUNCTIONS) {
+        if (Array.isArray(fname)) {
+          const [K, L] = fname;
+          assert.strictEqual(
+            typeof (fs[K] as any)[L],
+            'function',
+            `fs.${K}.${L} is not a function`
+          );
+        } else {
+          assert.strictEqual(
+            typeof fs[fname],
+            'function',
+            `fs.${fname} is not a function`
+          );
+        }
+      }
+    });
+  });
+
   describe('Syncronous API', () => {
     const selection: TestCase[] = tests.filter(
       ([, , , , options = {}]) => options.sync !== false