Skip to content

Commit 10df79f

Browse files
david-lunatrentm
andcommitted
feat: add instrumentation for aws-sdk S3 client (#3287)
* feat: add instrumentation for aws-sdk S3 client --------- Co-authored-by: Trent Mick <[email protected]>
1 parent 0d9da33 commit 10df79f

File tree

12 files changed

+14172
-10590
lines changed

12 files changed

+14172
-10590
lines changed

.ci/tav.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
"// todo": "We want versions=['20','19','18','16','14','12','10','8'], but versions*modules needs to be <256 for the GH Actions jobs limit",
33
"versions": [ "20", "18", "16", "14", "12", "10", "8" ],
44
"modules": [
5+
"@aws-sdk/client-s3",
56
"@elastic/elasticsearch",
67
"@elastic/elasticsearch-canary",
78
"@hapi/hapi",

.tav.yml

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,8 @@ redis:
7575
name: redis
7676
versions: '>=4.0.0 <5.0.0'
7777
commands:
78-
- node test/instrumentation/modules/redis.test.js
79-
- node test/instrumentation/modules/redis4-legacy.test.js
78+
- node test/instrumentation/modules/redis.test.js
79+
- node test/instrumentation/modules/redis4-legacy.test.js
8080

8181
# We want these version ranges:
8282
# # v3.1.3 is broken in older versions of Node because of https://github.com/luin/ioredis/commit/d5867f7c7f03a770a8c0ca5680fdcbfcaf8488e7
@@ -537,6 +537,22 @@ aws-sdk:
537537
- node test/instrumentation/modules/aws-sdk/sqs.test.js
538538
- node test/instrumentation/modules/aws-sdk/dynamodb.test.js
539539

540+
'@aws-sdk/client-s3':
541+
# We want this version range:
542+
# versions: '>=3 <4'
543+
# However, @awk-sdk/client-s3 releases *very* frequently (almost every day) and there
544+
# is no need to test *all* those releases. Instead we statically list a subset
545+
# of versions to test.
546+
#
547+
# Maintenance note: This should be updated periodically using:
548+
# ./dev-utils/aws-sdk-s3-client-tav-versions.sh
549+
#
550+
# Test v3.0.0, every N=41 of 210 releases, and current latest
551+
versions: '3.0.0 || 3.36.0 || 3.86.0 || 3.171.0 || 3.245.0 || 3.315.0 || 3.321.1 || >3.321.1 <4'
552+
commands:
553+
- node test/instrumentation/modules/@aws-sdk/client-s3.test.js
554+
node: '>=14'
555+
540556
# - [email protected] added its diagnostics_channel support.
541557
# - In [email protected] the `request.origin` property was added, which we need
542558
# in the 'undici:request:create' diagnostic message.

CHANGELOG.asciidoc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ Notes:
4040
[float]
4141
===== Features
4242
43+
* Add support for @aws-sdk/client-s3 ({pull}3287[#3287])
44+
4345
* Add <<capture-body>> support for Fastify instrumentation.
4446
Contributed by @xxzefgh. ({pull}2681[#2681])
4547
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#!/bin/sh
2+
#
3+
# Calculate and emit the "versions:" block of ".tav.yml" for aws-sdk.
4+
# This will include:
5+
# - the first supported release (2.858.0)
6+
# - the latest current release
7+
# - and ~5 releases in between
8+
9+
npm info -j @aws-sdk/client-s3 | node -e '
10+
var semver = require("semver");
11+
var chunks = [];
12+
process.stdin
13+
.resume()
14+
.on("data", (chunk) => { chunks.push(chunk) })
15+
.on("end", () => {
16+
var input = JSON.parse(chunks.join(""));
17+
var vers = input.versions.filter(v => semver.satisfies(v, ">=3 <4"));
18+
var modulus = Math.floor((vers.length - 2) / 5);
19+
console.log(" # Test v3.0.0, every N=%d of %d releases, and current latest.", modulus, vers.length);
20+
vers = vers.filter((v, idx, arr) => idx % modulus === 0 || idx === arr.length - 1);
21+
console.log(" versions: '\''%s || >%s <4'\''", vers.join(" || "), vers[vers.length-1])
22+
})
23+
'

docs/supported-technologies.asciidoc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ The Node.js agent will automatically instrument the following modules to give yo
139139
|=======================================================================
140140
|Module |Version |Note
141141
|https://www.npmjs.com/package/aws-sdk[aws-sdk] |>1 <3 |Will instrument SQS send/receive/delete messages, all S3 methods, all DynamoDB methods, and the SNS publish method
142+
|https://www.npmjs.com/package/@aws-sdk/client-s3[@aws-sdk/client-s3] |>=3 <4 |Will instrument all S3 methods
142143
|https://www.npmjs.com/package/cassandra-driver[cassandra-driver] |>=3.0.0 <5 |Will instrument all queries
143144
|https://www.npmjs.com/package/elasticsearch[elasticsearch] |>=8.0.0 |Will instrument all queries
144145
|https://www.npmjs.com/package/@elastic/elasticsearch[@elastic/elasticsearch] |>=7.0.0 <9.0.0 |Will instrument all queries

lib/instrumentation/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ const nodeSupportsAsyncLocalStorage = semver.satisfies(process.versions.node, '>
3131
const nodeHasInstrumentableFetch = typeof (global.fetch) === 'function'
3232

3333
var MODULES = [
34+
'@aws-sdk/smithy-client', // Instrument the base client which all AWS-SDK v3 clients extends
3435
['@elastic/elasticsearch', '@elastic/elasticsearch-canary'],
3536
'@node-redis/client/dist/lib/client',
3637
'@node-redis/client/dist/lib/client/commands-queue',
Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and other contributors where applicable.
3+
* Licensed under the BSD 2-Clause License; you may not use this file except in
4+
* compliance with the BSD 2-Clause License.
5+
*/
6+
7+
'use strict'
8+
9+
const constants = require('../../../constants')
10+
const NAME = 'S3'
11+
const TYPE = 'storage'
12+
const SUBTYPE = 's3'
13+
const elasticAPMStash = Symbol('elasticAPMStash')
14+
15+
/**
16+
* Gets the region from the ARN
17+
*
18+
* @param {String} s3Arn
19+
* @returns {String}
20+
*/
21+
function regionFromS3Arn (s3Arn) {
22+
return s3Arn.split(':')[3]
23+
}
24+
25+
/**
26+
* Return an APM "resource" string for the bucket, Access Point ARN, or Outpost
27+
* ARN. ARNs are normalized to a shorter resource name.
28+
* Known ARN patterns:
29+
* - arn:aws:s3:<region>:<account-id>:accesspoint/<accesspoint-name>
30+
* - arn:aws:s3-outposts:<region>:<account>:outpost/<outpost-id>/bucket/<bucket-name>
31+
* - arn:aws:s3-outposts:<region>:<account>:outpost/<outpost-id>/accesspoint/<accesspoint-name>
32+
*
33+
* In general that is:
34+
* arn:$partition:$service:$region:$accountId:$resource
35+
*
36+
* This parses using the same "split on colon" used by the JavaScript AWS SDK v3.
37+
* https://github.com/aws/aws-sdk-js-v3/blob/v3.18.0/packages/util-arn-parser/src/index.ts#L14-L37
38+
*
39+
* @param {String} bucket The bucket string
40+
* @returns {String | null}
41+
*/
42+
function resourceFromBucket (bucket) {
43+
let resource = null
44+
if (bucket) {
45+
resource = bucket
46+
if (resource.startsWith('arn:')) {
47+
resource = bucket.split(':').slice(5).join(':')
48+
}
49+
}
50+
return resource
51+
}
52+
53+
/**
54+
* Returns middlewares to instrument an S3Client instance
55+
*
56+
* @param {import('@aws-sdk/client-s3').S3Client} client
57+
* @param {any} agent
58+
* @returns {import('./smithy-client').AWSMiddlewareEntry[]}
59+
*/
60+
function s3MiddlewareFactory (client, agent) {
61+
return [
62+
{
63+
middleware: (next, context) => async (args) => {
64+
const input = args.input
65+
const bucket = input && input.Bucket
66+
const resource = resourceFromBucket(bucket)
67+
const span = agent._instrumentation.currSpan()
68+
69+
if (!span) {
70+
return await next(args)
71+
}
72+
// The given span comes with the operation name and we need to
73+
// add the resource if applies
74+
if (resource) {
75+
span.name += ' ' + resource
76+
span.setServiceTarget('s3', resource)
77+
}
78+
79+
// As for now OTel spec defines attributes for operations that require a Bucket
80+
// if that changes we should review this guard
81+
// https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/semantic_conventions/trace/instrumentation/aws-sdk.yml#L435
82+
if (bucket) {
83+
const otelAttrs = span._getOTelAttributes()
84+
85+
otelAttrs['aws.s3.bucket'] = bucket
86+
87+
if (input.Key) {
88+
otelAttrs['aws.s3.key'] = input.Key
89+
}
90+
}
91+
92+
let err
93+
let result
94+
let response
95+
let statusCode
96+
try {
97+
result = await next(args)
98+
response = result && result.response
99+
statusCode = response && response.statusCode
100+
} catch (ex) {
101+
// Save the error for use in `finally` below, but re-throw it to
102+
// not impact code flow.
103+
err = ex
104+
105+
// This code path happens with a GetObject conditional request
106+
// that returns a 304 Not Modified.
107+
statusCode = err && err.$metadata && err.$metadata.httpStatusCode
108+
throw ex
109+
} finally {
110+
if (statusCode) {
111+
span._setOutcomeFromHttpStatusCode(statusCode)
112+
} else {
113+
span._setOutcomeFromErrorCapture(constants.OUTCOME_FAILURE)
114+
}
115+
if (err && (!statusCode || statusCode >= 400)) {
116+
agent.captureError(err, { skipOutcome: true })
117+
}
118+
119+
// Set the httpContext
120+
if (statusCode) {
121+
const httpContext = {
122+
status_code: statusCode
123+
}
124+
125+
if (response && response.headers && response.headers['content-length']) {
126+
const encodedBodySize = Number(response.headers['content-length'])
127+
if (!isNaN(encodedBodySize)) {
128+
httpContext.response = { encoded_body_size: encodedBodySize }
129+
}
130+
}
131+
span.setHttpContext(httpContext)
132+
}
133+
134+
// Configuring `new S3Client({useArnRegion:true})` allows one to
135+
// use an Access Point bucket ARN for a region *other* than the
136+
// one for which the client is configured. Therefore, we attempt
137+
// to get the bucket region from the ARN first.
138+
const config = client.config
139+
let useArnRegion
140+
if (typeof config.useArnRegion === 'boolean') {
141+
useArnRegion = config.useArnRegion
142+
} else {
143+
useArnRegion = await config.useArnRegion()
144+
}
145+
146+
let region
147+
if (useArnRegion && bucket && bucket.startsWith('arn:')) {
148+
region = regionFromS3Arn(args.input.Bucket)
149+
} else {
150+
region = typeof config.region === 'boolean' ? region : await config.region()
151+
}
152+
153+
// Destination context.
154+
const destContext = {
155+
address: context[elasticAPMStash].hostname,
156+
port: context[elasticAPMStash].port,
157+
service: {
158+
name: SUBTYPE,
159+
type: TYPE
160+
}
161+
}
162+
if (resource) {
163+
destContext.service.resource = resource
164+
}
165+
166+
if (region) {
167+
destContext.cloud = { region }
168+
}
169+
span._setDestinationContext(destContext)
170+
171+
span.end()
172+
}
173+
174+
return result
175+
},
176+
options: { step: 'initialize', priority: 'high', name: 'elasticAPMSpan' }
177+
},
178+
{
179+
middleware: (next, context) => async (args) => {
180+
const req = args.request
181+
let port = req.port
182+
183+
// Resolve port for HTTP(S) protocols
184+
if (port === undefined) {
185+
if (req.protocol === 'https:') {
186+
port = 443
187+
} else if (req.protocol === 'http:') {
188+
port = 80
189+
}
190+
}
191+
192+
context[elasticAPMStash] = {
193+
protocol: req.protocol,
194+
hostname: req.hostname,
195+
port: port
196+
}
197+
return next(args)
198+
},
199+
options: { step: 'finalizeRequest', name: 'elasticAPMHTTPInfo' }
200+
}
201+
]
202+
}
203+
204+
module.exports = {
205+
S3_NAME: NAME,
206+
S3_TYPE: TYPE,
207+
S3_SUBTYPE: SUBTYPE,
208+
s3MiddlewareFactory
209+
}

0 commit comments

Comments
 (0)