Skip to content

Commit 7895306

Browse files
authored
fix(instr-aws-sdk): @smithy/middleware-stack@2.1.0 change broke aws-sdk-v3 instrumentation (#1913)
As of smithy-lang/smithy-typescript#1146 (details at smithy-lang/smithy-typescript#1113) the CommonJS export for many (all?) `@smithy/*` packages is now an esbuild bundle -- all in `dist-cjs/index.js`. That means that subfile patching like this no longer works: ```js const v3SmithyMiddlewareStackFile = new InstrumentationNodeModuleFile( '@smithy/middleware-stack/dist-cjs/MiddlewareStack.js', ['>=1.0.1'], this.patchV3ConstructStack.bind(this), this.unpatchV3ConstructStack.bind(this) ); ``` In our case this breaks as of `@smithy/middleware-stack@2.1.0` released 2024-01-17T22:26:42.432Z. This is considered a non-breaking change, so the dependency ranges for earlier released versions of `@smithy/smithy-client` will pick this up. A fix is to change the `@smithy/middleware-stack` patching to be on the top-level module exports. Because the `constructStack` field is only available as a getter we cannot use shimmer (InstrumentationBase._wrap). Instead this returns a new `moduleExports` object with a new getter that shims the call This PR also updates .tav.yml to reduce the number of aws-sdk package versions tested.
1 parent 84e1a6b commit 7895306

File tree

3 files changed

+174
-13
lines changed

3 files changed

+174
-13
lines changed
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
"aws-sdk":
22
# A small subset of releases in the range [2.308.0, 3) to reduce testing time.
3-
versions: "2.308.0 || 2.548.0 || 2.785.0 || 2.1025.0 || 2.1265.0 || 2.1506.0 || >=2.1508.0"
3+
versions: "2.308.0 || 2.556.0 || 2.801.0 || 2.1049.0 || 2.1297.0 || 2.1546.0 || >=2.1548.0"
44
commands:
55
- npm run test
66

77
"@aws-sdk/client-s3":
88
# A small subset of releases in the range [3.6.1, 4) to reduce testing time.
99
# - 3.377.0 was a bad release (see issue #1828).
10-
versions: "3.6.1 || 3.53.0 || 3.163.0 || 3.266.0 || 3.354.0 || 3.458.0 || >=3.462.0"
10+
versions: "3.6.1 || 3.55.0 || 3.180.0 || 3.289.0 || 3.385.0 || 3.498.0 || >=3.503.1"
1111
commands:
1212
- npm run test
1313

1414
"@aws-sdk/client-sqs":
1515
# A small subset of releases in the range [3.24.0, 4) to reduce testing time.
16-
versions: "3.24.0 || 3.85.0 || 3.194.0 || 3.278.0 || 3.357.0 || 3.461.0 || >=3.462.0"
16+
versions: "3.24.0 || 3.94.0 || 3.202.0 || 3.296.0 || 3.388.0 || 3.496.0 || >=3.503.1"
1717
commands:
1818
- npm run test

plugins/node/opentelemetry-instrumentation-aws-sdk/src/aws-sdk.ts

+16-10
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import {
5656
normalizeV3Request,
5757
removeSuffixFromStringIfExists,
5858
} from './utils';
59+
import { propwrap } from './propwrap';
5960
import { RequestMetadata } from './services/ServiceExtension';
6061
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
6162

@@ -112,19 +113,24 @@ export class AwsInstrumentation extends InstrumentationBase<any> {
112113
v3MiddlewareStackFileNewVersions,
113114
]);
114115

115-
// patch for @smithy/middleware-stack for aws-sdk packages v3.363.0+
116-
const v3SmithyMiddlewareStackFile = new InstrumentationNodeModuleFile(
117-
'@smithy/middleware-stack/dist-cjs/MiddlewareStack.js',
118-
['>=1.0.1'],
119-
this.patchV3ConstructStack.bind(this),
120-
this.unpatchV3ConstructStack.bind(this)
121-
);
116+
// Patch for @smithy/middleware-stack for @aws-sdk/* packages v3.363.0+.
117+
// As of @smithy/middleware-stack@2.1.0 `constructStack` is only available
118+
// as a getter, so we cannot use `this._wrap()`.
119+
const self = this;
122120
const v3SmithyMiddlewareStack = new InstrumentationNodeModuleDefinition(
123121
'@smithy/middleware-stack',
124122
['>=2.0.0'],
125-
undefined,
126-
undefined,
127-
[v3SmithyMiddlewareStackFile]
123+
(moduleExports, moduleVersion) => {
124+
const newExports = propwrap(
125+
moduleExports,
126+
'constructStack',
127+
(orig: any) => {
128+
self._diag.debug('propwrapping aws-sdk v3 constructStack');
129+
return self._getV3ConstructStackPatch(moduleVersion, orig);
130+
}
131+
);
132+
return newExports;
133+
}
128134
);
129135

130136
const v3SmithyClient = new InstrumentationNodeModuleDefinition<typeof AWS>(
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
/*
18+
* This block is derived from esbuild's bundling support.
19+
* https://github.com/evanw/esbuild/blob/v0.14.42/internal/runtime/runtime.go#L22
20+
*
21+
* License:
22+
* MIT License
23+
*
24+
* Copyright (c) 2020 Evan Wallace
25+
*
26+
* Permission is hereby granted, free of charge, to any person obtaining a copy
27+
* of this software and associated documentation files (the "Software"), to deal
28+
* in the Software without restriction, including without limitation the rights
29+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
30+
* copies of the Software, and to permit persons to whom the Software is
31+
* furnished to do so, subject to the following conditions:
32+
*
33+
* The above copyright notice and this permission notice shall be included in all
34+
* copies or substantial portions of the Software.
35+
*
36+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
37+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
38+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
39+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
40+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
41+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
42+
* SOFTWARE.
43+
*/
44+
const __defProp = Object.defineProperty;
45+
const __getOwnPropDesc = Object.getOwnPropertyDescriptor;
46+
const __hasOwnProp = Object.prototype.hasOwnProperty;
47+
const __getOwnPropNames = Object.getOwnPropertyNames;
48+
const __copyProps = (
49+
to: any,
50+
from: any,
51+
except: string,
52+
desc?: PropertyDescriptor | undefined
53+
) => {
54+
if ((from && typeof from === 'object') || typeof from === 'function') {
55+
for (const key of __getOwnPropNames(from)) {
56+
if (!__hasOwnProp.call(to, key) && key !== except) {
57+
__defProp(to, key, {
58+
get: () => from[key] as any,
59+
enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable,
60+
});
61+
}
62+
}
63+
}
64+
return to;
65+
};
66+
67+
/**
68+
* Return a new object that is a copy of `obj`, with its `subpath` property
69+
* replaced with the return value of `wrapper(original)`.
70+
*
71+
* This is similar to shimmer (i.e. `InstrumentationBase.prototype._wrap`).
72+
* However, it uses a different technique to support wrapping properties that
73+
* are only available via a getter (i.e. their property descriptor is `.writable
74+
* === false`).
75+
*
76+
* For example:
77+
* var os = propwrap(require('os'), 'platform', (orig) => {
78+
* return function wrappedPlatform () {
79+
* return orig().toUpperCase()
80+
* }
81+
* })
82+
* console.log(os.platform()) // => DARWIN
83+
*
84+
* The subpath can indicate a nested property. Each property in that subpath,
85+
* except the last, must identify an *Object*.
86+
*
87+
* Limitations:
88+
* - This doesn't handle possible Symbol properties on the copied object(s).
89+
* - This cannot wrap a property of a function, because we cannot create a
90+
* copy of the function.
91+
*
92+
* @param {object} obj
93+
* @param {string} subpath - The property subpath on `obj` to wrap. This may
94+
* point to a nested property by using a '.' to separate levels. For example:
95+
* var fs = wrap(fs, 'promises.sync', (orig) => { ... })
96+
* @param {Function} wrapper - A function of the form `function (orig)`, where
97+
* `orig` is the original property value. This must synchronously return the
98+
* new property value.
99+
* @returns {object} A new object with the wrapped property.
100+
* @throws {TypeError} if the subpath points to a non-existant property, or if
101+
* any but the last subpath part points to a non-Object.
102+
*/
103+
export const propwrap = (obj: any, subpath: string, wrapper: Function): any => {
104+
const parts = subpath.split('.');
105+
const namespaces = [obj];
106+
let namespace = obj;
107+
let key;
108+
let val;
109+
110+
// 1. Traverse the subpath parts to sanity check and get references to the
111+
// Objects that we will be copying.
112+
for (let i = 0; i < parts.length; i++) {
113+
key = parts[i];
114+
val = namespace[key];
115+
if (!val) {
116+
throw new TypeError(
117+
`cannot wrap "${subpath}": "<obj>.${parts
118+
.slice(0, i)
119+
.join('.')}" is ${typeof val}`
120+
);
121+
} else if (i < parts.length - 1) {
122+
if (typeof val !== 'object') {
123+
throw new TypeError(
124+
`cannot wrap "${subpath}": "<obj>.${parts
125+
.slice(0, i)
126+
.join('.')}" is not an Object`
127+
);
128+
}
129+
namespace = val;
130+
namespaces.push(namespace);
131+
}
132+
}
133+
134+
// 2. Now work backwards, wrapping each namespace with a new object that has a
135+
// copy of all the properties, except the one that we've wrapped.
136+
for (let i = parts.length - 1; i >= 0; i--) {
137+
key = parts[i];
138+
namespace = namespaces[i];
139+
if (i === parts.length - 1) {
140+
const orig = namespace[key];
141+
val = wrapper(orig);
142+
} else {
143+
val = namespaces[i + 1];
144+
}
145+
const desc = __getOwnPropDesc(namespace, key);
146+
const wrappedNamespace = __defProp({}, key, {
147+
value: val,
148+
enumerable: !desc || desc.enumerable,
149+
});
150+
__copyProps(wrappedNamespace, namespace, key);
151+
namespaces[i] = wrappedNamespace;
152+
}
153+
154+
return namespaces[0];
155+
};

0 commit comments

Comments
 (0)