Skip to content

Commit 6864e53

Browse files
authored
fix(tracer): include request pathname in trace data (#2955)
1 parent 5365440 commit 6864e53

File tree

5 files changed

+86
-16
lines changed

5 files changed

+86
-16
lines changed

Diff for: packages/tracer/src/provider/ProviderService.ts

+7-5
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import { addUserAgentMiddleware } from '@aws-lambda-powertools/commons';
2828
import type { DiagnosticsChannel } from 'undici-types';
2929
import {
3030
findHeaderAndDecode,
31-
getOriginURL,
31+
getRequestURL,
3232
isHttpSubsegment,
3333
} from './utilities.js';
3434

@@ -107,16 +107,18 @@ class ProviderService implements ProviderServiceInterface {
107107
const { request } = message as DiagnosticsChannel.RequestCreateMessage;
108108

109109
const parentSubsegment = this.getSegment();
110-
if (parentSubsegment && request.origin) {
111-
const origin = getOriginURL(request.origin);
110+
const requestURL = getRequestURL(request);
111+
if (parentSubsegment && requestURL) {
112112
const method = request.method;
113113

114-
const subsegment = parentSubsegment.addNewSubsegment(origin.hostname);
114+
const subsegment = parentSubsegment.addNewSubsegment(
115+
requestURL.hostname
116+
);
115117
subsegment.addAttribute('namespace', 'remote');
116118

117119
(subsegment as HttpSubsegment).http = {
118120
request: {
119-
url: origin.hostname,
121+
url: `${requestURL.protocol}//${requestURL.hostname}${requestURL.pathname}`,
120122
method,
121123
},
122124
};

Diff for: packages/tracer/src/provider/utilities.ts

+18-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { URL } from 'node:url';
22
import type { Segment, Subsegment } from 'aws-xray-sdk-core';
3+
import type { DiagnosticsChannel } from 'undici-types';
34
import type { HttpSubsegment } from '../types/ProviderService.js';
45

56
const decoder = new TextDecoder();
@@ -52,12 +53,24 @@ const isHttpSubsegment = (
5253
};
5354

5455
/**
55-
* Convert the origin url to a URL object when it is a string
56+
* Convert the origin url to a URL object when it is a string and append the path if provided
5657
*
57-
* @param origin The origin url
58+
* @param origin The request object containing the origin url and path
5859
*/
59-
const getOriginURL = (origin: string | URL): URL => {
60-
return origin instanceof URL ? origin : new URL(origin);
60+
const getRequestURL = (
61+
request: DiagnosticsChannel.Request
62+
): URL | undefined => {
63+
if (typeof request.origin === 'string') {
64+
return new URL(`${request.origin}${request.path || ''}`);
65+
}
66+
67+
if (request.origin instanceof URL) {
68+
request.origin.pathname = request.path || '';
69+
70+
return request.origin;
71+
}
72+
73+
return undefined;
6174
};
6275

63-
export { findHeaderAndDecode, isHttpSubsegment, getOriginURL };
76+
export { findHeaderAndDecode, isHttpSubsegment, getRequestURL };

Diff for: packages/tracer/tests/e2e/middy.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ describe('Tracer E2E tests, middy instrumentation', () => {
110110
const httpSubsegment = subsegments.get('docs.powertools.aws.dev');
111111
expect(httpSubsegment?.namespace).toBe('remote');
112112
expect(httpSubsegment?.http?.request?.url).toEqual(
113-
'docs.powertools.aws.dev'
113+
'https://docs.powertools.aws.dev/lambda/typescript/latest/'
114114
);
115115
expect(httpSubsegment?.http?.request?.method).toBe('GET');
116116
expect(httpSubsegment?.http?.response?.status).toEqual(expect.any(Number));

Diff for: packages/tracer/tests/helpers/mockRequests.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ import { channel } from 'node:diagnostics_channel';
22
import type { URL } from 'node:url';
33

44
type MockFetchOptions = {
5-
origin: string | URL;
5+
origin?: string | URL;
6+
path?: string;
67
method?: string;
78
headers?: { [key: string]: string };
89
} & (
@@ -25,6 +26,7 @@ type MockFetchOptions = {
2526
*/
2627
const mockFetch = ({
2728
origin,
29+
path,
2830
method,
2931
statusCode,
3032
headers,
@@ -37,6 +39,7 @@ const mockFetch = ({
3739
const request = {
3840
origin,
3941
method: method ?? 'GET',
42+
path,
4043
};
4144

4245
requestCreateChannel.publish({

Diff for: packages/tracer/tests/unit/ProviderService.test.ts

+56-4
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,8 @@ describe('Class: ProviderService', () => {
396396
// Act
397397
provider.instrumentFetch();
398398
mockFetch({
399-
origin: 'https://aws.amazon.com/blogs',
399+
origin: 'https://aws.amazon.com',
400+
path: '/blogs',
400401
headers: {
401402
'content-length': '100',
402403
},
@@ -407,7 +408,7 @@ describe('Class: ProviderService', () => {
407408
expect(segment.addNewSubsegment).toHaveBeenCalledWith('aws.amazon.com');
408409
expect((subsegment as HttpSubsegment).http).toEqual({
409410
request: {
410-
url: 'aws.amazon.com',
411+
url: 'https://aws.amazon.com/blogs',
411412
method: 'GET',
412413
},
413414
response: {
@@ -438,7 +439,8 @@ describe('Class: ProviderService', () => {
438439
// Act
439440
provider.instrumentFetch();
440441
mockFetch({
441-
origin: new URL('https://aws.amazon.com/blogs'),
442+
origin: new URL('https://aws.amazon.com'),
443+
path: '/blogs',
442444
headers: {
443445
'content-type': 'application/json',
444446
},
@@ -447,7 +449,7 @@ describe('Class: ProviderService', () => {
447449
// Assess
448450
expect((subsegment as HttpSubsegment).http).toEqual({
449451
request: {
450-
url: 'aws.amazon.com',
452+
url: 'https://aws.amazon.com/blogs',
451453
method: 'GET',
452454
},
453455
response: {
@@ -568,6 +570,56 @@ describe('Class: ProviderService', () => {
568570
expect(subsegment.close).toHaveBeenCalledTimes(1);
569571
expect(provider.setSegment).toHaveBeenLastCalledWith(segment);
570572
});
573+
574+
it('skips the segment creation when the request has no origin', async () => {
575+
// Prepare
576+
const provider: ProviderService = new ProviderService();
577+
const segment = new Subsegment('## dummySegment');
578+
jest.spyOn(segment, 'addNewSubsegment');
579+
jest.spyOn(provider, 'getSegment').mockImplementation(() => segment);
580+
jest.spyOn(provider, 'setSegment');
581+
582+
// Act
583+
provider.instrumentFetch();
584+
mockFetch({});
585+
586+
// Assess
587+
expect(segment.addNewSubsegment).toHaveBeenCalledTimes(0);
588+
expect(provider.setSegment).toHaveBeenCalledTimes(0);
589+
});
590+
591+
it('does not add any path to the segment when the request has no path', async () => {
592+
// Prepare
593+
const provider: ProviderService = new ProviderService();
594+
const segment = new Subsegment('## dummySegment');
595+
const subsegment = segment.addNewSubsegment('aws.amazon.com');
596+
jest
597+
.spyOn(segment, 'addNewSubsegment')
598+
.mockImplementationOnce(() => subsegment);
599+
jest
600+
.spyOn(provider, 'getSegment')
601+
.mockImplementationOnce(() => segment)
602+
.mockImplementationOnce(() => subsegment)
603+
.mockImplementationOnce(() => subsegment);
604+
jest.spyOn(subsegment, 'close');
605+
jest.spyOn(provider, 'setSegment');
606+
607+
// Act
608+
provider.instrumentFetch();
609+
mockFetch({
610+
origin: new URL('https://aws.amazon.com'),
611+
});
612+
613+
// Assess
614+
expect((subsegment as HttpSubsegment).http).toEqual(
615+
expect.objectContaining({
616+
request: {
617+
url: 'https://aws.amazon.com/',
618+
method: 'GET',
619+
},
620+
})
621+
);
622+
});
571623
});
572624

573625
it('closes the segment and adds a fault flag when the connection fails', async () => {

0 commit comments

Comments
 (0)