Skip to content

Commit

Permalink
only de-proxy in wrapExport
Browse files Browse the repository at this point in the history
  • Loading branch information
jsumners-nr committed Apr 26, 2024
1 parent 161cc0e commit e3a4227
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 19 deletions.
28 changes: 28 additions & 0 deletions lib/is-absolute-path.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright 2024 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

'use strict'

/**
* Determines if a given string represents an absolute path to a module.
*
* @param {string} target Path to a module.
*
* @returns {boolean} True if it is an absolute path.
*/
module.exports = function isAbsolutePath(target) {
const leadChar = target.slice(0, 1)
if (leadChar !== '.' && leadChar !== '/') {
return false
}

const suffix = target.slice(-4)
/* eslint-disable-next-line */
if (suffix.slice(-3) !== '.js' && suffix !== '.cjs' && suffix !== '.mjs') {
return false
}

return true
}
14 changes: 7 additions & 7 deletions lib/shim/shim.js
Original file line number Diff line number Diff line change
Expand Up @@ -317,13 +317,6 @@ function wrap(nodule, properties, spec, args) {
})
}

if (nodule[symbols.nrEsmProxy] === true) {
// A CJS module has been imported as ESM through import-in-the-middle. This
// means that `nodule` is set to an instance of our proxy. What we actually
// want is the thing to be instrumented.
nodule = nodule[symbols.nrEsmNodule]
}

// If we're just wrapping one thing, just wrap it and return.
if (properties == null) {
const name = this.getName(nodule)
Expand Down Expand Up @@ -570,6 +563,13 @@ function wrapClass(nodule, properties, spec, args) {
* @returns {*} The return value from `spec`.
*/
function wrapExport(nodule, spec) {
if (nodule[symbols.nrEsmProxy] === true) {
// A CJS module has been imported as ESM through import-in-the-middle. This
// means that `nodule` is set to an instance of our proxy. What we actually
// want is the thing to be instrumented. We assume it is the "default"
// export.
nodule = nodule.default
}
return (this._toExport = this.wrap(nodule, null, spec))
}

Expand Down
12 changes: 4 additions & 8 deletions lib/shimmer.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ const INSTRUMENTATIONS = require('./instrumentations')()
const shims = require('./shim')
const { Hook } = require('@newrelic/ritm')
const IitmHook = require('import-in-the-middle')
const { nrEsmProxy, nrEsmNodule } = require('./symbols')
const { nrEsmProxy } = require('./symbols')
const isAbsolutePath = require('./is-absolute-path')
const InstrumentationDescriptor = require('./instrumentation-descriptor')
const InstrumentationTracker = require('./instrumentation-tracker')
let pkgsToHook = []
Expand Down Expand Up @@ -463,8 +464,7 @@ const shimmer = (module.exports = {
moduleName = 'pg'
}

// TODO: implement a better test for an absolute path ~ jsumners
if (/\.(js|cjs|mjs)$/.test(moduleName) === true) {
if (isAbsolutePath(moduleName) === true) {
// moduleName is an absolute path to a module. So we need to look up
// the simple name from the registered instrumentations.
return this.registeredInstrumentations.simpleNameFromPath(moduleName)
Expand Down Expand Up @@ -635,9 +635,6 @@ function resolveNodule({ nodule, instrumentation, esmResolver }) {
if (name === nrEsmProxy) {
return true
}
if (name === nrEsmNodule) {
return target.origNodule
}
if (target.noduleDefault[name]) {
return target.noduleDefault[name]
}
Expand Down Expand Up @@ -743,15 +740,14 @@ function _postLoad(agent, nodule, name, resolvedName, esmResolver) {

// Check if this is a known instrumentation and then run it.
if (hasPostLoadInstrumentation) {
if (resolvedName === undefined && /\.(js|cjs|mjs)$/.test(name) === true) {
if (resolvedName === undefined && isAbsolutePath(name) === true) {
// `resolvedName` comes from the `basedir` returned by the `Hook`
// function from import-in-the-middle or require-in-the-middle. At least
// with IITM, if the path string does not include a `node_modules` then
// `basedir` will be `undefined`. But we need it for our instrumentation
// to work. We'll only reach this situation if the module being
// instrumented has an `absolutePath` defined. So we detect that and
// assign appropriately.
// TODO: implement a better test for an absolute path ~ jsumners
resolvedName = name
}
shimmer.registeredInstrumentations.setResolvedName(simpleName, resolvedName)
Expand Down
5 changes: 1 addition & 4 deletions lib/symbols.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,5 @@ module.exports = {
// Used to "decorate" the proxy returned during ESM importing so that
// instrumentations can determine if the module looks like an ESM module
// or not.
nrEsmProxy: Symbol('nr-esm-proxy'),
// Represents the "module" as returned by import-in-the-middle. Allows for
// retrieving the full module from our wrapping proxy.
nrEsmNodule: Symbol('nr-esm-nodule')
nrEsmProxy: Symbol('nr-esm-proxy')
}

0 comments on commit e3a4227

Please sign in to comment.