diff --git a/packages/tracer/src/provider/ProviderService.ts b/packages/tracer/src/provider/ProviderService.ts index 69e7255b8c..facfca45ef 100644 --- a/packages/tracer/src/provider/ProviderService.ts +++ b/packages/tracer/src/provider/ProviderService.ts @@ -28,7 +28,7 @@ import { addUserAgentMiddleware } from '@aws-lambda-powertools/commons'; import type { DiagnosticsChannel } from 'undici-types'; import { findHeaderAndDecode, - getOriginURL, + getRequestURL, isHttpSubsegment, } from './utilities.js'; @@ -107,16 +107,18 @@ class ProviderService implements ProviderServiceInterface { const { request } = message as DiagnosticsChannel.RequestCreateMessage; const parentSubsegment = this.getSegment(); - if (parentSubsegment && request.origin) { - const origin = getOriginURL(request.origin); + const requestURL = getRequestURL(request); + if (parentSubsegment && requestURL) { const method = request.method; - const subsegment = parentSubsegment.addNewSubsegment(origin.hostname); + const subsegment = parentSubsegment.addNewSubsegment( + requestURL.hostname + ); subsegment.addAttribute('namespace', 'remote'); (subsegment as HttpSubsegment).http = { request: { - url: origin.hostname, + url: `${requestURL.protocol}//${requestURL.hostname}${requestURL.pathname}`, method, }, }; diff --git a/packages/tracer/src/provider/utilities.ts b/packages/tracer/src/provider/utilities.ts index a4b9e8e587..8e9ab1f3fc 100644 --- a/packages/tracer/src/provider/utilities.ts +++ b/packages/tracer/src/provider/utilities.ts @@ -1,5 +1,6 @@ import { URL } from 'node:url'; import type { Segment, Subsegment } from 'aws-xray-sdk-core'; +import type { DiagnosticsChannel } from 'undici-types'; import type { HttpSubsegment } from '../types/ProviderService.js'; const decoder = new TextDecoder(); @@ -52,12 +53,24 @@ const isHttpSubsegment = ( }; /** - * Convert the origin url to a URL object when it is a string + * Convert the origin url to a URL object when it is a string and append the path if provided * - * @param origin The origin url + * @param origin The request object containing the origin url and path */ -const getOriginURL = (origin: string | URL): URL => { - return origin instanceof URL ? origin : new URL(origin); +const getRequestURL = ( + request: DiagnosticsChannel.Request +): URL | undefined => { + if (typeof request.origin === 'string') { + return new URL(`${request.origin}${request.path || ''}`); + } + + if (request.origin instanceof URL) { + request.origin.pathname = request.path || ''; + + return request.origin; + } + + return undefined; }; -export { findHeaderAndDecode, isHttpSubsegment, getOriginURL }; +export { findHeaderAndDecode, isHttpSubsegment, getRequestURL }; diff --git a/packages/tracer/tests/e2e/middy.test.ts b/packages/tracer/tests/e2e/middy.test.ts index e7b4e74cd2..408e188578 100644 --- a/packages/tracer/tests/e2e/middy.test.ts +++ b/packages/tracer/tests/e2e/middy.test.ts @@ -110,7 +110,7 @@ describe('Tracer E2E tests, middy instrumentation', () => { const httpSubsegment = subsegments.get('docs.powertools.aws.dev'); expect(httpSubsegment?.namespace).toBe('remote'); expect(httpSubsegment?.http?.request?.url).toEqual( - 'docs.powertools.aws.dev' + 'https://docs.powertools.aws.dev/lambda/typescript/latest/' ); expect(httpSubsegment?.http?.request?.method).toBe('GET'); expect(httpSubsegment?.http?.response?.status).toEqual(expect.any(Number)); diff --git a/packages/tracer/tests/helpers/mockRequests.ts b/packages/tracer/tests/helpers/mockRequests.ts index 8c5865e8d3..ff2d930f04 100644 --- a/packages/tracer/tests/helpers/mockRequests.ts +++ b/packages/tracer/tests/helpers/mockRequests.ts @@ -2,7 +2,8 @@ import { channel } from 'node:diagnostics_channel'; import type { URL } from 'node:url'; type MockFetchOptions = { - origin: string | URL; + origin?: string | URL; + path?: string; method?: string; headers?: { [key: string]: string }; } & ( @@ -25,6 +26,7 @@ type MockFetchOptions = { */ const mockFetch = ({ origin, + path, method, statusCode, headers, @@ -37,6 +39,7 @@ const mockFetch = ({ const request = { origin, method: method ?? 'GET', + path, }; requestCreateChannel.publish({ diff --git a/packages/tracer/tests/unit/ProviderService.test.ts b/packages/tracer/tests/unit/ProviderService.test.ts index f4d0b925aa..65688774a1 100644 --- a/packages/tracer/tests/unit/ProviderService.test.ts +++ b/packages/tracer/tests/unit/ProviderService.test.ts @@ -396,7 +396,8 @@ describe('Class: ProviderService', () => { // Act provider.instrumentFetch(); mockFetch({ - origin: 'https://aws.amazon.com/blogs', + origin: 'https://aws.amazon.com', + path: '/blogs', headers: { 'content-length': '100', }, @@ -407,7 +408,7 @@ describe('Class: ProviderService', () => { expect(segment.addNewSubsegment).toHaveBeenCalledWith('aws.amazon.com'); expect((subsegment as HttpSubsegment).http).toEqual({ request: { - url: 'aws.amazon.com', + url: 'https://aws.amazon.com/blogs', method: 'GET', }, response: { @@ -438,7 +439,8 @@ describe('Class: ProviderService', () => { // Act provider.instrumentFetch(); mockFetch({ - origin: new URL('https://aws.amazon.com/blogs'), + origin: new URL('https://aws.amazon.com'), + path: '/blogs', headers: { 'content-type': 'application/json', }, @@ -447,7 +449,7 @@ describe('Class: ProviderService', () => { // Assess expect((subsegment as HttpSubsegment).http).toEqual({ request: { - url: 'aws.amazon.com', + url: 'https://aws.amazon.com/blogs', method: 'GET', }, response: { @@ -568,6 +570,56 @@ describe('Class: ProviderService', () => { expect(subsegment.close).toHaveBeenCalledTimes(1); expect(provider.setSegment).toHaveBeenLastCalledWith(segment); }); + + it('skips the segment creation when the request has no origin', async () => { + // Prepare + const provider: ProviderService = new ProviderService(); + const segment = new Subsegment('## dummySegment'); + jest.spyOn(segment, 'addNewSubsegment'); + jest.spyOn(provider, 'getSegment').mockImplementation(() => segment); + jest.spyOn(provider, 'setSegment'); + + // Act + provider.instrumentFetch(); + mockFetch({}); + + // Assess + expect(segment.addNewSubsegment).toHaveBeenCalledTimes(0); + expect(provider.setSegment).toHaveBeenCalledTimes(0); + }); + + it('does not add any path to the segment when the request has no path', async () => { + // Prepare + const provider: ProviderService = new ProviderService(); + const segment = new Subsegment('## dummySegment'); + const subsegment = segment.addNewSubsegment('aws.amazon.com'); + jest + .spyOn(segment, 'addNewSubsegment') + .mockImplementationOnce(() => subsegment); + jest + .spyOn(provider, 'getSegment') + .mockImplementationOnce(() => segment) + .mockImplementationOnce(() => subsegment) + .mockImplementationOnce(() => subsegment); + jest.spyOn(subsegment, 'close'); + jest.spyOn(provider, 'setSegment'); + + // Act + provider.instrumentFetch(); + mockFetch({ + origin: new URL('https://aws.amazon.com'), + }); + + // Assess + expect((subsegment as HttpSubsegment).http).toEqual( + expect.objectContaining({ + request: { + url: 'https://aws.amazon.com/', + method: 'GET', + }, + }) + ); + }); }); it('closes the segment and adds a fault flag when the connection fails', async () => {