Skip to content

Commit e694757

Browse files
authored
fix: add "s3/" prefix to destination.service.resource for S3 spans (#3102)
Closes: #3084 Refs: elastic/apm#738
1 parent f76376c commit e694757

File tree

3 files changed

+22
-17
lines changed

3 files changed

+22
-17
lines changed

CHANGELOG.asciidoc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,17 @@ Notes:
5353
values, e.g. `https://user:pass@...`, then the auth fields were not
5454
included in the actual HTTP request. ({issues}2044[#2044])
5555
56+
* Fix `span.context.destination.service.resource` for S3 spans to have an
57+
"s3/" prefix.
58+
+
59+
*Note*: While this is considered a bugfix, but it can potentially be a breaking
60+
change in the Kibana APM app: It can break the history of the S3-Spans / metrics
61+
for users relying on `context.destination.service.resource`. If users happen to
62+
run agents both with and without this fix (for same or different languages), the
63+
same S3-buckets can appear twice in the service map (with and without
64+
s3-prefix).
65+
66+
5667
[float]
5768
===== Chores
5869

lib/instrumentation/modules/aws-sdk/s3.js

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,7 @@ function instrumentationS3 (orig, origArguments, request, AWS, agent, { version,
7979
const region = httpRequest && httpRequest.region
8080

8181
span.setServiceTarget('s3', resource)
82-
const destContext = {
83-
// Typically 'destination.service.resource' is calculated from
84-
// 'service.target' in Span.end(), but S3 is a special case in the spec.
85-
service: {
86-
resource
87-
}
88-
}
82+
const destContext = {}
8983
// '.httpRequest.endpoint' might differ from '.service.endpoint' if
9084
// the bucket is in a different region.
9185
const endpoint = httpRequest && httpRequest.endpoint

test/instrumentation/modules/aws-sdk/s3.test.js

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ tape.test('simple S3 usage scenario', function (t) {
127127
address: LOCALSTACK_HOST,
128128
port: 4566,
129129
cloud: { region: 'us-east-2' },
130-
service: { type: '', name: '', resource: 'elasticapmtest-bucket-1' }
130+
service: { type: '', name: '', resource: 's3/elasticapmtest-bucket-1' }
131131
},
132132
http: { status_code: 200, response: { encoded_body_size: 177 } }
133133
},
@@ -145,7 +145,7 @@ tape.test('simple S3 usage scenario', function (t) {
145145
address: LOCALSTACK_HOST,
146146
port: 4566,
147147
cloud: { region: 'us-east-2' },
148-
service: { type: '', name: '', resource: 'elasticapmtest-bucket-1' }
148+
service: { type: '', name: '', resource: 's3/elasticapmtest-bucket-1' }
149149
},
150150
http: { status_code: 200 }
151151
},
@@ -163,7 +163,7 @@ tape.test('simple S3 usage scenario', function (t) {
163163
address: LOCALSTACK_HOST,
164164
port: 4566,
165165
cloud: { region: 'us-east-2' },
166-
service: { type: '', name: '', resource: 'elasticapmtest-bucket-1' }
166+
service: { type: '', name: '', resource: 's3/elasticapmtest-bucket-1' }
167167
},
168168
http: { status_code: 200 }
169169
},
@@ -181,7 +181,7 @@ tape.test('simple S3 usage scenario', function (t) {
181181
address: LOCALSTACK_HOST,
182182
port: 4566,
183183
cloud: { region: 'us-east-2' },
184-
service: { type: '', name: '', resource: 'elasticapmtest-bucket-1' }
184+
service: { type: '', name: '', resource: 's3/elasticapmtest-bucket-1' }
185185
},
186186
http: { status_code: 200 }
187187
},
@@ -199,7 +199,7 @@ tape.test('simple S3 usage scenario', function (t) {
199199
address: LOCALSTACK_HOST,
200200
port: 4566,
201201
cloud: { region: 'us-east-2' },
202-
service: { type: '', name: '', resource: 'elasticapmtest-bucket-1' }
202+
service: { type: '', name: '', resource: 's3/elasticapmtest-bucket-1' }
203203
},
204204
http: { status_code: 200, response: { encoded_body_size: 8 } }
205205
},
@@ -217,7 +217,7 @@ tape.test('simple S3 usage scenario', function (t) {
217217
address: LOCALSTACK_HOST,
218218
port: 4566,
219219
cloud: { region: 'us-east-2' },
220-
service: { type: '', name: '', resource: 'elasticapmtest-bucket-1' }
220+
service: { type: '', name: '', resource: 's3/elasticapmtest-bucket-1' }
221221
},
222222
http: { status_code: 304 }
223223
},
@@ -235,7 +235,7 @@ tape.test('simple S3 usage scenario', function (t) {
235235
address: LOCALSTACK_HOST,
236236
port: 4566,
237237
cloud: { region: 'us-east-2' },
238-
service: { type: '', name: '', resource: 'elasticapmtest-bucket-1' }
238+
service: { type: '', name: '', resource: 's3/elasticapmtest-bucket-1' }
239239
},
240240
http: { status_code: 200, response: { encoded_body_size: 8 } }
241241
},
@@ -254,7 +254,7 @@ tape.test('simple S3 usage scenario', function (t) {
254254
address: LOCALSTACK_HOST,
255255
port: 4566,
256256
cloud: { region: 'us-east-2' },
257-
service: { type: '', name: '', resource: 'elasticapmtest-bucket-1' }
257+
service: { type: '', name: '', resource: 's3/elasticapmtest-bucket-1' }
258258
},
259259
http: { status_code: 404, response: { encoded_body_size: 207 } }
260260
},
@@ -276,7 +276,7 @@ tape.test('simple S3 usage scenario', function (t) {
276276
address: LOCALSTACK_HOST,
277277
port: 4566,
278278
cloud: { region: 'us-east-2' },
279-
service: { type: '', name: '', resource: 'elasticapmtest-bucket-1' }
279+
service: { type: '', name: '', resource: 's3/elasticapmtest-bucket-1' }
280280
},
281281
http: { status_code: 204 }
282282
},
@@ -294,7 +294,7 @@ tape.test('simple S3 usage scenario', function (t) {
294294
address: LOCALSTACK_HOST,
295295
port: 4566,
296296
cloud: { region: 'us-east-2' },
297-
service: { type: '', name: '', resource: 'elasticapmtest-bucket-1' }
297+
service: { type: '', name: '', resource: 's3/elasticapmtest-bucket-1' }
298298
},
299299
http: { status_code: 204 }
300300
},

0 commit comments

Comments
 (0)