Skip to content

Commit 6392a2e

Browse files
nhulstonnhulston
authored
[serverless] Add S3 Span Pointers (#4875)
* Add span pointer info on S3 `putObject`, `copyObject`, and `completeMultipartUpload` requests. * Unit tests * small improvement * Create `addSpanPointer()` so we don't have to export a context with 0s for trace+span id; add debug logs * Add integration test for completeMultipartUpload; update unit test * Rename to `addSpanPointers()` * Update comments and make getting eTag more reliable * Validate parameters before calling `generateS3PointerHash` * add unit tests * Rename var to `SPAN_LINK_POINTER_KIND`; standardize the hashing function. * Set the span link kind in the `addSpanPointer()` functions so that downstream callers don't have to worry about passing it. * Move constants to constants.js; move `generatePointerHash` to util.js
1 parent 2072a1f commit 6392a2e

11 files changed

Lines changed: 306 additions & 9 deletions

File tree

‎packages/datadog-plugin-aws-sdk/src/base.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ class BaseAwsSdkPlugin extends ClientPlugin {
9393
this.responseExtractDSMContext(operation, params, response.data ?? response, span)
9494
}
9595
this.addResponseTags(span, response)
96+
this.addSpanPointers(span, response)
9697
this.finish(span, response, response.error)
9798
})
9899
}
@@ -101,6 +102,10 @@ class BaseAwsSdkPlugin extends ClientPlugin {
101102
// implemented by subclasses, or not
102103
}
103104

105+
addSpanPointers (span, response) {
106+
// Optionally implemented by subclasses, for services where we're unable to inject trace context
107+
}
108+
104109
operationFromRequest (request) {
105110
// can be overriden by subclasses
106111
return this.operationName({

‎packages/datadog-plugin-aws-sdk/src/services/s3.js‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
'use strict'
22

33
const BaseAwsSdkPlugin = require('../base')
4+
const log = require('../../../dd-trace/src/log')
5+
const { generatePointerHash } = require('../../../dd-trace/src/util')
6+
const { S3_PTR_KIND, SPAN_POINTER_DIRECTION } = require('../../../dd-trace/src/constants')
47

58
class S3 extends BaseAwsSdkPlugin {
69
static get id () { return 's3' }
@@ -18,6 +21,37 @@ class S3 extends BaseAwsSdkPlugin {
1821
bucketname: params.Bucket
1922
})
2023
}
24+
25+
addSpanPointers (span, response) {
26+
const request = response?.request
27+
const operationName = request?.operation
28+
if (!['putObject', 'copyObject', 'completeMultipartUpload'].includes(operationName)) {
29+
// We don't create span links for other S3 operations.
30+
return
31+
}
32+
33+
// AWS v2: https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html
34+
// AWS v3: https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/client/s3/
35+
const bucketName = request?.params?.Bucket
36+
const objectKey = request?.params?.Key
37+
let eTag =
38+
response?.ETag || // v3 PutObject & CompleteMultipartUpload
39+
response?.CopyObjectResult?.ETag || // v3 CopyObject
40+
response?.data?.ETag || // v2 PutObject & CompleteMultipartUpload
41+
response?.data?.CopyObjectResult?.ETag // v2 CopyObject
42+
43+
if (!bucketName || !objectKey || !eTag) {
44+
log.debug('Unable to calculate span pointer hash because of missing parameters.')
45+
return
46+
}
47+
48+
// https://github.com/DataDog/dd-span-pointer-rules/blob/main/AWS/S3/Object/README.md
49+
if (eTag.startsWith('"') && eTag.endsWith('"')) {
50+
eTag = eTag.slice(1, -1)
51+
}
52+
const pointerHash = generatePointerHash([bucketName, objectKey, eTag])
53+
span.addSpanPointer(S3_PTR_KIND, SPAN_POINTER_DIRECTION.DOWNSTREAM, pointerHash)
54+
}
2155
}
2256

2357
module.exports = S3

‎packages/datadog-plugin-aws-sdk/test/s3.spec.js‎

Lines changed: 139 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const agent = require('../../dd-trace/test/plugins/agent')
44
const { setup } = require('./spec_helpers')
55
const axios = require('axios')
66
const { rawExpectedSchema } = require('./s3-naming')
7+
const { S3_PTR_KIND, SPAN_POINTER_DIRECTION } = require('../../dd-trace/src/constants')
78

89
const bucketName = 's3-bucket-name-test'
910

@@ -36,20 +37,19 @@ describe('Plugin', () => {
3637

3738
before(done => {
3839
AWS = require(`../../../versions/${s3ClientName}@${version}`).get()
40+
s3 = new AWS.S3({ endpoint: 'http://127.0.0.1:4566', s3ForcePathStyle: true, region: 'us-east-1' })
41+
42+
// Fix for LocationConstraint issue - only for SDK v2
43+
if (s3ClientName === 'aws-sdk') {
44+
s3.api.globalEndpoint = '127.0.0.1'
45+
}
3946

40-
s3 = new AWS.S3({ endpoint: 'http://127.0.0.1:4567', s3ForcePathStyle: true, region: 'us-east-1' })
4147
s3.createBucket({ Bucket: bucketName }, (err) => {
4248
if (err) return done(err)
4349
done()
4450
})
4551
})
4652

47-
after(done => {
48-
s3.deleteBucket({ Bucket: bucketName }, () => {
49-
done()
50-
})
51-
})
52-
5353
after(async () => {
5454
await resetLocalStackS3()
5555
return agent.close({ ritmReset: false })
@@ -74,6 +74,138 @@ describe('Plugin', () => {
7474
rawExpectedSchema.outbound
7575
)
7676

77+
describe('span pointers', () => {
78+
it('should add span pointer for putObject operation', (done) => {
79+
agent.use(traces => {
80+
try {
81+
const span = traces[0][0]
82+
const links = JSON.parse(span.meta?.['_dd.span_links'] || '[]')
83+
84+
expect(links).to.have.lengthOf(1)
85+
expect(links[0].attributes).to.deep.equal({
86+
'ptr.kind': S3_PTR_KIND,
87+
'ptr.dir': SPAN_POINTER_DIRECTION.DOWNSTREAM,
88+
'ptr.hash': '6d1a2fe194c6579187408f827f942be3',
89+
'link.kind': 'span-pointer'
90+
})
91+
done()
92+
} catch (error) {
93+
done(error)
94+
}
95+
}).catch(done)
96+
97+
s3.putObject({
98+
Bucket: bucketName,
99+
Key: 'test-key',
100+
Body: 'test body'
101+
}, (err) => {
102+
if (err) {
103+
done(err)
104+
}
105+
})
106+
})
107+
108+
it('should add span pointer for copyObject operation', (done) => {
109+
agent.use(traces => {
110+
try {
111+
const span = traces[0][0]
112+
const links = JSON.parse(span.meta?.['_dd.span_links'] || '[]')
113+
114+
expect(links).to.have.lengthOf(1)
115+
expect(links[0].attributes).to.deep.equal({
116+
'ptr.kind': S3_PTR_KIND,
117+
'ptr.dir': SPAN_POINTER_DIRECTION.DOWNSTREAM,
118+
'ptr.hash': '1542053ce6d393c424b1374bac1fc0c5',
119+
'link.kind': 'span-pointer'
120+
})
121+
done()
122+
} catch (error) {
123+
done(error)
124+
}
125+
}).catch(done)
126+
127+
s3.copyObject({
128+
Bucket: bucketName,
129+
Key: 'new-key',
130+
CopySource: `${bucketName}/test-key`
131+
}, (err) => {
132+
if (err) {
133+
done(err)
134+
}
135+
})
136+
})
137+
138+
it('should add span pointer for completeMultipartUpload operation', (done) => {
139+
// Create 5MiB+ buffers for parts
140+
const partSize = 5 * 1024 * 1024
141+
const part1Data = Buffer.alloc(partSize, 'a')
142+
const part2Data = Buffer.alloc(partSize, 'b')
143+
144+
// Start the multipart upload process
145+
s3.createMultipartUpload({
146+
Bucket: bucketName,
147+
Key: 'multipart-test'
148+
}, (err, multipartData) => {
149+
if (err) return done(err)
150+
151+
// Upload both parts in parallel
152+
Promise.all([
153+
new Promise((resolve, reject) => {
154+
s3.uploadPart({
155+
Bucket: bucketName,
156+
Key: 'multipart-test',
157+
PartNumber: 1,
158+
UploadId: multipartData.UploadId,
159+
Body: part1Data
160+
}, (err, data) => err ? reject(err) : resolve({ PartNumber: 1, ETag: data.ETag }))
161+
}),
162+
new Promise((resolve, reject) => {
163+
s3.uploadPart({
164+
Bucket: bucketName,
165+
Key: 'multipart-test',
166+
PartNumber: 2,
167+
UploadId: multipartData.UploadId,
168+
Body: part2Data
169+
}, (err, data) => err ? reject(err) : resolve({ PartNumber: 2, ETag: data.ETag }))
170+
})
171+
]).then(parts => {
172+
// Now complete the multipart upload
173+
const completeParams = {
174+
Bucket: bucketName,
175+
Key: 'multipart-test',
176+
UploadId: multipartData.UploadId,
177+
MultipartUpload: {
178+
Parts: parts
179+
}
180+
}
181+
182+
s3.completeMultipartUpload(completeParams, (err) => {
183+
if (err) done(err)
184+
agent.use(traces => {
185+
const span = traces[0][0]
186+
const operation = span.meta?.['aws.operation']
187+
if (operation === 'completeMultipartUpload') {
188+
try {
189+
const links = JSON.parse(span.meta?.['_dd.span_links'] || '[]')
190+
expect(links).to.have.lengthOf(1)
191+
expect(links[0].attributes).to.deep.equal({
192+
'ptr.kind': S3_PTR_KIND,
193+
'ptr.dir': SPAN_POINTER_DIRECTION.DOWNSTREAM,
194+
'ptr.hash': '422412aa6b472a7194f3e24f4b12b4a6',
195+
'link.kind': 'span-pointer'
196+
})
197+
done()
198+
} catch (error) {
199+
done(error)
200+
}
201+
}
202+
})
203+
})
204+
}).catch(done)
205+
})
206+
})
207+
})
208+
77209
it('should allow disabling a specific span kind of a service', (done) => {
78210
let total = 0
79211

‎packages/dd-trace/src/constants.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,10 @@ module.exports = {
4646
SCHEMA_OPERATION: 'schema.operation',
4747
SCHEMA_NAME: 'schema.name',
4848
GRPC_CLIENT_ERROR_STATUSES: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16],
49-
GRPC_SERVER_ERROR_STATUSES: [2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]
49+
GRPC_SERVER_ERROR_STATUSES: [2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16],
50+
S3_PTR_KIND: 'aws.s3.object',
51+
SPAN_POINTER_DIRECTION: Object.freeze({
52+
UPSTREAM: 'u',
53+
DOWNSTREAM: 'd'
54+
})
5055
}

‎packages/dd-trace/src/noop/span.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ class NoopSpan {
2222
setTag (key, value) { return this }
2323
addTags (keyValueMap) { return this }
2424
addLink (link) { return this }
25+
addSpanPointer (ptrKind, ptrDir, ptrHash) { return this }
2526
log () { return this }
2627
logEvent () {}
2728
finish (finishTime) {}

‎packages/dd-trace/src/opentelemetry/span.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ const { SERVICE_NAME, RESOURCE_NAME } = require('../../../../ext/tags')
1414
const kinds = require('../../../../ext/kinds')
1515

1616
const SpanContext = require('./span_context')
17+
const id = require('../id')
1718

1819
// The one built into OTel rounds so we lose sub-millisecond precision.
1920
function hrTimeToMilliseconds (time) {
@@ -217,6 +218,20 @@ class Span {
217218
return this
218219
}
219220

221+
addSpanPointer (ptrKind, ptrDir, ptrHash) {
222+
const zeroContext = new SpanContext({
223+
traceId: id('0'),
224+
spanId: id('0')
225+
})
226+
const attributes = {
227+
'ptr.kind': ptrKind,
228+
'ptr.dir': ptrDir,
229+
'ptr.hash': ptrHash,
230+
'link.kind': 'span-pointer'
231+
}
232+
return this.addLink(zeroContext, attributes)
233+
}
234+
220235
setStatus ({ code, message }) {
221236
if (!this.ended && !this._hasStatus && code) {
222237
this._hasStatus = true

‎packages/dd-trace/src/opentracing/span.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,20 @@ class DatadogSpan {
180180
})
181181
}
182182

183+
addSpanPointer (ptrKind, ptrDir, ptrHash) {
184+
const zeroContext = new SpanContext({
185+
traceId: id('0'),
186+
spanId: id('0')
187+
})
188+
const attributes = {
189+
'ptr.kind': ptrKind,
190+
'ptr.dir': ptrDir,
191+
'ptr.hash': ptrHash,
192+
'link.kind': 'span-pointer'
193+
}
194+
this.addLink(zeroContext, attributes)
195+
}
196+
183197
addEvent (name, attributesOrStartTime, startTime) {
184198
const event = { name }
185199
if (attributesOrStartTime) {

‎packages/dd-trace/src/util.js‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict'
22

3+
const crypto = require('crypto')
34
const path = require('path')
45

56
function isTrue (str) {
@@ -73,11 +74,25 @@ function hasOwn (object, prop) {
7374
return Object.prototype.hasOwnProperty.call(object, prop)
7475
}
7576

77+
/**
78+
* Generates a unique hash from an array of strings by joining them with | before hashing.
79+
* Used to uniquely identify AWS requests for span pointers.
80+
* @param {string[]} components - Array of strings to hash
81+
* @returns {string} A 32-character hash uniquely identifying the components
82+
*/
83+
function generatePointerHash (components) {
84+
// If passing S3's ETag as a component, make sure any quotes have already been removed!
85+
const dataToHash = components.join('|')
86+
const hash = crypto.createHash('sha256').update(dataToHash).digest('hex')
87+
return hash.substring(0, 32)
88+
}
89+
7690
module.exports = {
7791
isTrue,
7892
isFalse,
7993
isError,
8094
globMatch,
8195
calculateDDBasePath,
82-
hasOwn
96+
hasOwn,
97+
generatePointerHash
8398
}

‎packages/dd-trace/test/opentelemetry/span.spec.js‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,33 @@ describe('OTel Span', () => {
325325
expect(_links).to.have.lengthOf(2)
326326
})
327327

328+
it('should add span pointers', () => {
329+
const span = makeSpan('name')
330+
const { _links } = span._ddSpan
331+
332+
span.addSpanPointer('pointer_kind', 'd', 'abc123')
333+
expect(_links).to.have.lengthOf(1)
334+
expect(_links[0].attributes).to.deep.equal({
335+
'ptr.kind': 'pointer_kind',
336+
'ptr.dir': 'd',
337+
'ptr.hash': 'abc123',
338+
'link.kind': 'span-pointer'
339+
})
340+
expect(_links[0].context.toTraceId()).to.equal('0')
341+
expect(_links[0].context.toSpanId()).to.equal('0')
342+
343+
span.addSpanPointer('another_kind', 'd', '1234567')
344+
expect(_links).to.have.lengthOf(2)
345+
expect(_links[1].attributes).to.deep.equal({
346+
'ptr.kind': 'another_kind',
347+
'ptr.dir': 'd',
348+
'ptr.hash': '1234567',
349+
'link.kind': 'span-pointer'
350+
})
351+
expect(_links[1].context.toTraceId()).to.equal('0')
352+
expect(_links[1].context.toSpanId()).to.equal('0')
353+
})
354+
328355
it('should set status', () => {
329356
const unset = makeSpan('name')
330357
const unsetCtx = unset._ddSpan.context()

0 commit comments

Comments
 (0)