From f1fe3a71b4fae1bfcd5fa3d92d09417f67eb102b Mon Sep 17 00:00:00 2001 From: Blair Huang Date: Thu, 12 Jun 2025 16:13:23 -0700 Subject: [PATCH 01/10] Extract arn and account access key --- .../src/aws-attribute-keys.ts | 7 ++ .../src/patches/instrumentation-patch.ts | 100 +++++++++++++++++- .../patches/instrumentation-patch.test.ts | 75 ++++++++++++- 3 files changed, 180 insertions(+), 2 deletions(-) diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-attribute-keys.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-attribute-keys.ts index fde4ae2a..1ad49aed 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-attribute-keys.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-attribute-keys.ts @@ -5,12 +5,17 @@ import { SEMATTRS_AWS_DYNAMODB_TABLE_NAMES } from '@opentelemetry/semantic-conve // Utility class holding attribute keys with special meaning to AWS components export const AWS_ATTRIBUTE_KEYS = { + AWS_AUTH_ACCESS_KEY: 'aws.auth.account.access_key', + AWS_AUTH_REGION: 'aws.auth.region', AWS_SPAN_KIND: 'aws.span.kind', AWS_LOCAL_SERVICE: 'aws.local.service', AWS_LOCAL_OPERATION: 'aws.local.operation', AWS_REMOTE_SERVICE: 'aws.remote.service', AWS_REMOTE_ENVIRONMENT: 'aws.remote.environment', AWS_REMOTE_OPERATION: 'aws.remote.operation', + AWS_REMOTE_RESOURCE_ACCESS_KEY: 'aws.remote.resource.account.access_key', + AWS_REMOTE_RESOURCE_ACCOUNT_ID: 'aws.remote.resource.account.id', + AWS_REMOTE_RESOURCE_REGION: 'aws.remote.resource.region', AWS_REMOTE_RESOURCE_TYPE: 'aws.remote.resource.type', AWS_REMOTE_RESOURCE_IDENTIFIER: 'aws.remote.resource.identifier', AWS_SDK_DESCENDANT: 'aws.sdk.descendant', @@ -31,7 +36,9 @@ export const AWS_ATTRIBUTE_KEYS = { AWS_S3_BUCKET: 'aws.s3.bucket', AWS_SQS_QUEUE_URL: 'aws.sqs.queue.url', AWS_SQS_QUEUE_NAME: 'aws.sqs.queue.name', + AWS_KINESIS_STREAM_ARN: 'aws.kinesis.stream.arn', AWS_KINESIS_STREAM_NAME: 'aws.kinesis.stream.name', + AWS_DYNAMODB_TABLE_ARN: 'aws.dynamodb.table.arn', AWS_DYNAMODB_TABLE_NAMES: SEMATTRS_AWS_DYNAMODB_TABLE_NAMES, AWS_BEDROCK_DATA_SOURCE_ID: 'aws.bedrock.data_source.id', AWS_BEDROCK_KNOWLEDGE_BASE_ID: 'aws.bedrock.knowledge_base.id', diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/patches/instrumentation-patch.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/patches/instrumentation-patch.ts index 18f2b226..8fe314e5 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/patches/instrumentation-patch.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/patches/instrumentation-patch.ts @@ -80,6 +80,8 @@ export function applyInstrumentationPatches(instrumentations: Instrumentation[]) patchSqsServiceExtension(services.get('SQS')); patchSnsServiceExtension(services.get('SNS')); patchLambdaServiceExtension(services.get('Lambda')); + patchKinesisServiceExtension(services.get('Kinesis')); + patchDynamoDbServiceExtension(services.get('DynamoDB')); } } else if (instrumentation.instrumentationName === '@opentelemetry/instrumentation-aws-lambda') { diag.debug('Patching aws lambda instrumentation'); @@ -189,6 +191,68 @@ function patchSnsServiceExtension(snsServiceExtension: any): void { } } +/* + * This patch extends the existing upstream extension for Kinesis. Extensions allow for custom logic for adding + * service-specific information to spans, such as attributes. Specifically, we are adding logic to add + * `aws.kinesis.stream.arn` attribute, to be used to generate RemoteTarget and achieve parity with the Java/Python instrumentation. + * + * + * @param kinesisServiceExtension Kinesis Service Extension obtained the service extension list from the AWS SDK OTel Instrumentation + */ +function patchKinesisServiceExtension(kinesisServiceExtension: any): void { + if (kinesisServiceExtension) { + const requestPreSpanHook = kinesisServiceExtension.requestPreSpanHook; + kinesisServiceExtension._requestPreSpanHook = requestPreSpanHook; + + const patchedRequestPreSpanHook = ( + request: NormalizedRequest, + _config: AwsSdkInstrumentationConfig + ): RequestMetadata => { + const requestMetadata: RequestMetadata = kinesisServiceExtension._requestPreSpanHook(request, _config); + if (requestMetadata.spanAttributes) { + const streamArn = request.commandInput?.StreamARN; + if (streamArn) { + requestMetadata.spanAttributes[AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN] = streamArn; + } + } + return requestMetadata; + }; + + kinesisServiceExtension.requestPreSpanHook = patchedRequestPreSpanHook; + } +} + +/* + * This patch extends the existing upstream extension for DynamoDB. Extensions allow for custom logic for adding + * service-specific information to spans, such as attributes. Specifically, we are adding logic to add + * `aws.dynamodb.table.arn` attribute, to be used to generate RemoteTarget and achieve parity with the Java/Python instrumentation. + * + * + * @param dynamoDbServiceExtension DynamoDB Service Extension obtained the service extension list from the AWS SDK OTel Instrumentation + */ +function patchDynamoDbServiceExtension(dynamoDbServiceExtension: any): void { + if (dynamoDbServiceExtension) { + if (typeof dynamoDbServiceExtension.responseHook === 'function') { + const originalResponseHook = dynamoDbServiceExtension.responseHook; + dynamoDbServiceExtension.responseHook = ( + response: NormalizedResponse, + span: Span, + tracer: Tracer, + config: AwsSdkInstrumentationConfig + ): void => { + originalResponseHook.call(dynamoDbServiceExtension, response, span, tracer, config); + + if (response.data && response.data.Table) { + const tableArn = response.data.Table.TableArn; + if (tableArn) { + span.setAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN, tableArn); + } + } + }; + } + } +} + /* * This patch extends the existing upstream extension for Lambda. Extensions allow for custom logic for adding * service-specific information to spans, such as attributes. Specifically, we are adding logic to add @@ -293,7 +357,7 @@ function patchAwsLambdaInstrumentation(instrumentation: Instrumentation): void { } } -// Override the upstream private _getV3SmithyClientSendPatch method to add middleware to inject X-Ray Trace Context into HTTP Headers +// Override the upstream private _getV3SmithyClientSendPatch method to add middlewares to inject X-Ray Trace Context into HTTP Headers and to extract account access key id and region for cross-account support // https://github.com/open-telemetry/opentelemetry-js-contrib/blob/instrumentation-aws-sdk-v0.48.0/plugins/node/opentelemetry-instrumentation-aws-sdk/src/aws-sdk.ts#L373-L384 const awsXrayPropagator = new AWSXRayPropagator(); const V3_CLIENT_CONFIG_KEY = Symbol('opentelemetry.instrumentation.aws-sdk.client.config'); @@ -328,6 +392,40 @@ function patchAwsSdkInstrumentation(instrumentation: Instrumentation): void { } ); + this.middlewareStack?.add( + (next: any, context: any) => async (middlewareArgs: any) => { + const activeContext = otelContext.active(); + const span = trace.getSpan(activeContext); + + if (span) { + try { + const credsProvider = this.config.credentials; + if (credsProvider) { + const credentials = await credsProvider(); + if (credentials.accessKeyId) { + span.setAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCESS_KEY, credentials.accessKeyId); + } + } + if (this.config.region) { + const region = await this.config.region(); + if (region) { + span.setAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_REGION, region); + } + } + } catch (err) { + diag.debug('Fail to get auth account access key and region.'); + } + } + + return await next(middlewareArgs); + }, + { + step: 'build', + name: '_extractSignerCredentials', + override: true, + } + ); + command[V3_CLIENT_CONFIG_KEY] = this.config; return original.apply(this, [command, ...args]); }; diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/test/patches/instrumentation-patch.test.ts b/aws-distro-opentelemetry-node-autoinstrumentation/test/patches/instrumentation-patch.test.ts index 1f815c2a..f4a0b689 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/test/patches/instrumentation-patch.test.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/test/patches/instrumentation-patch.test.ts @@ -58,6 +58,8 @@ const _BEDROCK_GUARDRAIL_ARN: string = 'arn:aws:bedrock:us-east-1:123456789012:g const _BEDROCK_KNOWLEDGEBASE_ID: string = 'KnowledgeBaseId'; const _GEN_AI_SYSTEM: string = 'aws.bedrock'; const _GEN_AI_REQUEST_MODEL: string = 'genAiReuqestModelId'; +const _STREAM_ARN: string = 'arn:aws:kinesis:us-west-2:123456789012:stream/testStream'; +const _TABLE_ARN: string = 'arn:aws:dynamodb:us-west-2:123456789012:table/testTable'; const mockHeaders = { 'x-test-header': 'test-value', @@ -90,6 +92,8 @@ describe('InstrumentationPatchTest', () => { expect(services.get('Lambda').requestPreSpanHook).toBeTruthy(); expect(services.get('SQS')._requestPreSpanHook).toBeFalsy(); expect(services.get('SQS').requestPreSpanHook).toBeTruthy(); + expect(services.get('Kinesis')._requestPreSpanHook).toBeFalsy(); + expect(services.get('Kinesis').requestPreSpanHook).toBeTruthy(); expect(services.has('Bedrock')).toBeFalsy(); expect(services.has('BedrockAgent')).toBeFalsy(); expect(services.get('BedrockAgentRuntime')).toBeFalsy(); @@ -119,6 +123,8 @@ describe('InstrumentationPatchTest', () => { expect(services.get('Lambda').requestPreSpanHook).toBeTruthy(); expect(services.get('SQS')._requestPreSpanHook).toBeTruthy(); expect(services.get('SQS').requestPreSpanHook).toBeTruthy(); + expect(services.get('Kinesis')._requestPreSpanHook).toBeTruthy(); + expect(services.get('Kinesis').requestPreSpanHook).toBeTruthy(); expect(services.has('Bedrock')).toBeTruthy(); expect(services.has('BedrockAgent')).toBeTruthy(); expect(services.get('BedrockAgentRuntime')).toBeTruthy(); @@ -179,6 +185,14 @@ describe('InstrumentationPatchTest', () => { expect(() => doExtractBedrockAttributes(services, 'Bedrock')).toThrow(); }); + it('Kinesis without patching', () => { + const unpatchedAwsSdkInstrumentation: AwsInstrumentation = extractAwsSdkInstrumentation(UNPATCHED_INSTRUMENTATIONS); + const services: Map = extractServicesFromAwsSdkInstrumentation(unpatchedAwsSdkInstrumentation); + expect(() => doExtractKinesisAttributes(services)).not.toThrow(); + const kinesisAttributes: Attributes = doExtractKinesisAttributes(services); + expect(kinesisAttributes[AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN]).toBeUndefined(); + }); + it('SNS with patching', () => { const patchedAwsSdkInstrumentation: AwsInstrumentation = extractAwsSdkInstrumentation(PATCHED_INSTRUMENTATIONS); const services: Map = extractServicesFromAwsSdkInstrumentation(patchedAwsSdkInstrumentation); @@ -233,6 +247,20 @@ describe('InstrumentationPatchTest', () => { expect(responseHookSecretsManagerAttributes[AWS_ATTRIBUTE_KEYS.AWS_SECRETSMANAGER_SECRET_ARN]).toBe(_SECRETS_ARN); }); + it('Kinesis with patching', () => { + const patchedAwsSdkInstrumentation: AwsInstrumentation = extractAwsSdkInstrumentation(PATCHED_INSTRUMENTATIONS); + const services: Map = extractServicesFromAwsSdkInstrumentation(patchedAwsSdkInstrumentation); + const requestKinesisAttributes: Attributes = doExtractKinesisAttributes(services); + expect(requestKinesisAttributes[AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN]).toEqual(_STREAM_ARN); + }); + + it('DynamoDB with patching', () => { + const patchedAwsSdkInstrumentation: AwsInstrumentation = extractAwsSdkInstrumentation(PATCHED_INSTRUMENTATIONS); + const services: Map = extractServicesFromAwsSdkInstrumentation(patchedAwsSdkInstrumentation); + const responseDynamoDbAttributes: Attributes = doResponseHookDynamoDb(services); + expect(responseDynamoDbAttributes[AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN]).toEqual(_TABLE_ARN); + }); + it('Bedrock with patching', () => { const patchedAwsSdkInstrumentation: AwsInstrumentation = extractAwsSdkInstrumentation(PATCHED_INSTRUMENTATIONS); const services: Map = extractServicesFromAwsSdkInstrumentation(patchedAwsSdkInstrumentation); @@ -446,6 +474,18 @@ describe('InstrumentationPatchTest', () => { return doExtractAttributes(services, serviceName, params); } + function doExtractKinesisAttributes(services: Map): Attributes { + const serviceName: string = 'Kinesis'; + const params: NormalizedRequest = { + serviceName: serviceName, + commandName: 'mockCommandName', + commandInput: { + StreamARN: _STREAM_ARN, + }, + }; + return doExtractAttributes(services, serviceName, params); + } + function doExtractAttributes( services: Map, serviceName: string, @@ -492,6 +532,23 @@ describe('InstrumentationPatchTest', () => { return doResponseHook(services, 'Lambda', results as NormalizedResponse); } + function doResponseHookDynamoDb(services: Map): Attributes { + const results: Partial = { + data: { + Table: { + TableArn: _TABLE_ARN, + }, + }, + request: { + commandInput: {}, + commandName: 'dummy_operation', + serviceName: 'DynamoDB', + }, + }; + + return doResponseHook(services, 'DynamoDB', results as NormalizedResponse); + } + function doResponseHookBedrock( services: Map, serviceName: string, @@ -563,9 +620,13 @@ describe('InstrumentationPatchTest', () => { mockedMiddlewareStack = { add: (arg1: any, arg2: any) => mockedMiddlewareStackInternal.push([arg1, arg2]), }; + const mockConfig = { + credentials: () => Promise.resolve({ accessKeyId: 'test-access-key' }), + region: () => Promise.resolve('us-west-2'), + }; const send = extractAwsSdkInstrumentation(PATCHED_INSTRUMENTATIONS) ['_getV3SmithyClientSendPatch']((...args: unknown[]) => Promise.resolve()) - .bind({ middlewareStack: mockedMiddlewareStack }); + .bind({ middlewareStack: mockedMiddlewareStack, config: mockConfig }); middlewareArgsHeader = { request: { @@ -617,6 +678,18 @@ describe('InstrumentationPatchTest', () => { expect(mockedMiddlewareStackInternal[0][1].name).toEqual('_adotInjectXrayContextMiddleware'); }); + + it('Add cross account information span attributes from STS credentials', async () => { + const mockSpan = { setAttribute: sinon.stub() }; + sinon.stub(trace, 'getSpan').returns(mockSpan as unknown as Span); + const credentialsMiddlewareArgs: any = {}; + await mockedMiddlewareStackInternal[1][0]((arg: any) => Promise.resolve(arg), null)(credentialsMiddlewareArgs); + expect(mockedMiddlewareStackInternal[1][1].name).toEqual('_extractSignerCredentials'); + expect( + mockSpan.setAttribute.calledWith(AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCESS_KEY, 'test-access-key') + ).toBeTruthy(); + expect(mockSpan.setAttribute.calledWith(AWS_ATTRIBUTE_KEYS.AWS_AUTH_REGION, 'us-west-2')).toBeTruthy(); + }); }); it('injects trace context header into request via propagator', async () => { From 896204c6ceb3b9f685f2c3bca026ac55f1cb03c3 Mon Sep 17 00:00:00 2001 From: Blair Huang Date: Thu, 12 Jun 2025 16:13:43 -0700 Subject: [PATCH 02/10] Generate cross-account metric attributes --- .../src/aws-metric-attribute-generator.ts | 86 ++++- .../src/regional-resource-arn-parser.ts | 52 +++ .../src/sqs-url-parser.ts | 51 +++ .../aws-metric-attribute-generator.test.ts | 304 +++++++++++++++--- .../test/regional-resource-arn-parser.test.ts | 43 +++ .../test/sqs-url-parser.test.ts | 102 ++++-- 6 files changed, 563 insertions(+), 75 deletions(-) create mode 100644 aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts create mode 100644 aws-distro-opentelemetry-node-autoinstrumentation/test/regional-resource-arn-parser.test.ts diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-metric-attribute-generator.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-metric-attribute-generator.ts index c717fe7b..fc624622 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-metric-attribute-generator.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-metric-attribute-generator.ts @@ -32,6 +32,7 @@ import { MetricAttributeGenerator, SERVICE_METRIC, } from './metric-attribute-generator'; +import { RegionalResourceArnParser } from './regional-resource-arn-parser'; import { SqsUrlParser } from './sqs-url-parser'; import { LAMBDA_APPLICATION_SIGNALS_REMOTE_ENVIRONMENT } from './aws-opentelemetry-configurator'; @@ -112,8 +113,20 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { AwsMetricAttributeGenerator.setService(resource, span, attributes); AwsMetricAttributeGenerator.setEgressOperation(span, attributes); AwsMetricAttributeGenerator.setRemoteServiceAndOperation(span, attributes); - AwsMetricAttributeGenerator.setRemoteResourceTypeAndIdentifier(span, attributes); + const isRemoteResourceIdentifierPresent = AwsMetricAttributeGenerator.setRemoteResourceTypeAndIdentifier( + span, + attributes + ); AwsMetricAttributeGenerator.setRemoteEnvironment(span, attributes); + if (isRemoteResourceIdentifierPresent) { + const isAccountIdAndRegionPresent = AwsMetricAttributeGenerator.setRemoteResourceAccountIdAndRegion( + span, + attributes + ); + if (!isAccountIdAndRegionPresent) { + AwsMetricAttributeGenerator.setRemoteResourceAccessKeyAndRegion(span, attributes); + } + } AwsMetricAttributeGenerator.setSpanKindForDependency(span, attributes); AwsMetricAttributeGenerator.setRemoteDbUser(span, attributes); @@ -369,7 +382,7 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { * href="https://docs.aws.amazon.com/cloudcontrolapi/latest/userguide/supported-resources.html">AWS * Cloud Control resource format. */ - private static setRemoteResourceTypeAndIdentifier(span: ReadableSpan, attributes: Attributes): void { + private static setRemoteResourceTypeAndIdentifier(span: ReadableSpan, attributes: Attributes): boolean { let remoteResourceType: AttributeValue | undefined; let remoteResourceIdentifier: AttributeValue | undefined; let cloudFormationIdentifier: AttributeValue | undefined; @@ -383,11 +396,27 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { ) { remoteResourceType = NORMALIZED_DYNAMO_DB_SERVICE_NAME + '::Table'; remoteResourceIdentifier = AwsMetricAttributeGenerator.escapeDelimiters(awsTableNames[0]); + } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN)) { + remoteResourceType = NORMALIZED_DYNAMO_DB_SERVICE_NAME + '::Table'; + remoteResourceIdentifier = AwsMetricAttributeGenerator.escapeDelimiters( + this.extractResourceNameFromArn(span.attributes[AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN])?.replace( + 'table/', + '' + ) + ); } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_NAME)) { remoteResourceType = NORMALIZED_KINESIS_SERVICE_NAME + '::Stream'; remoteResourceIdentifier = AwsMetricAttributeGenerator.escapeDelimiters( span.attributes[AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_NAME] ); + } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN)) { + remoteResourceType = NORMALIZED_KINESIS_SERVICE_NAME + '::Stream'; + remoteResourceIdentifier = AwsMetricAttributeGenerator.escapeDelimiters( + this.extractResourceNameFromArn(span.attributes[AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN])?.replace( + 'stream/', + '' + ) + ); } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_S3_BUCKET)) { remoteResourceType = NORMALIZED_S3_SERVICE_NAME + '::Bucket'; remoteResourceIdentifier = AwsMetricAttributeGenerator.escapeDelimiters( @@ -500,7 +529,10 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { attributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_TYPE] = remoteResourceType; attributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_IDENTIFIER] = remoteResourceIdentifier; attributes[AWS_ATTRIBUTE_KEYS.AWS_CLOUDFORMATION_PRIMARY_IDENTIFIER] = cloudFormationIdentifier; + return true; } + + return false; } /** @@ -522,6 +554,56 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { } } + private static setRemoteResourceAccountIdAndRegion(span: ReadableSpan, attributes: Attributes): boolean { + const ARN_ATTRIBUTES: string[] = [ + AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN, + AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN, + AWS_ATTRIBUTE_KEYS.AWS_SNS_TOPIC_ARN, + AWS_ATTRIBUTE_KEYS.AWS_SECRETSMANAGER_SECRET_ARN, + AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, + AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_ACTIVITY_ARN, + AWS_ATTRIBUTE_KEYS.AWS_LAMBDA_FUNCTION_ARN, + AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_GUARDRAIL_ARN, + ]; + let remoteResourceAccountId: string | undefined = undefined; + let remoteResourceRegion: string | undefined = undefined; + + if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_URL)) { + const sqsQueueUrl = AwsMetricAttributeGenerator.escapeDelimiters( + span.attributes[AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_URL] + ); + remoteResourceAccountId = SqsUrlParser.getAccountId(sqsQueueUrl); + remoteResourceRegion = SqsUrlParser.getRegion(sqsQueueUrl); + } else { + for (const attributeKey of ARN_ATTRIBUTES) { + if (AwsSpanProcessingUtil.isKeyPresent(span, attributeKey)) { + const arn = span.attributes[attributeKey]; + remoteResourceAccountId = RegionalResourceArnParser.getAccountId(arn); + remoteResourceRegion = RegionalResourceArnParser.getRegion(arn); + break; + } + } + } + + if (remoteResourceAccountId !== undefined && remoteResourceRegion !== undefined) { + attributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID] = remoteResourceAccountId; + attributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_REGION] = remoteResourceRegion; + return true; + } + + return false; + } + + private static setRemoteResourceAccessKeyAndRegion(span: ReadableSpan, attributes: Attributes): void { + if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCESS_KEY)) { + attributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCESS_KEY] = + span.attributes[AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCESS_KEY]; + } + if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_AUTH_REGION)) { + attributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_REGION] = span.attributes[AWS_ATTRIBUTE_KEYS.AWS_AUTH_REGION]; + } + } + /** * RemoteResourceIdentifier is populated with rule * ^[{db.name}|]?{address}[|{port}]? diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts new file mode 100644 index 00000000..91394469 --- /dev/null +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts @@ -0,0 +1,52 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { AttributeValue } from '@opentelemetry/api'; + +export class RegionalResourceArnParser { + public static getAccountId(arn: AttributeValue | undefined): string | undefined { + if (this.isArn(arn)) { + return (arn! as string).split(':')[4]; + } + return undefined; + } + + public static getRegion(arn: AttributeValue | undefined): string | undefined { + if (this.isArn(arn)) { + return (arn! as string).split(':')[3]; + } + return undefined; + } + + public static isArn(arn: AttributeValue | undefined): boolean { + // Check if arn follows the format: + // arn:partition:service:region:account-id:resource-type/resource-id or + // arn:partition:service:region:account-id:resource-type:resource-id + if (!arn || typeof arn !== 'string') { + return false; + } + + if (!arn.startsWith('arn')) { + return false; + } + + const arnParts = arn.split(':'); + return arnParts.length >= 6 && this.isAccountId(arnParts[4]); + } + + private static isAccountId(input: string): boolean { + if (input == null || input.length !== 12) { + return false; + } + + if (!this._checkDigits(input)) { + return false; + } + + return true; + } + + private static _checkDigits(str: string): boolean { + return /^\d+$/.test(str); + } +} diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts index 37d0a1bc..fcb1fa30 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts @@ -30,6 +30,57 @@ export class SqsUrlParser { return undefined; } + /** + * Extracts the account ID from an SQS URL. + */ + public static getAccountId(url: AttributeValue | undefined): string | undefined { + if (typeof url !== 'string') { + return undefined; + } + + url = url.replace(HTTP_SCHEMA, '').replace(HTTPS_SCHEMA, ''); + if (this.isValidSqsUrl(url)) { + const splitUrl: string[] = url.split('/'); + return splitUrl[1]; + } + + return undefined; + } + + /** + * Extracts the region from an SQS URL. + */ + public static getRegion(url: AttributeValue | undefined): string | undefined { + if (typeof url !== 'string') { + return undefined; + } + + url = url.replace(HTTP_SCHEMA, '').replace(HTTPS_SCHEMA, ''); + if (this.isValidSqsUrl(url)) { + const splitUrl: string[] = url.split('/'); + const domain: string = splitUrl[0]; + const domainParts: string[] = domain.split('.'); + if (domainParts.length === 4) { + return domainParts[1]; + } + } + + return undefined; + } + + /** + * Checks if the URL is a valid SQS URL. + */ + private static isValidSqsUrl(url: string): boolean { + const splitUrl: string[] = url.split('/'); + return ( + splitUrl.length === 3 && + splitUrl[0].toLowerCase().startsWith('sqs') && + this.isAccountId(splitUrl[1]) && + this.isValidQueueName(splitUrl[2]) + ); + } + private static isAccountId(input: string): boolean { if (input == null || input.length !== 12) { return false; diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts b/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts index f860e270..eadad05b 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts @@ -61,6 +61,8 @@ const UNKNOWN_REMOTE_SERVICE: string = 'UnknownRemoteService'; const UNKNOWN_REMOTE_OPERATION: string = 'UnknownRemoteOperation'; const INTERNAL_OPERATION: string = 'InternalOperation'; const LOCAL_ROOT: string = 'LOCAL_ROOT'; +const AWS_REMOTE_RESOURCE_REGION: string = 'us-east-1'; +const AWS_REMOTE_RESOURCE_ACCESS_KEY: string = 'AWS access key'; const GENERATOR: AwsMetricAttributeGenerator = new AwsMetricAttributeGenerator(); @@ -731,28 +733,49 @@ describe('AwsMetricAttributeGeneratorTest', () => { it('testSdkClientSpanWithRemoteResourceAttributes', () => { mockAttribute(SEMATTRS_RPC_SYSTEM, 'aws-api'); + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCESS_KEY, AWS_REMOTE_RESOURCE_ACCESS_KEY); + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_REGION, AWS_REMOTE_RESOURCE_REGION); // Validate behaviour of aws bucket name attribute, then remove it. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_S3_BUCKET, 'aws_s3_bucket_name'); - validateRemoteResourceAttributes('AWS::S3::Bucket', 'aws_s3_bucket_name'); + validateRemoteResourceAttributes( + 'AWS::S3::Bucket', + 'aws_s3_bucket_name', + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_S3_BUCKET, undefined); // Validate behaviour of AWS_SQS_QUEUE_NAME attribute, then remove it. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_NAME, 'aws_queue_name'); - validateRemoteResourceAttributes('AWS::SQS::Queue', 'aws_queue_name'); + validateRemoteResourceAttributes( + 'AWS::SQS::Queue', + 'aws_queue_name', + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_NAME, undefined); // Validate behaviour of having both AWS_SQS_QUEUE_NAME and AWS_SQS_QUEUE_URL attribute, then remove // them. Queue name is more reliable than queue URL, so we prefer to use name over URL. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_URL, 'https://sqs.us-east-2.amazonaws.com/123456789012/Queue'); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_NAME, 'aws_queue_name'); - validateRemoteResourceAttributes('AWS::SQS::Queue', 'aws_queue_name'); + validateRemoteResourceAttributes('AWS::SQS::Queue', 'aws_queue_name', 'us-east-2', '123456789012', undefined); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_URL, undefined); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_NAME, undefined); // Valid queue name with invalid queue URL, we should default to using the queue name. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_URL, 'invalidUrl'); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_NAME, 'aws_queue_name'); - validateRemoteResourceAttributes('AWS::SQS::Queue', 'aws_queue_name'); + validateRemoteResourceAttributes( + 'AWS::SQS::Queue', + 'aws_queue_name', + undefined, + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_URL, undefined); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_NAME, undefined); @@ -767,12 +790,33 @@ describe('AwsMetricAttributeGeneratorTest', () => { // Validate behaviour of AWS_KINESIS_STREAM_NAME attribute, then remove it. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_NAME, 'AWS_KINESIS_STREAM_NAME'); - validateRemoteResourceAttributes('AWS::Kinesis::Stream', 'AWS_KINESIS_STREAM_NAME'); + validateRemoteResourceAttributes( + 'AWS::Kinesis::Stream', + 'AWS_KINESIS_STREAM_NAME', + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_NAME, undefined); + // Validate behaviour of AWS_KINESIS_STREAM_ARN attribute, then remove it. + mockAttribute( + AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN, + 'arn:aws:kinesis:us-west-2:123456789012:stream/aws_stream_name' + ); + validateRemoteResourceAttributes('AWS::Kinesis::Stream', 'aws_stream_name', 'us-west-2', '123456789012', undefined); + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN, undefined); + // Validate behaviour of AWS_SNS_TOPIC_ARN attribute then remove it. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SNS_TOPIC_ARN, 'arn:aws:sns:us-east-1:123456789012:testTopic'); - validateRemoteResourceAttributes('AWS::SNS::Topic', 'testTopic', 'arn:aws:sns:us-east-1:123456789012:testTopic'); + validateRemoteResourceAttributes( + 'AWS::SNS::Topic', + 'testTopic', + 'arn:aws:sns:us-east-1:123456789012:testTopic', + 'us-east-1', + '123456789012', + undefined + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SNS_TOPIC_ARN, undefined); // Validate behaviour of AWS_SECRETSMANAGER_SECRET_ARN attributes then remove it. @@ -783,7 +827,10 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::SecretsManager::Secret', 'testSecret', - 'arn:aws:secretsmanager:us-east-1:123456789123:secret:testSecret' + 'arn:aws:secretsmanager:us-east-1:123456789123:secret:testSecret', + 'us-east-1', + '123456789123', + undefined ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SECRETSMANAGER_SECRET_ARN, undefined); @@ -796,14 +843,23 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::Lambda::Function', 'aws_lambda_function_name', - 'arn:aws:lambda:us-east-1:123456789012:function:aws_lambda_function_name' + 'arn:aws:lambda:us-east-1:123456789012:function:aws_lambda_function_name', + 'us-east-1', + '123456789012', + undefined ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_LAMBDA_FUNCTION_NAME, undefined); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_LAMBDA_FUNCTION_ARN, undefined); // Validate behaviour of AWS_LAMBDA_RESOURCE_MAPPING_ID attribute then remove it. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_LAMBDA_RESOURCE_MAPPING_ID, 'aws_lambda_resource_mapping_id'); - validateRemoteResourceAttributes('AWS::Lambda::EventSourceMapping', 'aws_lambda_resource_mapping_id'); + validateRemoteResourceAttributes( + 'AWS::Lambda::EventSourceMapping', + 'aws_lambda_resource_mapping_id', + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_LAMBDA_RESOURCE_MAPPING_ID, undefined); // Validate behaviour of AWS_STEPFUNCTIONS_STATEMACHINE_ARN and AWS_STEPFUNCTIONS_ACTIVITY_ARN attributes then remove them. @@ -813,8 +869,12 @@ describe('AwsMetricAttributeGeneratorTest', () => { ); validateRemoteResourceAttributes( 'AWS::StepFunctions::StateMachine', + 'testStateMachine', - 'arn:aws:states:us-east-1:123456789123:stateMachine:testStateMachine' + 'arn:aws:states:us-east-1:123456789123:stateMachine:testStateMachine', + 'us-east-1', + '123456789123', + undefined ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, undefined); @@ -824,14 +884,24 @@ describe('AwsMetricAttributeGeneratorTest', () => { ); validateRemoteResourceAttributes( 'AWS::StepFunctions::Activity', + 'testActivity', - 'arn:aws:states:us-east-1:123456789123:activity:testActivity' + 'arn:aws:states:us-east-1:123456789123:activity:testActivity', + 'us-east-1', + '123456789123', + undefined ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_ACTIVITY_ARN, undefined); // Validate behaviour of AWS_TABLE_NAMES attribute with one table name, then remove it. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_NAMES, ['aws_table_name']); - validateRemoteResourceAttributes('AWS::DynamoDB::Table', 'aws_table_name'); + validateRemoteResourceAttributes( + 'AWS::DynamoDB::Table', + 'aws_table_name', + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_NAMES, undefined); // Validate behaviour of AWS_TABLE_NAMES attribute with no table name, then remove it. @@ -846,28 +916,67 @@ describe('AwsMetricAttributeGeneratorTest', () => { // Validate behaviour of AWS_TABLE_NAMES attribute with special chars(|), then remove it. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_NAMES, ['aws_table|name']); - validateRemoteResourceAttributes('AWS::DynamoDB::Table', 'aws_table^|name'); + validateRemoteResourceAttributes( + 'AWS::DynamoDB::Table', + 'aws_table^|name', + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_NAMES, undefined); // Validate behaviour of AWS_TABLE_NAMES attribute with special chars(^), then remove it. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_NAMES, ['aws_table^name']); - validateRemoteResourceAttributes('AWS::DynamoDB::Table', 'aws_table^^name'); + validateRemoteResourceAttributes( + 'AWS::DynamoDB::Table', + 'aws_table^^name', + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_NAMES, undefined); + // Validate behaviour of AWS_DYNAMODB_TABLE_ARN attribute, then remove it. + mockAttribute( + AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN, + 'arn:aws:dynamodb:us-west-2:123456789012:table/aws_table_name' + ); + validateRemoteResourceAttributes('AWS::DynamoDB::Table', 'aws_table_name', 'us-west-2', '123456789012', undefined); + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN, undefined); + // Validate behaviour of AWS_BEDROCK_AGENT_ID attribute, then remove it. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_AGENT_ID, 'test_agent_id'); - validateRemoteResourceAttributes('AWS::Bedrock::Agent', 'test_agent_id'); + validateRemoteResourceAttributes( + 'AWS::Bedrock::Agent', + 'test_agent_id', + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_NAMES, undefined); // Validate behaviour of AWS_BEDROCK_AGENT_ID attribute with special chars(^), then remove it. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_AGENT_ID, 'test_agent_^id'); - validateRemoteResourceAttributes('AWS::Bedrock::Agent', 'test_agent_^^id'); + validateRemoteResourceAttributes( + 'AWS::Bedrock::Agent', + 'test_agent_^^id', + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_AGENT_ID, undefined); // Validate behaviour of AWS_BEDROCK_DATA_SOURCE_ID attribute, then remove it. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_DATA_SOURCE_ID, 'test_datasource_id'); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_KNOWLEDGE_BASE_ID, 'test_kb_id'); - validateRemoteResourceAttributes('AWS::Bedrock::DataSource', 'test_datasource_id', 'test_kb_id|test_datasource_id'); + validateRemoteResourceAttributes( + 'AWS::Bedrock::DataSource', + 'test_datasource_id', + 'test_kb_id|test_datasource_id', + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_DATA_SOURCE_ID, undefined); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_KNOWLEDGE_BASE_ID, undefined); @@ -876,8 +985,12 @@ describe('AwsMetricAttributeGeneratorTest', () => { mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_KNOWLEDGE_BASE_ID, 'test_kb_^id'); validateRemoteResourceAttributes( 'AWS::Bedrock::DataSource', + 'test_datasource_^^id', - 'test_kb_^^id|test_datasource_^^id' + 'test_kb_^^id|test_datasource_^^id', + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_DATA_SOURCE_ID, undefined); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_KNOWLEDGE_BASE_ID, undefined); @@ -890,8 +1003,12 @@ describe('AwsMetricAttributeGeneratorTest', () => { ); validateRemoteResourceAttributes( 'AWS::Bedrock::Guardrail', + 'test_guardrail_id', - 'arn:aws:bedrock:us-east-1:123456789012:guardrail/test_guardrail_id' + 'arn:aws:bedrock:us-east-1:123456789012:guardrail/test_guardrail_id', + 'us-east-1', + '123456789012', + undefined ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_GUARDRAIL_ID, undefined); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_GUARDRAIL_ARN, undefined); @@ -904,31 +1021,113 @@ describe('AwsMetricAttributeGeneratorTest', () => { ); validateRemoteResourceAttributes( 'AWS::Bedrock::Guardrail', + 'test_guardrail_^^id', - 'arn:aws:bedrock:us-east-1:123456789012:guardrail/test_guardrail_^^id' + 'arn:aws:bedrock:us-east-1:123456789012:guardrail/test_guardrail_^^id', + 'us-east-1', + '123456789012', + undefined ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_GUARDRAIL_ID, undefined); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_GUARDRAIL_ARN, undefined); // Validate behaviour of AWS_BEDROCK_KNOWLEDGE_BASE_ID attribute, then remove it. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_KNOWLEDGE_BASE_ID, 'test_knowledgeBase_id'); - validateRemoteResourceAttributes('AWS::Bedrock::KnowledgeBase', 'test_knowledgeBase_id'); + validateRemoteResourceAttributes( + 'AWS::Bedrock::KnowledgeBase', + 'test_knowledgeBase_id', + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_KNOWLEDGE_BASE_ID, undefined); // Validate behaviour of AWS_BEDROCK_KNOWLEDGE_BASE_ID attribute with special chars(^), then remove it. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_KNOWLEDGE_BASE_ID, 'test_knowledgeBase_^id'); - validateRemoteResourceAttributes('AWS::Bedrock::KnowledgeBase', 'test_knowledgeBase_^^id'); + validateRemoteResourceAttributes( + 'AWS::Bedrock::KnowledgeBase', + 'test_knowledgeBase_^^id', + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_KNOWLEDGE_BASE_ID, undefined); // Validate behaviour of GEN_AI_REQUEST_MODEL attribute, then remove it. mockAttribute(AwsSpanProcessingUtil.GEN_AI_REQUEST_MODEL, 'test.service_id'); - validateRemoteResourceAttributes('AWS::Bedrock::Model', 'test.service_id'); + validateRemoteResourceAttributes( + 'AWS::Bedrock::Model', + 'test.service_id', + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY + ); mockAttribute(AwsSpanProcessingUtil.GEN_AI_REQUEST_MODEL, undefined); // Validate behaviour of GEN_AI_REQUEST_MODEL attribute with special chars(^), then remove it. mockAttribute(AwsSpanProcessingUtil.GEN_AI_REQUEST_MODEL, 'test.service_^id'); - validateRemoteResourceAttributes('AWS::Bedrock::Model', 'test.service_^^id'); + validateRemoteResourceAttributes( + 'AWS::Bedrock::Model', + 'test.service_^^id', + AWS_REMOTE_RESOURCE_REGION, + undefined, + AWS_REMOTE_RESOURCE_ACCESS_KEY + ); mockAttribute(AwsSpanProcessingUtil.GEN_AI_REQUEST_MODEL, undefined); + + // Cross account support + // Invalid arn but account access key is available + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, 'invalid_arn'); + validateRemoteResourceAttributes(undefined, undefined); + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, undefined); + + // Invalid arn and no account access key + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCESS_KEY, undefined); + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_REGION, undefined); + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, 'invalid_arn'); + validateRemoteResourceAttributes(undefined, undefined); + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, undefined); + + // Both account access key and account id are not available + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_S3_BUCKET, 'aws_s3_bucket_name'); + validateRemoteResourceAttributes('AWS::S3::Bucket', 'aws_s3_bucket_name'); + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_S3_BUCKET, undefined); + + // Account access key is not available + mockAttribute( + AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, + 'arn:aws:states:us-east-1:123456789123:stateMachine:testStateMachine' + ); + validateRemoteResourceAttributes( + 'AWS::StepFunctions::StateMachine', + 'testStateMachine', + 'us-east-1', + '123456789123', + undefined + ); + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, undefined); + + // Arn with invalid account id + mockAttribute( + AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, + 'arn:aws:states:us-east-1:invalid_account_id:stateMachine:testStateMachine' + ); + validateRemoteResourceAttributes('AWS::StepFunctions::StateMachine', 'testStateMachine'); + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, undefined); + + // Arn with invalid region + mockAttribute( + AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, + 'arn:aws:states:invalid_region:123456789123:stateMachine:testStateMachine' + ); + validateRemoteResourceAttributes( + 'AWS::StepFunctions::StateMachine', + 'testStateMachine', + 'invalid_region', + '123456789123', + undefined + ); + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, undefined); }); it('testDBClientSpanWithRemoteResourceAttributes', () => { @@ -1168,7 +1367,10 @@ describe('AwsMetricAttributeGeneratorTest', () => { function validateRemoteResourceAttributes( type: string | undefined, identifier: string | undefined, - cfnPrimaryId: string | undefined = undefined + cfnPrimaryId: string | undefined = undefined, + region: string | undefined = undefined, + accountId: string | undefined = undefined, + accessKey: string | undefined = undefined ): void { // If cfnPrimaryId is not provided, it defaults to identifier if (cfnPrimaryId === undefined) { @@ -1176,32 +1378,48 @@ describe('AwsMetricAttributeGeneratorTest', () => { } // Client, Producer and Consumer spans should generate the expected remote resource attributes - (spanDataMock as any).kind = SpanKind.CLIENT; - let actualAttributes: Attributes = GENERATOR.generateMetricAttributeMapFromSpan(spanDataMock, resource)[ - DEPENDENCY_METRIC - ]; - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_TYPE]).toEqual(type); - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_IDENTIFIER]).toEqual(identifier); - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_CLOUDFORMATION_PRIMARY_IDENTIFIER]).toEqual(cfnPrimaryId); + const spanKinds = [SpanKind.CLIENT, SpanKind.PRODUCER, SpanKind.CONSUMER]; + + for (const kind of spanKinds) { + (spanDataMock as any).kind = kind; + const actualAttributes: Attributes = GENERATOR.generateMetricAttributeMapFromSpan(spanDataMock, resource)[ + DEPENDENCY_METRIC + ]; + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_TYPE]).toEqual(type); + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_IDENTIFIER]).toEqual(identifier); + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_CLOUDFORMATION_PRIMARY_IDENTIFIER]).toEqual(cfnPrimaryId); + + // Cross account support + if (region !== undefined) { + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_REGION]).toEqual(region); + } else { + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_REGION]).toEqual(undefined); + } - (spanDataMock as any).kind = SpanKind.PRODUCER; - actualAttributes = GENERATOR.generateMetricAttributeMapFromSpan(spanDataMock, resource)[DEPENDENCY_METRIC]; - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_TYPE]).toEqual(type); - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_IDENTIFIER]).toEqual(identifier); - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_CLOUDFORMATION_PRIMARY_IDENTIFIER]).toEqual(cfnPrimaryId); + if (accountId !== undefined) { + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID]).toEqual(accountId); + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCESS_KEY]).toEqual(undefined); + } else { + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID]).toEqual(undefined); + } - (spanDataMock as any).kind = SpanKind.CONSUMER; - actualAttributes = GENERATOR.generateMetricAttributeMapFromSpan(spanDataMock, resource)[DEPENDENCY_METRIC]; - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_TYPE]).toEqual(type); - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_IDENTIFIER]).toEqual(identifier); - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_CLOUDFORMATION_PRIMARY_IDENTIFIER]).toEqual(cfnPrimaryId); + if (accessKey !== undefined) { + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCESS_KEY]).toEqual(accessKey); + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID]).toEqual(undefined); + } else { + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCESS_KEY]).toEqual(undefined); + } + } // Server span should not generate remote resource attributes (spanDataMock as any).kind = SpanKind.SERVER; - actualAttributes = GENERATOR.generateMetricAttributeMapFromSpan(spanDataMock, resource)[SERVICE_METRIC]; + const actualAttributes = GENERATOR.generateMetricAttributeMapFromSpan(spanDataMock, resource)[SERVICE_METRIC]; expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_TYPE]).toEqual(undefined); expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_IDENTIFIER]).toEqual(undefined); expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_CLOUDFORMATION_PRIMARY_IDENTIFIER]).toEqual(undefined); + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_REGION]).toEqual(undefined); + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID]).toEqual(undefined); + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCESS_KEY]).toEqual(undefined); } it('testDBUserAttribute', () => { diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/test/regional-resource-arn-parser.test.ts b/aws-distro-opentelemetry-node-autoinstrumentation/test/regional-resource-arn-parser.test.ts new file mode 100644 index 00000000..fc0bd6b5 --- /dev/null +++ b/aws-distro-opentelemetry-node-autoinstrumentation/test/regional-resource-arn-parser.test.ts @@ -0,0 +1,43 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { expect } from 'expect'; +import { RegionalResourceArnParser } from '../src/regional-resource-arn-parser'; + +describe('RegionalResourceArnParserTest', () => { + it('testGetAccountId', () => { + validateAccountId(undefined, undefined); + validateAccountId('', undefined); + validateAccountId(' ', undefined); + validateAccountId(':', undefined); + validateAccountId('::::::', undefined); + validateAccountId('not:an:arn:string', undefined); + validateAccountId('arn:aws:ec2:us-west-2:123456', undefined); + validateAccountId('arn:aws:ec2:us-west-2:1234567xxxxx', undefined); + validateAccountId('arn:aws:ec2:us-west-2:123456789012', undefined); + validateAccountId('arn:aws:dynamodb:us-west-2:123456789012:table/test_table', '123456789012'); + validateAccountId('arn:aws:acm:us-east-1:123456789012:certificate:abc-123', '123456789012'); + }); + + it('testGetRegion', () => { + validateRegion(undefined, undefined); + validateRegion('', undefined); + validateRegion(' ', undefined); + validateRegion(':', undefined); + validateRegion('::::::', undefined); + validateRegion('not:an:arn:string', undefined); + validateRegion('arn:aws:ec2:us-west-2:123456', undefined); + validateRegion('arn:aws:ec2:us-west-2:1234567xxxxx', undefined); + validateRegion('arn:aws:ec2:us-west-2:123456789012', undefined); + validateRegion('arn:aws:dynamodb:us-west-2:123456789012:table/test_table', 'us-west-2'); + validateRegion('arn:aws:acm:us-east-1:123456789012:certificate:abc-123', 'us-east-1'); + }); +}); + +function validateAccountId(arn: string | undefined, expectedAccountId: string | undefined): void { + expect(RegionalResourceArnParser.getAccountId(arn)).toEqual(expectedAccountId); +} + +function validateRegion(arn: string | undefined, expectedName: string | undefined): void { + expect(RegionalResourceArnParser.getRegion(arn)).toEqual(expectedName); +} diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/test/sqs-url-parser.test.ts b/aws-distro-opentelemetry-node-autoinstrumentation/test/sqs-url-parser.test.ts index 53f6e296..1777bd46 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/test/sqs-url-parser.test.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/test/sqs-url-parser.test.ts @@ -6,53 +6,95 @@ import { SqsUrlParser } from '../src/sqs-url-parser'; describe('SqsUrlParserTest', () => { it('testSqsClientSpanBasicUrls', async () => { - validate('https://sqs.us-east-1.amazonaws.com/123412341234/Q_Name-5', 'Q_Name-5'); - validate('https://sqs.af-south-1.amazonaws.com/999999999999/-_ThisIsValid', '-_ThisIsValid'); - validate('http://sqs.eu-west-3.amazonaws.com/000000000000/FirstQueue', 'FirstQueue'); - validate('sqs.sa-east-1.amazonaws.com/123456781234/SecondQueue', 'SecondQueue'); + validateGetQueueName('https://sqs.us-east-1.amazonaws.com/123412341234/Q_Name-5', 'Q_Name-5'); + validateGetQueueName('https://sqs.af-south-1.amazonaws.com/999999999999/-_ThisIsValid', '-_ThisIsValid'); + validateGetQueueName('http://sqs.eu-west-3.amazonaws.com/000000000000/FirstQueue', 'FirstQueue'); + validateGetQueueName('sqs.sa-east-1.amazonaws.com/123456781234/SecondQueue', 'SecondQueue'); }); it('testSqsClientSpanLegacyFormatUrls', () => { - validate('https://ap-northeast-2.queue.amazonaws.com/123456789012/MyQueue', 'MyQueue'); - validate('http://cn-northwest-1.queue.amazonaws.com/123456789012/MyQueue', 'MyQueue'); - validate('http://cn-north-1.queue.amazonaws.com/123456789012/MyQueue', 'MyQueue'); - validate('ap-south-1.queue.amazonaws.com/123412341234/MyLongerQueueNameHere', 'MyLongerQueueNameHere'); - validate('https://queue.amazonaws.com/123456789012/MyQueue', 'MyQueue'); + validateGetQueueName('https://ap-northeast-2.queue.amazonaws.com/123456789012/MyQueue', 'MyQueue'); + validateGetQueueName('http://cn-northwest-1.queue.amazonaws.com/123456789012/MyQueue', 'MyQueue'); + validateGetQueueName('http://cn-north-1.queue.amazonaws.com/123456789012/MyQueue', 'MyQueue'); + validateGetQueueName('ap-south-1.queue.amazonaws.com/123412341234/MyLongerQueueNameHere', 'MyLongerQueueNameHere'); + validateGetQueueName('https://queue.amazonaws.com/123456789012/MyQueue', 'MyQueue'); }); it('testSqsClientSpanCustomUrls', () => { - validate('http://127.0.0.1:1212/123456789012/MyQueue', 'MyQueue'); - validate('https://127.0.0.1:1212/123412341234/RRR', 'RRR'); - validate('127.0.0.1:1212/123412341234/QQ', 'QQ'); - validate('https://amazon.com/123412341234/BB', 'BB'); + validateGetQueueName('http://127.0.0.1:1212/123456789012/MyQueue', 'MyQueue'); + validateGetQueueName('https://127.0.0.1:1212/123412341234/RRR', 'RRR'); + validateGetQueueName('127.0.0.1:1212/123412341234/QQ', 'QQ'); + validateGetQueueName('https://amazon.com/123412341234/BB', 'BB'); }); it('testSqsClientSpanLongUrls', () => { const queueName: string = 'a'.repeat(80); - validate('http://127.0.0.1:1212/123456789012/' + queueName, queueName); + validateGetQueueName('http://127.0.0.1:1212/123456789012/' + queueName, queueName); const queueNameTooLong: string = 'a'.repeat(81); - validate('http://127.0.0.1:1212/123456789012/' + queueNameTooLong, undefined); + validateGetQueueName('http://127.0.0.1:1212/123456789012/' + queueNameTooLong, undefined); }); it('testClientSpanSqsInvalidOrEmptyUrls', () => { - validate(undefined, undefined); - validate('', undefined); - validate(' ', undefined); - validate('/', undefined); - validate('//', undefined); - validate('///', undefined); - validate('//asdf', undefined); - validate('/123412341234/as&df', undefined); - validate('invalidUrl', undefined); - validate('https://www.amazon.com', undefined); - validate('https://sqs.us-east-1.amazonaws.com/123412341234/.', undefined); - validate('https://sqs.us-east-1.amazonaws.com/12/Queue', undefined); - validate('https://sqs.us-east-1.amazonaws.com/A/A', undefined); - validate('https://sqs.us-east-1.amazonaws.com/123412341234/A/ThisShouldNotBeHere', undefined); + validateGetQueueName(undefined, undefined); + validateGetQueueName('', undefined); + validateGetQueueName(' ', undefined); + validateGetQueueName('/', undefined); + validateGetQueueName('//', undefined); + validateGetQueueName('///', undefined); + validateGetQueueName('//asdf', undefined); + validateGetQueueName('/123412341234/as&df', undefined); + validateGetQueueName('invalidUrl', undefined); + validateGetQueueName('https://www.amazon.com', undefined); + validateGetQueueName('https://sqs.us-east-1.amazonaws.com/123412341234/.', undefined); + validateGetQueueName('https://sqs.us-east-1.amazonaws.com/12/Queue', undefined); + validateGetQueueName('https://sqs.us-east-1.amazonaws.com/A/A', undefined); + validateGetQueueName('https://sqs.us-east-1.amazonaws.com/123412341234/A/ThisShouldNotBeHere', undefined); + }); + + it('testGetAccountId', () => { + validateGetAccountId(undefined, undefined); + validateGetAccountId('', undefined); + validateGetAccountId(' ', undefined); + validateGetAccountId('/', undefined); + validateGetAccountId('//', undefined); + validateGetAccountId('///', undefined); + validateGetAccountId('//asdf', undefined); + validateGetAccountId('/123412341234/as&df', undefined); + validateGetAccountId('invalidUrl', undefined); + validateGetAccountId('https://www.amazon.com', undefined); + validateGetAccountId('https://sqs.us-east-1.amazonaws.com/12341234/Queue', undefined); + validateGetAccountId('https://sqs.us-east-1.amazonaws.com/1234123412xx/Queue', undefined); + validateGetAccountId('https://sqs.us-east-1.amazonaws.com/1234123412xx', undefined); + validateGetAccountId('https://sqs.us-east-1.amazonaws.com/123412341234/Q_Namez-5', '123412341234'); + }); + + it('testGetRegion', () => { + validateGetRegion(undefined, undefined); + validateGetRegion('', undefined); + validateGetRegion(' ', undefined); + validateGetRegion('/', undefined); + validateGetRegion('//', undefined); + validateGetRegion('///', undefined); + validateGetRegion('//asdf', undefined); + validateGetRegion('/123412341234/as&df', undefined); + validateGetRegion('invalidUrl', undefined); + validateGetRegion('https://www.amazon.com', undefined); + validateGetRegion('https://sqs.us-east-1.amazonaws.com/12341234/Queue', undefined); + validateGetRegion('https://sqs.us-east-1.amazonaws.com/1234123412xx/Queue', undefined); + validateGetRegion('https://sqs.us-east-1.amazonaws.com/1234123412xx', undefined); + validateGetRegion('https://sqs.us-east-1.amazonaws.com/123412341234/Q_Namez-5', 'us-east-1'); }); }); -function validate(url: string | undefined, expectedName: string | undefined): void { +function validateGetRegion(url: string | undefined, expectedRegion: string | undefined): void { + expect(SqsUrlParser.getRegion(url)).toEqual(expectedRegion); +} + +function validateGetAccountId(url: string | undefined, expectedAccountId: string | undefined): void { + expect(SqsUrlParser.getAccountId(url)).toEqual(expectedAccountId); +} + +function validateGetQueueName(url: string | undefined, expectedName: string | undefined): void { expect(SqsUrlParser.getQueueName(url)).toEqual(expectedName); } From 1c92ea99b75120c979ad81d5a03d320f768677ac Mon Sep 17 00:00:00 2001 From: Blair Huang Date: Thu, 12 Jun 2025 16:42:42 -0700 Subject: [PATCH 03/10] Add contract tests --- .../images/applications/aws-sdk/server.js | 62 ++++++++- .../tests/test/amazon/aws-sdk/aws_sdk_test.py | 119 +++++++++++++++++- .../utils/application_signals_constants.py | 3 + 3 files changed, 179 insertions(+), 5 deletions(-) diff --git a/contract-tests/images/applications/aws-sdk/server.js b/contract-tests/images/applications/aws-sdk/server.js index 288bb967..4a63881e 100644 --- a/contract-tests/images/applications/aws-sdk/server.js +++ b/contract-tests/images/applications/aws-sdk/server.js @@ -9,9 +9,9 @@ const fetch = require('node-fetch'); const JSZip = require('jszip'); const { S3Client, CreateBucketCommand, PutObjectCommand, GetObjectCommand } = require('@aws-sdk/client-s3'); -const { DynamoDBClient, CreateTableCommand, PutItemCommand } = require('@aws-sdk/client-dynamodb'); +const { DynamoDBClient, CreateTableCommand, PutItemCommand, DescribeTableCommand } = require('@aws-sdk/client-dynamodb'); const { SQSClient, CreateQueueCommand, SendMessageCommand, ReceiveMessageCommand } = require('@aws-sdk/client-sqs'); -const { KinesisClient, CreateStreamCommand, PutRecordCommand } = require('@aws-sdk/client-kinesis'); +const { KinesisClient, CreateStreamCommand, PutRecordCommand, DescribeStreamCommand } = require('@aws-sdk/client-kinesis'); const { BedrockClient, GetGuardrailCommand } = require('@aws-sdk/client-bedrock'); const { BedrockAgentClient, GetKnowledgeBaseCommand, GetDataSourceCommand, GetAgentCommand } = require('@aws-sdk/client-bedrock-agent'); const { BedrockRuntimeClient, InvokeModelCommand } = require('@aws-sdk/client-bedrock-runtime'); @@ -194,6 +194,8 @@ async function handleGetRequest(req, res, path) { await handleSnsRequest(req, res, path); } else if (path.includes('lambda')) { await handleLambdaRequest(req, res, path); + } else if (path.includes('cross-account')) { + await handleCrossAccountRequest(req, res, path); } else { res.writeHead(404); res.end(); @@ -383,6 +385,19 @@ async function handleDdbRequest(req, res, path) { res.statusCode = 500; } res.end(); + } else if (path.includes('describe/some-table')) { + try { + const response = await ddbClient.send( + new DescribeTableCommand({ + TableName: 'put_test_table' + }) + ); + res.statusCode = 200; + } catch (err) { + console.log('Error describing the table', err); + res.statusCode = 500; + } + res.end(); } else { res.statusCode = 404; res.end(); @@ -532,6 +547,20 @@ async function handleKinesisRequest(req, res, path) { res.statusCode = 500; } res.end(); + } else if (path.includes('describe/my-stream')) { + try { + await kinesisClient.send( + new DescribeStreamCommand ({ + StreamName: 'test_stream', + StreamARN: 'arn:aws:kinesis:us-west-2:000000000000:stream/test_stream' + }) + ); + res.statusCode = 200; + } catch (err) { + console.log('Error describing the stream', err); + res.statusCode = 500; + } + res.end(); } else { res.statusCode = 404; res.end(); @@ -1041,6 +1070,35 @@ async function handleLambdaRequest(req, res, path) { res.end(); } +async function handleCrossAccountRequest(req, res, path) { + const s3Client = new S3Client({ + endpoint: _AWS_SDK_S3_ENDPOINT, + forcePathStyle: true, + region: "eu-central-1", + credentials: { + accessKeyId: "account_b_access_key_id", + secretAccessKey: "account_b_secret_access_key", + sessionToken: "account_b_token" + } + }); + + if (path.includes('createbucket/account_b')) { + try { + await s3Client.send( + new CreateBucketCommand({ + Bucket: "cross-account-bucket", + CreateBucketConfiguration: { LocationConstraint: "eu-central-1" } + }) + ); + res.statusCode = 200; + } catch (err) { + console.log('Error creating a bucket in another account', err); + res.statusCode = 500; + } + res.end(); + } +} + async function setupLambda() { const lambdaClient = new LambdaClient({ endpoint: _AWS_SDK_ENDPOINT, diff --git a/contract-tests/tests/test/amazon/aws-sdk/aws_sdk_test.py b/contract-tests/tests/test/amazon/aws-sdk/aws_sdk_test.py index 005e286b..24175fd1 100644 --- a/contract-tests/tests/test/amazon/aws-sdk/aws_sdk_test.py +++ b/contract-tests/tests/test/amazon/aws-sdk/aws_sdk_test.py @@ -19,7 +19,10 @@ AWS_REMOTE_RESOURCE_TYPE, AWS_REMOTE_SERVICE, AWS_SPAN_KIND, - AWS_CLOUDFORMATION_PRIMARY_IDENTIFIER + AWS_CLOUDFORMATION_PRIMARY_IDENTIFIER, + AWS_REMOTE_RESOURCE_ACCESS_KEY, + AWS_REMOTE_RESOURCE_ACCOUNT_ID, + AWS_REMOTE_RESOURCE_REGION ) from opentelemetry.proto.common.v1.common_pb2 import AnyValue, KeyValue from opentelemetry.proto.metrics.v1.metrics_pb2 import ExponentialHistogramDataPoint, Metric @@ -31,6 +34,7 @@ _AWS_SQS_QUEUE_URL: str = "aws.sqs.queue.url" _AWS_SQS_QUEUE_NAME: str = "aws.sqs.queue.name" +_AWS_KINESIS_STREAM_ARN: str = "aws.kinesis.stream.arn" _AWS_KINESIS_STREAM_NAME: str = "aws.kinesis.stream.name" _AWS_BEDROCK_AGENT_ID: str = "aws.bedrock.agent.id" _AWS_BEDROCK_GUARDRAIL_ID: str = "aws.bedrock.guardrail.id" @@ -49,6 +53,7 @@ _GEN_AI_RESPONSE_FINISH_REASONS: str = "gen_ai.response.finish_reasons" _GEN_AI_USAGE_INPUT_TOKENS: str = 'gen_ai.usage.input_tokens' _GEN_AI_USAGE_OUTPUT_TOKENS: str = 'gen_ai.usage.output_tokens' +_AWS_DYNAMODB_TABLE_ARN: str = "aws.dynamodb.table.arn" # pylint: disable=too-many-public-methods class AWSSDKTest(ContractTestBase): @@ -236,6 +241,30 @@ def test_dynamodb_put_item(self): span_name="DynamoDB.PutItem", ) + def test_dynamodb_describe_table(self): + self.do_test_requests( + "ddb/describe/some-table", + "GET", + 200, + 0, + 0, + local_operation="GET /ddb", + remote_service="AWS::DynamoDB", + remote_operation="DescribeTable", + remote_resource_type="AWS::DynamoDB::Table", + remote_resource_identifier="put_test_table", + cloudformation_primary_identifier="put_test_table", + remote_resource_account_id="000000000000", + remote_resource_region="us-west-2", + request_specific_attributes={ + SpanAttributes.AWS_DYNAMODB_TABLE_NAMES: ["put_test_table"], + }, + response_specific_attributes={ + _AWS_DYNAMODB_TABLE_ARN: r"arn:aws:dynamodb:us-west-2:000000000000:table/put_test_table", + }, + span_name="DynamoDB.DescribeTable", + ) + def test_dynamodb_error(self): self.do_test_requests( "ddb/error", @@ -397,6 +426,28 @@ def test_kinesis_put_record(self): }, span_name="Kinesis.PutRecord", ) + + def test_kinesis_describe_stream(self): + self.do_test_requests( + "kinesis/describe/my-stream", + "GET", + 200, + 0, + 0, + local_operation="GET /kinesis", + remote_service="AWS::Kinesis", + remote_operation="DescribeStream", + remote_resource_type="AWS::Kinesis::Stream", + remote_resource_identifier="test_stream", + cloudformation_primary_identifier="test_stream", + remote_resource_account_id="000000000000", + remote_resource_region="us-west-2", + request_specific_attributes={ + _AWS_KINESIS_STREAM_NAME: "test_stream", + _AWS_KINESIS_STREAM_ARN: "arn:aws:kinesis:us-west-2:000000000000:stream/test_stream" + }, + span_name="Kinesis.DescribeStream", + ) def test_kinesis_error(self): self.do_test_requests( @@ -1034,6 +1085,27 @@ def test_lambda_error(self): span_name="Lambda.GetEventSourceMapping", ) + def test_cross_account(self): + self.do_test_requests( + "cross-account/createbucket/account_b", + "GET", + 200, + 0, + 0, + local_operation="GET /cross-account", + remote_service="AWS::S3", + remote_operation="CreateBucket", + remote_resource_type="AWS::S3::Bucket", + remote_resource_identifier="cross-account-bucket", + cloudformation_primary_identifier="cross-account-bucket", + request_specific_attributes={ + SpanAttributes.AWS_S3_BUCKET: "cross-account-bucket", + }, + remote_resource_access_key="account_b_access_key_id", + remote_resource_region="eu-central-1", + span_name="S3.CreateBucket", + ) + #TODO: Need to add test_lambda_get_event_source_mapping once workaround is figured out for storing UUID between tests @override @@ -1062,6 +1134,9 @@ def _assert_aws_span_attributes(self, resource_scope_spans: List[ResourceScopeSp kwargs.get("remote_resource_type", "None"), kwargs.get("remote_resource_identifier", "None"), kwargs.get("cloudformation_primary_identifier", "None"), + kwargs.get("remote_resource_account_id", "None"), + kwargs.get("remote_resource_region", "None"), + kwargs.get("remote_resource_account_access_key", "None"), ) def _assert_aws_attributes( @@ -1073,7 +1148,10 @@ def _assert_aws_attributes( span_kind: str, remote_resource_type: str, remote_resource_identifier: str, - cloudformation_primary_identifier: str + cloudformation_primary_identifier: str, + remote_resource_account_id: str, + remote_resource_region: str, + remote_resource_account_access_key: str ) -> None: attributes_dict: Dict[str, AnyValue] = self._get_attributes_dict(attributes_list) self._assert_str_attribute(attributes_dict, AWS_LOCAL_SERVICE, self.get_application_otel_service_name()) @@ -1086,7 +1164,16 @@ def _assert_aws_attributes( self._assert_attribute(attributes_dict, AWS_REMOTE_RESOURCE_IDENTIFIER, remote_resource_identifier) if cloudformation_primary_identifier != "None": self._assert_attribute(attributes_dict, AWS_CLOUDFORMATION_PRIMARY_IDENTIFIER, cloudformation_primary_identifier) - + if remote_resource_account_id != "None": + assert remote_resource_identifier != "None" + self._assert_str_attribute(attributes_dict, AWS_REMOTE_RESOURCE_ACCOUNT_ID, remote_resource_account_id) + self.assertIsNone(attributes_dict.get(AWS_REMOTE_RESOURCE_ACCESS_KEY)) + if remote_resource_account_access_key != "None": + assert remote_resource_identifier != "None" + self._assert_str_attribute(attributes_dict, AWS_REMOTE_RESOURCE_ACCESS_KEY, remote_resource_account_access_key) + self.assertIsNone(attributes_dict.get(AWS_REMOTE_RESOURCE_ACCOUNT_ID)) + if remote_resource_region != "None": + self._assert_str_attribute(attributes_dict, AWS_REMOTE_RESOURCE_REGION, remote_resource_region) self._assert_str_attribute(attributes_dict, AWS_SPAN_KIND, span_kind) @override @@ -1187,10 +1274,23 @@ def _assert_metric_attributes( self._assert_attribute(attribute_dict, AWS_SPAN_KIND, kwargs.get("dependency_metric_span_kind") or "CLIENT") remote_resource_type = kwargs.get("remote_resource_type", "None") remote_resource_identifier = kwargs.get("remote_resource_identifier", "None") + remote_resource_account_id = kwargs.get("remote_resource_account_id", "None") + remote_resource_account_access_key = kwargs.get("remote_resource_account_access_key", "None") + remote_resource_region = kwargs.get("remote_resource_region", "None") if remote_resource_type != "None": self._assert_attribute(attribute_dict, AWS_REMOTE_RESOURCE_TYPE, remote_resource_type) if remote_resource_identifier != "None": self._assert_attribute(attribute_dict, AWS_REMOTE_RESOURCE_IDENTIFIER, remote_resource_identifier) + if remote_resource_account_id != "None": + assert remote_resource_identifier != "None" + self._assert_str_attribute(attribute_dict, AWS_REMOTE_RESOURCE_ACCOUNT_ID, remote_resource_account_id) + self.assertIsNone(attribute_dict.get(AWS_REMOTE_RESOURCE_ACCESS_KEY)) + if remote_resource_account_access_key != "None": + assert remote_resource_identifier != "None" + self._assert_str_attribute(attribute_dict, AWS_REMOTE_RESOURCE_ACCESS_KEY, remote_resource_account_access_key) + self.assertIsNone(attribute_dict.get(AWS_REMOTE_RESOURCE_ACCOUNT_ID)) + if remote_resource_region != "None": + self._assert_str_attribute(attribute_dict, AWS_REMOTE_RESOURCE_REGION, remote_resource_region) self.check_sum(metric_name, dependency_dp.sum, expected_sum) attribute_dict: Dict[str, AnyValue] = self._get_attributes_dict(service_dp.attributes) @@ -1212,10 +1312,23 @@ def _assert_metric_attributes( self._assert_str_attribute(attribute_dict, AWS_SPAN_KIND, kwargs.get("dependency_metric_span_kind") or "CLIENT") remote_resource_type = kwargs.get("remote_resource_type", "None") remote_resource_identifier = kwargs.get("remote_resource_identifier", "None") + remote_resource_account_id = kwargs.get("remote_resource_account_id", "None") + remote_resource_account_access_key = kwargs.get("remote_resource_account_access_key", "None") + remote_resource_region = kwargs.get("remote_resource_region", "None") if remote_resource_type != "None": self._assert_attribute(attribute_dict, AWS_REMOTE_RESOURCE_TYPE, remote_resource_type) if remote_resource_identifier != "None": self._assert_attribute(attribute_dict, AWS_REMOTE_RESOURCE_IDENTIFIER, remote_resource_identifier) + if remote_resource_account_id != "None": + assert remote_resource_identifier != "None" + self._assert_str_attribute(attribute_dict, AWS_REMOTE_RESOURCE_ACCOUNT_ID, remote_resource_account_id) + self.assertIsNone(attribute_dict.get(AWS_REMOTE_RESOURCE_ACCESS_KEY)) + if remote_resource_account_access_key != "None": + assert remote_resource_identifier != "None" + self._assert_str_attribute(attribute_dict, AWS_REMOTE_RESOURCE_ACCESS_KEY, remote_resource_account_access_key) + self.assertIsNone(attribute_dict.get(AWS_REMOTE_RESOURCE_ACCOUNT_ID)) + if remote_resource_region != "None": + self._assert_str_attribute(attribute_dict, AWS_REMOTE_RESOURCE_REGION, remote_resource_region) self.check_sum(metric_name, dependency_dp.sum, expected_sum) attribute_dict_service: Dict[str, AnyValue] = self._get_attributes_dict(service_dp.attributes) diff --git a/contract-tests/tests/test/amazon/utils/application_signals_constants.py b/contract-tests/tests/test/amazon/utils/application_signals_constants.py index 14b602e8..59a5920d 100644 --- a/contract-tests/tests/test/amazon/utils/application_signals_constants.py +++ b/contract-tests/tests/test/amazon/utils/application_signals_constants.py @@ -19,3 +19,6 @@ AWS_REMOTE_RESOURCE_IDENTIFIER: str = "aws.remote.resource.identifier" AWS_CLOUDFORMATION_PRIMARY_IDENTIFIER: str = 'aws.remote.resource.cfn.primary.identifier' AWS_SPAN_KIND: str = "aws.span.kind" +AWS_REMOTE_RESOURCE_ACCESS_KEY: str = "aws.remote.resource.account.access_key" +AWS_REMOTE_RESOURCE_ACCOUNT_ID: str = "aws.remote.resource.account.id" +AWS_REMOTE_RESOURCE_REGION: str = "aws.remote.resource.region" \ No newline at end of file From 6a4bfe1ea5162acbd1e755ec4ed255671b966d7a Mon Sep 17 00:00:00 2001 From: Blair Huang Date: Tue, 24 Jun 2025 14:06:09 -0700 Subject: [PATCH 04/10] improve data checking; update queue url & arn parser --- .../src/aws-attribute-keys.ts | 4 +- .../src/aws-metric-attribute-generator.ts | 24 +++-- .../src/patches/instrumentation-patch.ts | 33 +++--- .../src/regional-resource-arn-parser.ts | 37 ++----- .../src/sqs-url-parser.ts | 40 +++---- .../src/utils.ts | 16 +++ .../aws-metric-attribute-generator.test.ts | 100 ++++++++++++------ .../patches/instrumentation-patch.test.ts | 4 +- .../tests/test/amazon/aws-sdk/aws_sdk_test.py | 16 +-- .../utils/application_signals_constants.py | 2 +- 10 files changed, 150 insertions(+), 126 deletions(-) diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-attribute-keys.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-attribute-keys.ts index 1ad49aed..8b90204a 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-attribute-keys.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-attribute-keys.ts @@ -5,7 +5,7 @@ import { SEMATTRS_AWS_DYNAMODB_TABLE_NAMES } from '@opentelemetry/semantic-conve // Utility class holding attribute keys with special meaning to AWS components export const AWS_ATTRIBUTE_KEYS = { - AWS_AUTH_ACCESS_KEY: 'aws.auth.account.access_key', + AWS_AUTH_ACCOUNT_ACCESS_KEY: 'aws.auth.account.access_key', AWS_AUTH_REGION: 'aws.auth.region', AWS_SPAN_KIND: 'aws.span.kind', AWS_LOCAL_SERVICE: 'aws.local.service', @@ -13,7 +13,7 @@ export const AWS_ATTRIBUTE_KEYS = { AWS_REMOTE_SERVICE: 'aws.remote.service', AWS_REMOTE_ENVIRONMENT: 'aws.remote.environment', AWS_REMOTE_OPERATION: 'aws.remote.operation', - AWS_REMOTE_RESOURCE_ACCESS_KEY: 'aws.remote.resource.account.access_key', + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY: 'aws.remote.resource.account.access_key', AWS_REMOTE_RESOURCE_ACCOUNT_ID: 'aws.remote.resource.account.id', AWS_REMOTE_RESOURCE_REGION: 'aws.remote.resource.region', AWS_REMOTE_RESOURCE_TYPE: 'aws.remote.resource.type', diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-metric-attribute-generator.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-metric-attribute-generator.ts index fc624622..8d7159b9 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-metric-attribute-generator.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-metric-attribute-generator.ts @@ -399,10 +399,7 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN)) { remoteResourceType = NORMALIZED_DYNAMO_DB_SERVICE_NAME + '::Table'; remoteResourceIdentifier = AwsMetricAttributeGenerator.escapeDelimiters( - this.extractResourceNameFromArn(span.attributes[AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN])?.replace( - 'table/', - '' - ) + this.extractDynamoDbTableNameFromArn(span.attributes[AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN]) ); } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_NAME)) { remoteResourceType = NORMALIZED_KINESIS_SERVICE_NAME + '::Stream'; @@ -412,10 +409,7 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN)) { remoteResourceType = NORMALIZED_KINESIS_SERVICE_NAME + '::Stream'; remoteResourceIdentifier = AwsMetricAttributeGenerator.escapeDelimiters( - this.extractResourceNameFromArn(span.attributes[AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN])?.replace( - 'stream/', - '' - ) + this.extractKinesisStreamNameFromArn(span.attributes[AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN]) ); } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_S3_BUCKET)) { remoteResourceType = NORMALIZED_S3_SERVICE_NAME + '::Bucket'; @@ -595,9 +589,9 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { } private static setRemoteResourceAccessKeyAndRegion(span: ReadableSpan, attributes: Attributes): void { - if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCESS_KEY)) { - attributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCESS_KEY] = - span.attributes[AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCESS_KEY]; + if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCOUNT_ACCESS_KEY)) { + attributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY] = + span.attributes[AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCOUNT_ACCESS_KEY]; } if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_AUTH_REGION)) { attributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_REGION] = span.attributes[AWS_ATTRIBUTE_KEYS.AWS_AUTH_REGION]; @@ -731,6 +725,14 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { return rpcService === 'Lambda' && span.attributes[SEMATTRS_RPC_METHOD] === LAMBDA_INVOKE_OPERATION; } + private static extractDynamoDbTableNameFromArn(attribute: AttributeValue | undefined): string | undefined { + return this.extractResourceNameFromArn(attribute)?.replace('table/', ''); + } + + private static extractKinesisStreamNameFromArn(attribute: AttributeValue | undefined): string | undefined { + return this.extractResourceNameFromArn(attribute)?.replace('stream/', ''); + } + // Extracts the name of the resource from an arn private static extractResourceNameFromArn(attribute: AttributeValue | undefined): string | undefined { if (typeof attribute === 'string' && attribute.startsWith('arn:aws:')) { diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/patches/instrumentation-patch.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/patches/instrumentation-patch.ts index 8fe314e5..9194c1a1 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/patches/instrumentation-patch.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/patches/instrumentation-patch.ts @@ -233,22 +233,23 @@ function patchKinesisServiceExtension(kinesisServiceExtension: any): void { function patchDynamoDbServiceExtension(dynamoDbServiceExtension: any): void { if (dynamoDbServiceExtension) { if (typeof dynamoDbServiceExtension.responseHook === 'function') { - const originalResponseHook = dynamoDbServiceExtension.responseHook; - dynamoDbServiceExtension.responseHook = ( + const responseHook = dynamoDbServiceExtension.responseHook; + + const patchedResponseHook = ( response: NormalizedResponse, span: Span, tracer: Tracer, config: AwsSdkInstrumentationConfig ): void => { - originalResponseHook.call(dynamoDbServiceExtension, response, span, tracer, config); + responseHook.call(dynamoDbServiceExtension, response, span, tracer, config); - if (response.data && response.data.Table) { - const tableArn = response.data.Table.TableArn; - if (tableArn) { - span.setAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN, tableArn); - } + const tableArn = response?.data?.Table?.TableArn; + if (tableArn) { + span.setAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN, tableArn); } }; + + dynamoDbServiceExtension.responseHook = patchedResponseHook; } } } @@ -400,20 +401,24 @@ function patchAwsSdkInstrumentation(instrumentation: Instrumentation): void { if (span) { try { const credsProvider = this.config.credentials; - if (credsProvider) { + if (credsProvider instanceof Function) { const credentials = await credsProvider(); - if (credentials.accessKeyId) { - span.setAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCESS_KEY, credentials.accessKeyId); + if (credentials?.accessKeyId) { + span.setAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCOUNT_ACCESS_KEY, credentials.accessKeyId); } } - if (this.config.region) { + if (this.config.region instanceof Function) { const region = await this.config.region(); if (region) { span.setAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_REGION, region); } } } catch (err) { - diag.debug('Fail to get auth account access key and region.'); + diag.debug( + `Failed to get auth account access key and region: ${ + err instanceof Error ? err.message : String(err) + }` + ); } } @@ -421,7 +426,7 @@ function patchAwsSdkInstrumentation(instrumentation: Instrumentation): void { }, { step: 'build', - name: '_extractSignerCredentials', + name: '_adotExtractSignerCredentials', override: true, } ); diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts index 91394469..932ab911 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts @@ -2,51 +2,28 @@ // SPDX-License-Identifier: Apache-2.0 import { AttributeValue } from '@opentelemetry/api'; +import { isAccountId } from './utils'; export class RegionalResourceArnParser { public static getAccountId(arn: AttributeValue | undefined): string | undefined { - if (this.isArn(arn)) { - return (arn! as string).split(':')[4]; + if (typeof arn == 'string' && this.isArn(arn)) { + return arn.split(':')[4]; } return undefined; } public static getRegion(arn: AttributeValue | undefined): string | undefined { - if (this.isArn(arn)) { - return (arn! as string).split(':')[3]; + if (typeof arn == 'string' && this.isArn(arn)) { + return arn.split(':')[3]; } return undefined; } - public static isArn(arn: AttributeValue | undefined): boolean { + public static isArn(arn: string): boolean { // Check if arn follows the format: // arn:partition:service:region:account-id:resource-type/resource-id or // arn:partition:service:region:account-id:resource-type:resource-id - if (!arn || typeof arn !== 'string') { - return false; - } - - if (!arn.startsWith('arn')) { - return false; - } - const arnParts = arn.split(':'); - return arnParts.length >= 6 && this.isAccountId(arnParts[4]); - } - - private static isAccountId(input: string): boolean { - if (input == null || input.length !== 12) { - return false; - } - - if (!this._checkDigits(input)) { - return false; - } - - return true; - } - - private static _checkDigits(str: string): boolean { - return /^\d+$/.test(str); + return arnParts.length >= 6 && arnParts[0] === 'arn' && isAccountId(arnParts[4]); } } diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts index fcb1fa30..041d8fd8 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { AttributeValue } from '@opentelemetry/api'; +import { isAccountId } from './utils'; const HTTP_SCHEMA: string = 'http://'; const HTTPS_SCHEMA: string = 'https://'; @@ -22,9 +23,9 @@ export class SqsUrlParser { if (typeof url !== 'string') { return undefined; } - url = url.replace(HTTP_SCHEMA, '').replace(HTTPS_SCHEMA, ''); - const splitUrl: string[] = url.split('/'); - if (splitUrl.length === 3 && this.isAccountId(splitUrl[1]) && this.isValidQueueName(splitUrl[2])) { + const urlWithoutProtocol = this.removeProtocol(url); + const splitUrl: string[] = urlWithoutProtocol.split('/'); + if (splitUrl.length === 3 && isAccountId(splitUrl[1]) && this.isValidQueueName(splitUrl[2])) { return splitUrl[2]; } return undefined; @@ -38,9 +39,9 @@ export class SqsUrlParser { return undefined; } - url = url.replace(HTTP_SCHEMA, '').replace(HTTPS_SCHEMA, ''); if (this.isValidSqsUrl(url)) { - const splitUrl: string[] = url.split('/'); + const urlWithoutProtocol = this.removeProtocol(url); + const splitUrl: string[] = urlWithoutProtocol.split('/'); return splitUrl[1]; } @@ -55,9 +56,9 @@ export class SqsUrlParser { return undefined; } - url = url.replace(HTTP_SCHEMA, '').replace(HTTPS_SCHEMA, ''); if (this.isValidSqsUrl(url)) { - const splitUrl: string[] = url.split('/'); + const urlWithoutProtocol = this.removeProtocol(url); + const splitUrl: string[] = urlWithoutProtocol.split('/'); const domain: string = splitUrl[0]; const domainParts: string[] = domain.split('.'); if (domainParts.length === 4) { @@ -68,35 +69,24 @@ export class SqsUrlParser { return undefined; } + private static removeProtocol(url: string): string { + return url.replace(HTTP_SCHEMA, '').replace(HTTPS_SCHEMA, ''); + } + /** * Checks if the URL is a valid SQS URL. */ private static isValidSqsUrl(url: string): boolean { - const splitUrl: string[] = url.split('/'); + const urlWithoutProtocol = this.removeProtocol(url); + const splitUrl: string[] = urlWithoutProtocol.split('/'); return ( splitUrl.length === 3 && splitUrl[0].toLowerCase().startsWith('sqs') && - this.isAccountId(splitUrl[1]) && + isAccountId(splitUrl[1]) && this.isValidQueueName(splitUrl[2]) ); } - private static isAccountId(input: string): boolean { - if (input == null || input.length !== 12) { - return false; - } - - if (!this._checkDigits(input)) { - return false; - } - - return true; - } - - private static _checkDigits(str: string): boolean { - return /^\d+$/.test(str); - } - private static isValidQueueName(input: string): boolean { if (input == null || input.length === 0 || input.length > 80) { return false; diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/utils.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/utils.ts index adb7fa18..bd10b17f 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/utils.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/utils.ts @@ -55,3 +55,19 @@ export const getAwsRegionFromEnvironment = (): string | undefined => { return undefined; }; + +export const checkDigits = (str: string): boolean => { + return /^\d+$/.test(str); +}; + +export const isAccountId = (input: string): boolean => { + if (input == null || input.length !== 12) { + return false; + } + + if (!checkDigits(input)) { + return false; + } + + return true; +}; diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts b/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts index eadad05b..3e347ffa 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts @@ -62,7 +62,7 @@ const UNKNOWN_REMOTE_OPERATION: string = 'UnknownRemoteOperation'; const INTERNAL_OPERATION: string = 'InternalOperation'; const LOCAL_ROOT: string = 'LOCAL_ROOT'; const AWS_REMOTE_RESOURCE_REGION: string = 'us-east-1'; -const AWS_REMOTE_RESOURCE_ACCESS_KEY: string = 'AWS access key'; +const AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY: string = 'AWS access key'; const GENERATOR: AwsMetricAttributeGenerator = new AwsMetricAttributeGenerator(); @@ -733,16 +733,17 @@ describe('AwsMetricAttributeGeneratorTest', () => { it('testSdkClientSpanWithRemoteResourceAttributes', () => { mockAttribute(SEMATTRS_RPC_SYSTEM, 'aws-api'); - mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCESS_KEY, AWS_REMOTE_RESOURCE_ACCESS_KEY); + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCOUNT_ACCESS_KEY, AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_REGION, AWS_REMOTE_RESOURCE_REGION); // Validate behaviour of aws bucket name attribute, then remove it. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_S3_BUCKET, 'aws_s3_bucket_name'); validateRemoteResourceAttributes( 'AWS::S3::Bucket', 'aws_s3_bucket_name', + undefined, AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_S3_BUCKET, undefined); @@ -751,9 +752,10 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::SQS::Queue', 'aws_queue_name', + undefined, AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_NAME, undefined); @@ -761,7 +763,14 @@ describe('AwsMetricAttributeGeneratorTest', () => { // them. Queue name is more reliable than queue URL, so we prefer to use name over URL. mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_URL, 'https://sqs.us-east-2.amazonaws.com/123456789012/Queue'); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_NAME, 'aws_queue_name'); - validateRemoteResourceAttributes('AWS::SQS::Queue', 'aws_queue_name', 'us-east-2', '123456789012', undefined); + validateRemoteResourceAttributes( + 'AWS::SQS::Queue', + 'aws_queue_name', + undefined, + 'us-east-2', + '123456789012', + undefined + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_URL, undefined); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_NAME, undefined); @@ -774,7 +783,7 @@ describe('AwsMetricAttributeGeneratorTest', () => { undefined, AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_URL, undefined); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_NAME, undefined); @@ -784,7 +793,10 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::SQS::Queue', 'MyQueue', - 'https://sqs.us-east-2.amazonaws.com/123456789012/MyQueue' + 'https://sqs.us-east-2.amazonaws.com/123456789012/MyQueue', + 'us-east-2', + '123456789012', + undefined, ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_URL, undefined); @@ -793,9 +805,10 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::Kinesis::Stream', 'AWS_KINESIS_STREAM_NAME', + undefined, AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_NAME, undefined); @@ -804,7 +817,14 @@ describe('AwsMetricAttributeGeneratorTest', () => { AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN, 'arn:aws:kinesis:us-west-2:123456789012:stream/aws_stream_name' ); - validateRemoteResourceAttributes('AWS::Kinesis::Stream', 'aws_stream_name', 'us-west-2', '123456789012', undefined); + validateRemoteResourceAttributes( + 'AWS::Kinesis::Stream', + 'aws_stream_name', + undefined, + 'us-west-2', + '123456789012', + undefined + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN, undefined); // Validate behaviour of AWS_SNS_TOPIC_ARN attribute then remove it. @@ -856,9 +876,10 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::Lambda::EventSourceMapping', 'aws_lambda_resource_mapping_id', + undefined, AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_LAMBDA_RESOURCE_MAPPING_ID, undefined); @@ -869,7 +890,6 @@ describe('AwsMetricAttributeGeneratorTest', () => { ); validateRemoteResourceAttributes( 'AWS::StepFunctions::StateMachine', - 'testStateMachine', 'arn:aws:states:us-east-1:123456789123:stateMachine:testStateMachine', 'us-east-1', @@ -884,7 +904,6 @@ describe('AwsMetricAttributeGeneratorTest', () => { ); validateRemoteResourceAttributes( 'AWS::StepFunctions::Activity', - 'testActivity', 'arn:aws:states:us-east-1:123456789123:activity:testActivity', 'us-east-1', @@ -898,9 +917,10 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::DynamoDB::Table', 'aws_table_name', + undefined, AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_NAMES, undefined); @@ -919,9 +939,10 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::DynamoDB::Table', 'aws_table^|name', + undefined, AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_NAMES, undefined); @@ -930,9 +951,10 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::DynamoDB::Table', 'aws_table^^name', + undefined, AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_NAMES, undefined); @@ -941,7 +963,14 @@ describe('AwsMetricAttributeGeneratorTest', () => { AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN, 'arn:aws:dynamodb:us-west-2:123456789012:table/aws_table_name' ); - validateRemoteResourceAttributes('AWS::DynamoDB::Table', 'aws_table_name', 'us-west-2', '123456789012', undefined); + validateRemoteResourceAttributes( + 'AWS::DynamoDB::Table', + 'aws_table_name', + undefined, + 'us-west-2', + '123456789012', + undefined + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN, undefined); // Validate behaviour of AWS_BEDROCK_AGENT_ID attribute, then remove it. @@ -949,9 +978,10 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::Bedrock::Agent', 'test_agent_id', + undefined, AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_NAMES, undefined); @@ -960,9 +990,10 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::Bedrock::Agent', 'test_agent_^^id', + undefined, AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_AGENT_ID, undefined); @@ -975,7 +1006,7 @@ describe('AwsMetricAttributeGeneratorTest', () => { 'test_kb_id|test_datasource_id', AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_DATA_SOURCE_ID, undefined); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_KNOWLEDGE_BASE_ID, undefined); @@ -985,12 +1016,11 @@ describe('AwsMetricAttributeGeneratorTest', () => { mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_KNOWLEDGE_BASE_ID, 'test_kb_^id'); validateRemoteResourceAttributes( 'AWS::Bedrock::DataSource', - 'test_datasource_^^id', 'test_kb_^^id|test_datasource_^^id', AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_DATA_SOURCE_ID, undefined); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_KNOWLEDGE_BASE_ID, undefined); @@ -1003,7 +1033,6 @@ describe('AwsMetricAttributeGeneratorTest', () => { ); validateRemoteResourceAttributes( 'AWS::Bedrock::Guardrail', - 'test_guardrail_id', 'arn:aws:bedrock:us-east-1:123456789012:guardrail/test_guardrail_id', 'us-east-1', @@ -1021,7 +1050,6 @@ describe('AwsMetricAttributeGeneratorTest', () => { ); validateRemoteResourceAttributes( 'AWS::Bedrock::Guardrail', - 'test_guardrail_^^id', 'arn:aws:bedrock:us-east-1:123456789012:guardrail/test_guardrail_^^id', 'us-east-1', @@ -1036,9 +1064,10 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::Bedrock::KnowledgeBase', 'test_knowledgeBase_id', + undefined, AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_KNOWLEDGE_BASE_ID, undefined); @@ -1047,9 +1076,10 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::Bedrock::KnowledgeBase', 'test_knowledgeBase_^^id', + undefined, AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_BEDROCK_KNOWLEDGE_BASE_ID, undefined); @@ -1058,9 +1088,10 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::Bedrock::Model', 'test.service_id', + undefined, AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AwsSpanProcessingUtil.GEN_AI_REQUEST_MODEL, undefined); @@ -1069,9 +1100,10 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::Bedrock::Model', 'test.service_^^id', + undefined, AWS_REMOTE_RESOURCE_REGION, undefined, - AWS_REMOTE_RESOURCE_ACCESS_KEY + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY ); mockAttribute(AwsSpanProcessingUtil.GEN_AI_REQUEST_MODEL, undefined); @@ -1082,7 +1114,7 @@ describe('AwsMetricAttributeGeneratorTest', () => { mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, undefined); // Invalid arn and no account access key - mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCESS_KEY, undefined); + mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCOUNT_ACCESS_KEY, undefined); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_REGION, undefined); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, 'invalid_arn'); validateRemoteResourceAttributes(undefined, undefined); @@ -1101,6 +1133,7 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::StepFunctions::StateMachine', 'testStateMachine', + 'arn:aws:states:us-east-1:123456789123:stateMachine:testStateMachine', 'us-east-1', '123456789123', undefined @@ -1112,7 +1145,7 @@ describe('AwsMetricAttributeGeneratorTest', () => { AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, 'arn:aws:states:us-east-1:invalid_account_id:stateMachine:testStateMachine' ); - validateRemoteResourceAttributes('AWS::StepFunctions::StateMachine', 'testStateMachine'); + validateRemoteResourceAttributes('AWS::StepFunctions::StateMachine', 'testStateMachine', 'arn:aws:states:us-east-1:invalid_account_id:stateMachine:testStateMachine'); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, undefined); // Arn with invalid region @@ -1123,6 +1156,7 @@ describe('AwsMetricAttributeGeneratorTest', () => { validateRemoteResourceAttributes( 'AWS::StepFunctions::StateMachine', 'testStateMachine', + 'arn:aws:states:invalid_region:123456789123:stateMachine:testStateMachine', 'invalid_region', '123456789123', undefined @@ -1398,16 +1432,16 @@ describe('AwsMetricAttributeGeneratorTest', () => { if (accountId !== undefined) { expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID]).toEqual(accountId); - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCESS_KEY]).toEqual(undefined); + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY]).toEqual(undefined); } else { expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID]).toEqual(undefined); } if (accessKey !== undefined) { - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCESS_KEY]).toEqual(accessKey); + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY]).toEqual(accessKey); expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID]).toEqual(undefined); } else { - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCESS_KEY]).toEqual(undefined); + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY]).toEqual(undefined); } } @@ -1419,7 +1453,7 @@ describe('AwsMetricAttributeGeneratorTest', () => { expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_CLOUDFORMATION_PRIMARY_IDENTIFIER]).toEqual(undefined); expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_REGION]).toEqual(undefined); expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID]).toEqual(undefined); - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCESS_KEY]).toEqual(undefined); + expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY]).toEqual(undefined); } it('testDBUserAttribute', () => { diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/test/patches/instrumentation-patch.test.ts b/aws-distro-opentelemetry-node-autoinstrumentation/test/patches/instrumentation-patch.test.ts index f4a0b689..12fa5ee9 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/test/patches/instrumentation-patch.test.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/test/patches/instrumentation-patch.test.ts @@ -684,9 +684,9 @@ describe('InstrumentationPatchTest', () => { sinon.stub(trace, 'getSpan').returns(mockSpan as unknown as Span); const credentialsMiddlewareArgs: any = {}; await mockedMiddlewareStackInternal[1][0]((arg: any) => Promise.resolve(arg), null)(credentialsMiddlewareArgs); - expect(mockedMiddlewareStackInternal[1][1].name).toEqual('_extractSignerCredentials'); + expect(mockedMiddlewareStackInternal[1][1].name).toEqual('_adotExtractSignerCredentials'); expect( - mockSpan.setAttribute.calledWith(AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCESS_KEY, 'test-access-key') + mockSpan.setAttribute.calledWith(AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCOUNT_ACCESS_KEY, 'test-access-key') ).toBeTruthy(); expect(mockSpan.setAttribute.calledWith(AWS_ATTRIBUTE_KEYS.AWS_AUTH_REGION, 'us-west-2')).toBeTruthy(); }); diff --git a/contract-tests/tests/test/amazon/aws-sdk/aws_sdk_test.py b/contract-tests/tests/test/amazon/aws-sdk/aws_sdk_test.py index 24175fd1..73fd8a12 100644 --- a/contract-tests/tests/test/amazon/aws-sdk/aws_sdk_test.py +++ b/contract-tests/tests/test/amazon/aws-sdk/aws_sdk_test.py @@ -20,7 +20,7 @@ AWS_REMOTE_SERVICE, AWS_SPAN_KIND, AWS_CLOUDFORMATION_PRIMARY_IDENTIFIER, - AWS_REMOTE_RESOURCE_ACCESS_KEY, + AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY, AWS_REMOTE_RESOURCE_ACCOUNT_ID, AWS_REMOTE_RESOURCE_REGION ) @@ -1101,7 +1101,7 @@ def test_cross_account(self): request_specific_attributes={ SpanAttributes.AWS_S3_BUCKET: "cross-account-bucket", }, - remote_resource_access_key="account_b_access_key_id", + remote_resource_account_access_key="account_b_access_key_id", remote_resource_region="eu-central-1", span_name="S3.CreateBucket", ) @@ -1167,10 +1167,10 @@ def _assert_aws_attributes( if remote_resource_account_id != "None": assert remote_resource_identifier != "None" self._assert_str_attribute(attributes_dict, AWS_REMOTE_RESOURCE_ACCOUNT_ID, remote_resource_account_id) - self.assertIsNone(attributes_dict.get(AWS_REMOTE_RESOURCE_ACCESS_KEY)) + self.assertIsNone(attributes_dict.get(AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY)) if remote_resource_account_access_key != "None": assert remote_resource_identifier != "None" - self._assert_str_attribute(attributes_dict, AWS_REMOTE_RESOURCE_ACCESS_KEY, remote_resource_account_access_key) + self._assert_str_attribute(attributes_dict, AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY, remote_resource_account_access_key) self.assertIsNone(attributes_dict.get(AWS_REMOTE_RESOURCE_ACCOUNT_ID)) if remote_resource_region != "None": self._assert_str_attribute(attributes_dict, AWS_REMOTE_RESOURCE_REGION, remote_resource_region) @@ -1284,10 +1284,10 @@ def _assert_metric_attributes( if remote_resource_account_id != "None": assert remote_resource_identifier != "None" self._assert_str_attribute(attribute_dict, AWS_REMOTE_RESOURCE_ACCOUNT_ID, remote_resource_account_id) - self.assertIsNone(attribute_dict.get(AWS_REMOTE_RESOURCE_ACCESS_KEY)) + self.assertIsNone(attribute_dict.get(AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY)) if remote_resource_account_access_key != "None": assert remote_resource_identifier != "None" - self._assert_str_attribute(attribute_dict, AWS_REMOTE_RESOURCE_ACCESS_KEY, remote_resource_account_access_key) + self._assert_str_attribute(attribute_dict, AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY, remote_resource_account_access_key) self.assertIsNone(attribute_dict.get(AWS_REMOTE_RESOURCE_ACCOUNT_ID)) if remote_resource_region != "None": self._assert_str_attribute(attribute_dict, AWS_REMOTE_RESOURCE_REGION, remote_resource_region) @@ -1322,10 +1322,10 @@ def _assert_metric_attributes( if remote_resource_account_id != "None": assert remote_resource_identifier != "None" self._assert_str_attribute(attribute_dict, AWS_REMOTE_RESOURCE_ACCOUNT_ID, remote_resource_account_id) - self.assertIsNone(attribute_dict.get(AWS_REMOTE_RESOURCE_ACCESS_KEY)) + self.assertIsNone(attribute_dict.get(AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY)) if remote_resource_account_access_key != "None": assert remote_resource_identifier != "None" - self._assert_str_attribute(attribute_dict, AWS_REMOTE_RESOURCE_ACCESS_KEY, remote_resource_account_access_key) + self._assert_str_attribute(attribute_dict, AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY, remote_resource_account_access_key) self.assertIsNone(attribute_dict.get(AWS_REMOTE_RESOURCE_ACCOUNT_ID)) if remote_resource_region != "None": self._assert_str_attribute(attribute_dict, AWS_REMOTE_RESOURCE_REGION, remote_resource_region) diff --git a/contract-tests/tests/test/amazon/utils/application_signals_constants.py b/contract-tests/tests/test/amazon/utils/application_signals_constants.py index 59a5920d..dc4b9f0e 100644 --- a/contract-tests/tests/test/amazon/utils/application_signals_constants.py +++ b/contract-tests/tests/test/amazon/utils/application_signals_constants.py @@ -19,6 +19,6 @@ AWS_REMOTE_RESOURCE_IDENTIFIER: str = "aws.remote.resource.identifier" AWS_CLOUDFORMATION_PRIMARY_IDENTIFIER: str = 'aws.remote.resource.cfn.primary.identifier' AWS_SPAN_KIND: str = "aws.span.kind" -AWS_REMOTE_RESOURCE_ACCESS_KEY: str = "aws.remote.resource.account.access_key" +AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY: str = "aws.remote.resource.account.access_key" AWS_REMOTE_RESOURCE_ACCOUNT_ID: str = "aws.remote.resource.account.id" AWS_REMOTE_RESOURCE_REGION: str = "aws.remote.resource.region" \ No newline at end of file From 2b61ba81a00254a29c6f0a0542112eeb8e4c85e1 Mon Sep 17 00:00:00 2001 From: Blair Huang Date: Tue, 24 Jun 2025 20:52:22 -0700 Subject: [PATCH 05/10] lint --- .../test/aws-metric-attribute-generator.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts b/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts index 3e347ffa..043b8bbd 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts @@ -796,7 +796,7 @@ describe('AwsMetricAttributeGeneratorTest', () => { 'https://sqs.us-east-2.amazonaws.com/123456789012/MyQueue', 'us-east-2', '123456789012', - undefined, + undefined ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_SQS_QUEUE_URL, undefined); @@ -1145,7 +1145,11 @@ describe('AwsMetricAttributeGeneratorTest', () => { AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, 'arn:aws:states:us-east-1:invalid_account_id:stateMachine:testStateMachine' ); - validateRemoteResourceAttributes('AWS::StepFunctions::StateMachine', 'testStateMachine', 'arn:aws:states:us-east-1:invalid_account_id:stateMachine:testStateMachine'); + validateRemoteResourceAttributes( + 'AWS::StepFunctions::StateMachine', + 'testStateMachine', + 'arn:aws:states:us-east-1:invalid_account_id:stateMachine:testStateMachine' + ); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, undefined); // Arn with invalid region From 4d29de0091885315209c9eda5cdca652c20a2bf3 Mon Sep 17 00:00:00 2001 From: Blair Huang Date: Tue, 24 Jun 2025 21:44:45 -0700 Subject: [PATCH 06/10] update sql parser to split only once --- .../src/sqs-url-parser.ts | 69 ++++++++++--------- 1 file changed, 38 insertions(+), 31 deletions(-) diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts index 041d8fd8..082225bc 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts @@ -11,6 +11,12 @@ const HTTPS_SCHEMA: string = 'https://'; // eslint-disable-next-line @typescript-eslint/typedef const ALPHABET_REGEX = /^[a-zA-Z]+$/; +export interface ParsedSqsUrl { + queueName: string; + accountId: string; + region?: string; +} + export class SqsUrlParser { /** * Best-effort logic to extract queue name from an HTTP url. This method should only be used with @@ -23,7 +29,7 @@ export class SqsUrlParser { if (typeof url !== 'string') { return undefined; } - const urlWithoutProtocol = this.removeProtocol(url); + const urlWithoutProtocol = url.replace(HTTP_SCHEMA, '').replace(HTTPS_SCHEMA, ''); const splitUrl: string[] = urlWithoutProtocol.split('/'); if (splitUrl.length === 3 && isAccountId(splitUrl[1]) && this.isValidQueueName(splitUrl[2])) { return splitUrl[2]; @@ -39,13 +45,8 @@ export class SqsUrlParser { return undefined; } - if (this.isValidSqsUrl(url)) { - const urlWithoutProtocol = this.removeProtocol(url); - const splitUrl: string[] = urlWithoutProtocol.split('/'); - return splitUrl[1]; - } - - return undefined; + const parsedUrl = this.parseUrl(url); + return parsedUrl?.accountId; } /** @@ -56,37 +57,43 @@ export class SqsUrlParser { return undefined; } - if (this.isValidSqsUrl(url)) { - const urlWithoutProtocol = this.removeProtocol(url); - const splitUrl: string[] = urlWithoutProtocol.split('/'); - const domain: string = splitUrl[0]; - const domainParts: string[] = domain.split('.'); - if (domainParts.length === 4) { - return domainParts[1]; - } + const parsedUrl = this.parseUrl(url); + return parsedUrl?.region; + } + + /** + * Parses an SQS URL and extracts its components. + * Format: https://sqs..amazonaws.com// + * @param url - The SQS URL to parse + * @returns Object containing queue name, account ID and region, or undefined if invalid + * @private + */ + private static parseUrl(url: string): ParsedSqsUrl | undefined { + const urlWithoutProtocol = url.replace(HTTP_SCHEMA, '').replace(HTTPS_SCHEMA, ''); + const splitUrl = urlWithoutProtocol.split('/'); + + if ( + splitUrl.length !== 3 || + !splitUrl[0].toLowerCase().startsWith('sqs') || + !isAccountId(splitUrl[1]) || + !this.isValidQueueName(splitUrl[2]) + ) { + return undefined; } - return undefined; - } + const domain = splitUrl[0]; + const domainParts = domain.split('.'); - private static removeProtocol(url: string): string { - return url.replace(HTTP_SCHEMA, '').replace(HTTPS_SCHEMA, ''); + return { + queueName: splitUrl[2], + accountId: splitUrl[1], + region: domainParts.length === 4 ? domainParts[1] : undefined, + }; } /** * Checks if the URL is a valid SQS URL. */ - private static isValidSqsUrl(url: string): boolean { - const urlWithoutProtocol = this.removeProtocol(url); - const splitUrl: string[] = urlWithoutProtocol.split('/'); - return ( - splitUrl.length === 3 && - splitUrl[0].toLowerCase().startsWith('sqs') && - isAccountId(splitUrl[1]) && - this.isValidQueueName(splitUrl[2]) - ); - } - private static isValidQueueName(input: string): boolean { if (input == null || input.length === 0 || input.length > 80) { return false; From d28721dccb311423cb820239498640b090d5dd40 Mon Sep 17 00:00:00 2001 From: Blair Huang Date: Wed, 25 Jun 2025 18:09:40 -0700 Subject: [PATCH 07/10] remove length check for account id & move resource extraction into arn parser --- .../src/aws-metric-attribute-generator.ts | 34 ++++--------- .../src/regional-resource-arn-parser.ts | 18 +++++++ .../src/sqs-url-parser.ts | 4 +- .../src/utils.ts | 2 +- .../aws-metric-attribute-generator.test.ts | 6 +-- .../test/regional-resource-arn-parser.test.ts | 48 ++++++++++++++++++- .../test/sqs-url-parser.test.ts | 6 +-- 7 files changed, 82 insertions(+), 36 deletions(-) diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-metric-attribute-generator.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-metric-attribute-generator.ts index 8d7159b9..6cf9f710 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-metric-attribute-generator.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-metric-attribute-generator.ts @@ -399,7 +399,9 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN)) { remoteResourceType = NORMALIZED_DYNAMO_DB_SERVICE_NAME + '::Table'; remoteResourceIdentifier = AwsMetricAttributeGenerator.escapeDelimiters( - this.extractDynamoDbTableNameFromArn(span.attributes[AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN]) + RegionalResourceArnParser.extractDynamoDbTableNameFromArn( + span.attributes[AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN] + ) ); } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_NAME)) { remoteResourceType = NORMALIZED_KINESIS_SERVICE_NAME + '::Stream'; @@ -409,7 +411,9 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN)) { remoteResourceType = NORMALIZED_KINESIS_SERVICE_NAME + '::Stream'; remoteResourceIdentifier = AwsMetricAttributeGenerator.escapeDelimiters( - this.extractKinesisStreamNameFromArn(span.attributes[AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN]) + RegionalResourceArnParser.extractKinesisStreamNameFromArn( + span.attributes[AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN] + ) ); } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_S3_BUCKET)) { remoteResourceType = NORMALIZED_S3_SERVICE_NAME + '::Bucket'; @@ -421,7 +425,7 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { remoteResourceType = NORMALIZED_SNS_SERVICE_NAME + '::Topic'; remoteResourceIdentifier = AwsMetricAttributeGenerator.escapeDelimiters( - this.extractResourceNameFromArn(snsArn) + RegionalResourceArnParser.extractResourceNameFromArn(snsArn) ); cloudFormationIdentifier = AwsMetricAttributeGenerator.escapeDelimiters(snsArn); } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_SECRETSMANAGER_SECRET_ARN)) { @@ -429,7 +433,7 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { remoteResourceType = NORMALIZED_SECRETSMANAGER_SERVICE_NAME + '::Secret'; remoteResourceIdentifier = AwsMetricAttributeGenerator.escapeDelimiters( - this.extractResourceNameFromArn(secretsArn) + RegionalResourceArnParser.extractResourceNameFromArn(secretsArn) ); cloudFormationIdentifier = AwsMetricAttributeGenerator.escapeDelimiters(secretsArn); } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN)) { @@ -437,7 +441,7 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { remoteResourceType = NORMALIZED_STEPFUNCTIONS_SERVICE_NAME + '::StateMachine'; remoteResourceIdentifier = AwsMetricAttributeGenerator.escapeDelimiters( - this.extractResourceNameFromArn(stateMachineArn) + RegionalResourceArnParser.extractResourceNameFromArn(stateMachineArn) ); cloudFormationIdentifier = AwsMetricAttributeGenerator.escapeDelimiters(stateMachineArn); } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_ACTIVITY_ARN)) { @@ -445,7 +449,7 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { remoteResourceType = NORMALIZED_STEPFUNCTIONS_SERVICE_NAME + '::Activity'; remoteResourceIdentifier = AwsMetricAttributeGenerator.escapeDelimiters( - this.extractResourceNameFromArn(activityArn) + RegionalResourceArnParser.extractResourceNameFromArn(activityArn) ); cloudFormationIdentifier = AwsMetricAttributeGenerator.escapeDelimiters(activityArn); } else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_LAMBDA_FUNCTION_NAME)) { @@ -725,24 +729,6 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator { return rpcService === 'Lambda' && span.attributes[SEMATTRS_RPC_METHOD] === LAMBDA_INVOKE_OPERATION; } - private static extractDynamoDbTableNameFromArn(attribute: AttributeValue | undefined): string | undefined { - return this.extractResourceNameFromArn(attribute)?.replace('table/', ''); - } - - private static extractKinesisStreamNameFromArn(attribute: AttributeValue | undefined): string | undefined { - return this.extractResourceNameFromArn(attribute)?.replace('stream/', ''); - } - - // Extracts the name of the resource from an arn - private static extractResourceNameFromArn(attribute: AttributeValue | undefined): string | undefined { - if (typeof attribute === 'string' && attribute.startsWith('arn:aws:')) { - const split = attribute.split(':'); - return split[split.length - 1]; - } - - return undefined; - } - /** Span kind is needed for differentiating metrics in the EMF exporter */ private static setSpanKindForService(span: ReadableSpan, attributes: Attributes): void { let spanKind: string = SpanKind[span.kind]; diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts index 932ab911..3fa094ce 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts @@ -19,6 +19,24 @@ export class RegionalResourceArnParser { return undefined; } + public static extractDynamoDbTableNameFromArn(arn: AttributeValue | undefined): string | undefined { + return this.extractResourceNameFromArn(arn)?.replace('table/', ''); + } + + public static extractKinesisStreamNameFromArn(arn: AttributeValue | undefined): string | undefined { + return this.extractResourceNameFromArn(arn)?.replace('stream/', ''); + } + + // Extracts the name of the resource from an arn + public static extractResourceNameFromArn(arn: AttributeValue | undefined): string | undefined { + if (typeof arn === 'string' && this.isArn(arn)) { + const split = arn.split(':'); + return split[split.length - 1]; + } + + return undefined; + } + public static isArn(arn: string): boolean { // Check if arn follows the format: // arn:partition:service:region:account-id:resource-type/resource-id or diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts index 082225bc..7ba4ad09 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts @@ -11,7 +11,7 @@ const HTTPS_SCHEMA: string = 'https://'; // eslint-disable-next-line @typescript-eslint/typedef const ALPHABET_REGEX = /^[a-zA-Z]+$/; -export interface ParsedSqsUrl { +export interface ParsedSqsUrl { queueName: string; accountId: string; region?: string; @@ -22,7 +22,7 @@ export class SqsUrlParser { * Best-effort logic to extract queue name from an HTTP url. This method should only be used with * a string that is, with reasonably high confidence, an SQS queue URL. Handles new/legacy/some * custom URLs. Essentially, we require that the URL should have exactly three parts, delimited by - * /'s (excluding schema), the second part should be a 12-digit account id, and the third part + * /'s (excluding schema), the second part should be a account id consisting of digits, and the third part * should be a valid queue name, per SQS naming conventions. */ public static getQueueName(url: AttributeValue | undefined): string | undefined { diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/utils.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/utils.ts index bd10b17f..c252f58e 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/utils.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/utils.ts @@ -61,7 +61,7 @@ export const checkDigits = (str: string): boolean => { }; export const isAccountId = (input: string): boolean => { - if (input == null || input.length !== 12) { + if (input == null) { return false; } diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts b/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts index 043b8bbd..3a436f68 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts @@ -1145,11 +1145,7 @@ describe('AwsMetricAttributeGeneratorTest', () => { AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, 'arn:aws:states:us-east-1:invalid_account_id:stateMachine:testStateMachine' ); - validateRemoteResourceAttributes( - 'AWS::StepFunctions::StateMachine', - 'testStateMachine', - 'arn:aws:states:us-east-1:invalid_account_id:stateMachine:testStateMachine' - ); + validateRemoteResourceAttributes(undefined, undefined); mockAttribute(AWS_ATTRIBUTE_KEYS.AWS_STEPFUNCTIONS_STATEMACHINE_ARN, undefined); // Arn with invalid region diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/test/regional-resource-arn-parser.test.ts b/aws-distro-opentelemetry-node-autoinstrumentation/test/regional-resource-arn-parser.test.ts index fc0bd6b5..607ff53f 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/test/regional-resource-arn-parser.test.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/test/regional-resource-arn-parser.test.ts @@ -13,8 +13,8 @@ describe('RegionalResourceArnParserTest', () => { validateAccountId('::::::', undefined); validateAccountId('not:an:arn:string', undefined); validateAccountId('arn:aws:ec2:us-west-2:123456', undefined); - validateAccountId('arn:aws:ec2:us-west-2:1234567xxxxx', undefined); validateAccountId('arn:aws:ec2:us-west-2:123456789012', undefined); + validateAccountId('arn:aws:ec2:us-west-2:1234567xxxxx:table/test_table', undefined); validateAccountId('arn:aws:dynamodb:us-west-2:123456789012:table/test_table', '123456789012'); validateAccountId('arn:aws:acm:us-east-1:123456789012:certificate:abc-123', '123456789012'); }); @@ -34,6 +34,52 @@ describe('RegionalResourceArnParserTest', () => { }); }); +it('testExtractDynamoDbTableNameFromArn', () => { + validateDynamoDbTableName(undefined, undefined); + validateDynamoDbTableName('', undefined); + validateDynamoDbTableName(' ', undefined); + validateDynamoDbTableName(':', undefined); + validateDynamoDbTableName('::::::', undefined); + validateDynamoDbTableName('not:an:arn:string', undefined); + validateDynamoDbTableName('arn:aws:dynamodb:us-west-2:123456789012:table/test_table', 'test_table'); + validateDynamoDbTableName('arn:aws:dynamodb:us-west-2:123456789012:table/my-table-name', 'my-table-name'); +}); + +it('testExtractKinesisStreamNameFromArn', () => { + validateKinesisStreamName(undefined, undefined); + validateKinesisStreamName('', undefined); + validateKinesisStreamName(' ', undefined); + validateKinesisStreamName(':', undefined); + validateKinesisStreamName('::::::', undefined); + validateKinesisStreamName('not:an:arn:string', undefined); + validateKinesisStreamName('arn:aws:kinesis:us-west-2:123456789012:stream/test_stream', 'test_stream'); + validateKinesisStreamName('arn:aws:kinesis:us-west-2:123456789012:stream/my-stream-name', 'my-stream-name'); +}); + +it('testExtractResourceNameFromArn', () => { + validateResourceName(undefined, undefined); + validateResourceName('', undefined); + validateResourceName(' ', undefined); + validateResourceName(':', undefined); + validateResourceName('::::::', undefined); + validateResourceName('not:an:arn:string', undefined); + validateResourceName('arn:aws:dynamodb:us-west-2:123456789012:table/test_table', 'table/test_table'); + validateResourceName('arn:aws:kinesis:us-west-2:123456789012:stream/test_stream', 'stream/test_stream'); + validateResourceName('arn:aws:s3:us-west-2:123456789012:my-bucket', 'my-bucket'); +}); + +function validateDynamoDbTableName(arn: string | undefined, expectedName: string | undefined): void { + expect(RegionalResourceArnParser.extractDynamoDbTableNameFromArn(arn)).toEqual(expectedName); +} + +function validateKinesisStreamName(arn: string | undefined, expectedName: string | undefined): void { + expect(RegionalResourceArnParser.extractKinesisStreamNameFromArn(arn)).toEqual(expectedName); +} + +function validateResourceName(arn: string | undefined, expectedName: string | undefined): void { + expect(RegionalResourceArnParser.extractResourceNameFromArn(arn)).toEqual(expectedName); +} + function validateAccountId(arn: string | undefined, expectedAccountId: string | undefined): void { expect(RegionalResourceArnParser.getAccountId(arn)).toEqual(expectedAccountId); } diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/test/sqs-url-parser.test.ts b/aws-distro-opentelemetry-node-autoinstrumentation/test/sqs-url-parser.test.ts index 1777bd46..1d2a8020 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/test/sqs-url-parser.test.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/test/sqs-url-parser.test.ts @@ -47,7 +47,7 @@ describe('SqsUrlParserTest', () => { validateGetQueueName('invalidUrl', undefined); validateGetQueueName('https://www.amazon.com', undefined); validateGetQueueName('https://sqs.us-east-1.amazonaws.com/123412341234/.', undefined); - validateGetQueueName('https://sqs.us-east-1.amazonaws.com/12/Queue', undefined); + validateGetQueueName('https://sqs.us-east-1.amazonaws.com/12341234xxxx/.', undefined); validateGetQueueName('https://sqs.us-east-1.amazonaws.com/A/A', undefined); validateGetQueueName('https://sqs.us-east-1.amazonaws.com/123412341234/A/ThisShouldNotBeHere', undefined); }); @@ -63,7 +63,7 @@ describe('SqsUrlParserTest', () => { validateGetAccountId('/123412341234/as&df', undefined); validateGetAccountId('invalidUrl', undefined); validateGetAccountId('https://www.amazon.com', undefined); - validateGetAccountId('https://sqs.us-east-1.amazonaws.com/12341234/Queue', undefined); + validateGetAccountId('https://sqs.us-east-1.amazonaws.com/12341234/Queue', '12341234'); validateGetAccountId('https://sqs.us-east-1.amazonaws.com/1234123412xx/Queue', undefined); validateGetAccountId('https://sqs.us-east-1.amazonaws.com/1234123412xx', undefined); validateGetAccountId('https://sqs.us-east-1.amazonaws.com/123412341234/Q_Namez-5', '123412341234'); @@ -80,7 +80,7 @@ describe('SqsUrlParserTest', () => { validateGetRegion('/123412341234/as&df', undefined); validateGetRegion('invalidUrl', undefined); validateGetRegion('https://www.amazon.com', undefined); - validateGetRegion('https://sqs.us-east-1.amazonaws.com/12341234/Queue', undefined); + validateGetRegion('https://sqs.us-east-1.amazonaws.com/12341234/Queue', 'us-east-1'); validateGetRegion('https://sqs.us-east-1.amazonaws.com/1234123412xx/Queue', undefined); validateGetRegion('https://sqs.us-east-1.amazonaws.com/1234123412xx', undefined); validateGetRegion('https://sqs.us-east-1.amazonaws.com/123412341234/Q_Namez-5', 'us-east-1'); From 0ffb21fe54c094dacc0f4ec491a56cd4109f549e Mon Sep 17 00:00:00 2001 From: Blair Huang Date: Wed, 2 Jul 2025 20:45:59 -0700 Subject: [PATCH 08/10] add parseArn helper function; address minor comments --- .../src/patches/instrumentation-patch.ts | 6 +--- .../src/regional-resource-arn-parser.ts | 33 ++++++------------- .../aws-metric-attribute-generator.test.ts | 16 ++++----- 3 files changed, 19 insertions(+), 36 deletions(-) diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/patches/instrumentation-patch.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/patches/instrumentation-patch.ts index 9194c1a1..478d9924 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/patches/instrumentation-patch.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/patches/instrumentation-patch.ts @@ -414,11 +414,7 @@ function patchAwsSdkInstrumentation(instrumentation: Instrumentation): void { } } } catch (err) { - diag.debug( - `Failed to get auth account access key and region: ${ - err instanceof Error ? err.message : String(err) - }` - ); + diag.debug('Failed to get auth account access key and region:', err); } } diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts index 3fa094ce..e9b37c2b 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts @@ -5,18 +5,18 @@ import { AttributeValue } from '@opentelemetry/api'; import { isAccountId } from './utils'; export class RegionalResourceArnParser { + private static parseArn(arn: AttributeValue | undefined): string[] | undefined { + if (typeof arn !== 'string') return undefined; + const parts = arn.split(':'); + return parts.length >= 6 && parts[0] === 'arn' && isAccountId(parts[4]) ? parts : undefined; + } + public static getAccountId(arn: AttributeValue | undefined): string | undefined { - if (typeof arn == 'string' && this.isArn(arn)) { - return arn.split(':')[4]; - } - return undefined; + return this.parseArn(arn)?.[4]; } public static getRegion(arn: AttributeValue | undefined): string | undefined { - if (typeof arn == 'string' && this.isArn(arn)) { - return arn.split(':')[3]; - } - return undefined; + return this.parseArn(arn)?.[3]; } public static extractDynamoDbTableNameFromArn(arn: AttributeValue | undefined): string | undefined { @@ -27,21 +27,8 @@ export class RegionalResourceArnParser { return this.extractResourceNameFromArn(arn)?.replace('stream/', ''); } - // Extracts the name of the resource from an arn public static extractResourceNameFromArn(arn: AttributeValue | undefined): string | undefined { - if (typeof arn === 'string' && this.isArn(arn)) { - const split = arn.split(':'); - return split[split.length - 1]; - } - - return undefined; - } - - public static isArn(arn: string): boolean { - // Check if arn follows the format: - // arn:partition:service:region:account-id:resource-type/resource-id or - // arn:partition:service:region:account-id:resource-type:resource-id - const arnParts = arn.split(':'); - return arnParts.length >= 6 && arnParts[0] === 'arn' && isAccountId(arnParts[4]); + const parts = this.parseArn(arn); + return parts?.[parts.length - 1]; } } diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts b/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts index 3a436f68..fc7af59b 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-metric-attribute-generator.test.ts @@ -1427,21 +1427,21 @@ describe('AwsMetricAttributeGeneratorTest', () => { if (region !== undefined) { expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_REGION]).toEqual(region); } else { - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_REGION]).toEqual(undefined); + expect(actualAttributes).not.toHaveProperty(AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_REGION); } if (accountId !== undefined) { expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID]).toEqual(accountId); - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY]).toEqual(undefined); + expect(actualAttributes).not.toHaveProperty(AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY); } else { - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID]).toEqual(undefined); + expect(actualAttributes).not.toHaveProperty(AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID); } if (accessKey !== undefined) { expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY]).toEqual(accessKey); - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID]).toEqual(undefined); + expect(actualAttributes).not.toHaveProperty(AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID); } else { - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY]).toEqual(undefined); + expect(actualAttributes).not.toHaveProperty(AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY); } } @@ -1451,9 +1451,9 @@ describe('AwsMetricAttributeGeneratorTest', () => { expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_TYPE]).toEqual(undefined); expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_IDENTIFIER]).toEqual(undefined); expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_CLOUDFORMATION_PRIMARY_IDENTIFIER]).toEqual(undefined); - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_REGION]).toEqual(undefined); - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID]).toEqual(undefined); - expect(actualAttributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY]).toEqual(undefined); + expect(actualAttributes).not.toHaveProperty(AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_REGION); + expect(actualAttributes).not.toHaveProperty(AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ID); + expect(actualAttributes).not.toHaveProperty(AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY); } it('testDBUserAttribute', () => { From df21b7420922dfaa1c32797d75dc704909ac174f Mon Sep 17 00:00:00 2001 From: Blair Huang Date: Thu, 3 Jul 2025 13:43:31 -0700 Subject: [PATCH 09/10] add docstring & refactor isAccountId --- .../src/regional-resource-arn-parser.ts | 4 ++++ .../src/sqs-url-parser.ts | 3 +++ .../src/utils.ts | 10 +--------- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts index e9b37c2b..d7342db4 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts @@ -5,6 +5,10 @@ import { AttributeValue } from '@opentelemetry/api'; import { isAccountId } from './utils'; export class RegionalResourceArnParser { + /** Parses ARN with formats: + * arn:partition:service:region:account-id:resource-type/resource-id or + * arn:partition:service:region:account-id:resource-type:resource-id + */ private static parseArn(arn: AttributeValue | undefined): string[] | undefined { if (typeof arn !== 'string') return undefined; const parts = arn.split(':'); diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts index 7ba4ad09..14ccc86d 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts @@ -24,6 +24,9 @@ export class SqsUrlParser { * custom URLs. Essentially, we require that the URL should have exactly three parts, delimited by * /'s (excluding schema), the second part should be a account id consisting of digits, and the third part * should be a valid queue name, per SQS naming conventions. + * + * Unlike parseUrl which only handles new URLs and their queuename parsing, this + * implements its own queue name parsing logic to support multiple URL formats. */ public static getQueueName(url: AttributeValue | undefined): string | undefined { if (typeof url !== 'string') { diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/utils.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/utils.ts index c252f58e..68505245 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/utils.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/utils.ts @@ -61,13 +61,5 @@ export const checkDigits = (str: string): boolean => { }; export const isAccountId = (input: string): boolean => { - if (input == null) { - return false; - } - - if (!checkDigits(input)) { - return false; - } - - return true; + return input != null && checkDigits(input); }; From a73648a5b3a31a753b7eae7f828a7eb7e89068c8 Mon Sep 17 00:00:00 2001 From: Blair Huang Date: Thu, 3 Jul 2025 14:59:01 -0700 Subject: [PATCH 10/10] lint & update unit test --- .../src/sqs-url-parser.ts | 2 +- .../test/patches/instrumentation-patch.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts index 14ccc86d..96c57300 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts @@ -24,7 +24,7 @@ export class SqsUrlParser { * custom URLs. Essentially, we require that the URL should have exactly three parts, delimited by * /'s (excluding schema), the second part should be a account id consisting of digits, and the third part * should be a valid queue name, per SQS naming conventions. - * + * * Unlike parseUrl which only handles new URLs and their queuename parsing, this * implements its own queue name parsing logic to support multiple URL formats. */ diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/test/patches/instrumentation-patch.test.ts b/aws-distro-opentelemetry-node-autoinstrumentation/test/patches/instrumentation-patch.test.ts index 12fa5ee9..6ba36cd6 100644 --- a/aws-distro-opentelemetry-node-autoinstrumentation/test/patches/instrumentation-patch.test.ts +++ b/aws-distro-opentelemetry-node-autoinstrumentation/test/patches/instrumentation-patch.test.ts @@ -190,7 +190,7 @@ describe('InstrumentationPatchTest', () => { const services: Map = extractServicesFromAwsSdkInstrumentation(unpatchedAwsSdkInstrumentation); expect(() => doExtractKinesisAttributes(services)).not.toThrow(); const kinesisAttributes: Attributes = doExtractKinesisAttributes(services); - expect(kinesisAttributes[AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN]).toBeUndefined(); + expect(kinesisAttributes).not.toHaveProperty(AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN); }); it('SNS with patching', () => {