Skip to content

Commit 23f3b2c

Browse files
blairhyy-amazonblairhyy-amazon
committed
improve data checking; update queue url & arn parser
1 parent 61ff701 commit 23f3b2c

File tree

10 files changed

+150
-126
lines changed

10 files changed

+150
-126
lines changed

‎aws-distro-opentelemetry-node-autoinstrumentation/src/aws-attribute-keys.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,15 @@ import { SEMATTRS_AWS_DYNAMODB_TABLE_NAMES } from '@opentelemetry/semantic-conve
55

66
// Utility class holding attribute keys with special meaning to AWS components
77
export const AWS_ATTRIBUTE_KEYS = {
8-
AWS_AUTH_ACCESS_KEY: 'aws.auth.account.access_key',
8+
AWS_AUTH_ACCOUNT_ACCESS_KEY: 'aws.auth.account.access_key',
99
AWS_AUTH_REGION: 'aws.auth.region',
1010
AWS_SPAN_KIND: 'aws.span.kind',
1111
AWS_LOCAL_SERVICE: 'aws.local.service',
1212
AWS_LOCAL_OPERATION: 'aws.local.operation',
1313
AWS_REMOTE_SERVICE: 'aws.remote.service',
1414
AWS_REMOTE_ENVIRONMENT: 'aws.remote.environment',
1515
AWS_REMOTE_OPERATION: 'aws.remote.operation',
16-
AWS_REMOTE_RESOURCE_ACCESS_KEY: 'aws.remote.resource.account.access_key',
16+
AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY: 'aws.remote.resource.account.access_key',
1717
AWS_REMOTE_RESOURCE_ACCOUNT_ID: 'aws.remote.resource.account.id',
1818
AWS_REMOTE_RESOURCE_REGION: 'aws.remote.resource.region',
1919
AWS_REMOTE_RESOURCE_TYPE: 'aws.remote.resource.type',

‎aws-distro-opentelemetry-node-autoinstrumentation/src/aws-metric-attribute-generator.ts‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -399,10 +399,7 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator {
399399
} else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN)) {
400400
remoteResourceType = NORMALIZED_DYNAMO_DB_SERVICE_NAME + '::Table';
401401
remoteResourceIdentifier = AwsMetricAttributeGenerator.escapeDelimiters(
402-
this.extractResourceNameFromArn(span.attributes[AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN])?.replace(
403-
'table/',
404-
''
405-
)
402+
this.extractDynamoDbTableNameFromArn(span.attributes[AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN])
406403
);
407404
} else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_NAME)) {
408405
remoteResourceType = NORMALIZED_KINESIS_SERVICE_NAME + '::Stream';
@@ -412,10 +409,7 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator {
412409
} else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN)) {
413410
remoteResourceType = NORMALIZED_KINESIS_SERVICE_NAME + '::Stream';
414411
remoteResourceIdentifier = AwsMetricAttributeGenerator.escapeDelimiters(
415-
this.extractResourceNameFromArn(span.attributes[AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN])?.replace(
416-
'stream/',
417-
''
418-
)
412+
this.extractKinesisStreamNameFromArn(span.attributes[AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_ARN])
419413
);
420414
} else if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_S3_BUCKET)) {
421415
remoteResourceType = NORMALIZED_S3_SERVICE_NAME + '::Bucket';
@@ -595,9 +589,9 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator {
595589
}
596590

597591
private static setRemoteResourceAccessKeyAndRegion(span: ReadableSpan, attributes: Attributes): void {
598-
if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCESS_KEY)) {
599-
attributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCESS_KEY] =
600-
span.attributes[AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCESS_KEY];
592+
if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCOUNT_ACCESS_KEY)) {
593+
attributes[AWS_ATTRIBUTE_KEYS.AWS_REMOTE_RESOURCE_ACCOUNT_ACCESS_KEY] =
594+
span.attributes[AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCOUNT_ACCESS_KEY];
601595
}
602596
if (AwsSpanProcessingUtil.isKeyPresent(span, AWS_ATTRIBUTE_KEYS.AWS_AUTH_REGION)) {
603597
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 {
731725
return rpcService === 'Lambda' && span.attributes[SEMATTRS_RPC_METHOD] === LAMBDA_INVOKE_OPERATION;
732726
}
733727

728+
private static extractDynamoDbTableNameFromArn(attribute: AttributeValue | undefined): string | undefined {
729+
return this.extractResourceNameFromArn(attribute)?.replace('table/', '');
730+
}
731+
732+
private static extractKinesisStreamNameFromArn(attribute: AttributeValue | undefined): string | undefined {
733+
return this.extractResourceNameFromArn(attribute)?.replace('stream/', '');
734+
}
735+
734736
// Extracts the name of the resource from an arn
735737
private static extractResourceNameFromArn(attribute: AttributeValue | undefined): string | undefined {
736738
if (typeof attribute === 'string' && attribute.startsWith('arn:aws:')) {

‎aws-distro-opentelemetry-node-autoinstrumentation/src/patches/instrumentation-patch.ts‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -233,22 +233,23 @@ function patchKinesisServiceExtension(kinesisServiceExtension: any): void {
233233
function patchDynamoDbServiceExtension(dynamoDbServiceExtension: any): void {
234234
if (dynamoDbServiceExtension) {
235235
if (typeof dynamoDbServiceExtension.responseHook === 'function') {
236-
const originalResponseHook = dynamoDbServiceExtension.responseHook;
237-
dynamoDbServiceExtension.responseHook = (
236+
const responseHook = dynamoDbServiceExtension.responseHook;
237+
238+
const patchedResponseHook = (
238239
response: NormalizedResponse,
239240
span: Span,
240241
tracer: Tracer,
241242
config: AwsSdkInstrumentationConfig
242243
): void => {
243-
originalResponseHook.call(dynamoDbServiceExtension, response, span, tracer, config);
244+
responseHook.call(dynamoDbServiceExtension, response, span, tracer, config);
244245

245-
if (response.data && response.data.Table) {
246-
const tableArn = response.data.Table.TableArn;
247-
if (tableArn) {
248-
span.setAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN, tableArn);
249-
}
246+
const tableArn = response?.data?.Table?.TableArn;
247+
if (tableArn) {
248+
span.setAttribute(AWS_ATTRIBUTE_KEYS.AWS_DYNAMODB_TABLE_ARN, tableArn);
250249
}
251250
};
251+
252+
dynamoDbServiceExtension.responseHook = patchedResponseHook;
252253
}
253254
}
254255
}
@@ -400,28 +401,32 @@ function patchAwsSdkInstrumentation(instrumentation: Instrumentation): void {
400401
if (span) {
401402
try {
402403
const credsProvider = this.config.credentials;
403-
if (credsProvider) {
404+
if (credsProvider instanceof Function) {
404405
const credentials = await credsProvider();
405-
if (credentials.accessKeyId) {
406-
span.setAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCESS_KEY, credentials.accessKeyId);
406+
if (credentials?.accessKeyId) {
407+
span.setAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_ACCOUNT_ACCESS_KEY, credentials.accessKeyId);
407408
}
408409
}
409-
if (this.config.region) {
410+
if (this.config.region instanceof Function) {
410411
const region = await this.config.region();
411412
if (region) {
412413
span.setAttribute(AWS_ATTRIBUTE_KEYS.AWS_AUTH_REGION, region);
413414
}
414415
}
415416
} catch (err) {
416-
diag.debug('Fail to get auth account access key and region.');
417+
diag.debug(
418+
`Failed to get auth account access key and region: ${
419+
err instanceof Error ? err.message : String(err)
420+
}`
421+
);
417422
}
418423
}
419424

420425
return await next(middlewareArgs);
421426
},
422427
{
423428
step: 'build',
424-
name: '_extractSignerCredentials',
429+
name: '_adotExtractSignerCredentials',
425430
override: true,
426431
}
427432
);

‎aws-distro-opentelemetry-node-autoinstrumentation/src/regional-resource-arn-parser.ts‎

Lines changed: 7 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,51 +2,28 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
import { AttributeValue } from '@opentelemetry/api';
5+
import { isAccountId } from './utils';
56

67
export class RegionalResourceArnParser {
78
public static getAccountId(arn: AttributeValue | undefined): string | undefined {
8-
if (this.isArn(arn)) {
9-
return (arn! as string).split(':')[4];
9+
if (typeof arn == 'string' && this.isArn(arn)) {
10+
return arn.split(':')[4];
1011
}
1112
return undefined;
1213
}
1314

1415
public static getRegion(arn: AttributeValue | undefined): string | undefined {
15-
if (this.isArn(arn)) {
16-
return (arn! as string).split(':')[3];
16+
if (typeof arn == 'string' && this.isArn(arn)) {
17+
return arn.split(':')[3];
1718
}
1819
return undefined;
1920
}
2021

21-
public static isArn(arn: AttributeValue | undefined): boolean {
22+
public static isArn(arn: string): boolean {
2223
// Check if arn follows the format:
2324
// arn:partition:service:region:account-id:resource-type/resource-id or
2425
// arn:partition:service:region:account-id:resource-type:resource-id
25-
if (!arn || typeof arn !== 'string') {
26-
return false;
27-
}
28-
29-
if (!arn.startsWith('arn')) {
30-
return false;
31-
}
32-
3326
const arnParts = arn.split(':');
34-
return arnParts.length >= 6 && this.isAccountId(arnParts[4]);
35-
}
36-
37-
private static isAccountId(input: string): boolean {
38-
if (input == null || input.length !== 12) {
39-
return false;
40-
}
41-
42-
if (!this._checkDigits(input)) {
43-
return false;
44-
}
45-
46-
return true;
47-
}
48-
49-
private static _checkDigits(str: string): boolean {
50-
return /^\d+$/.test(str);
27+
return arnParts.length >= 6 && arnParts[0] === 'arn' && isAccountId(arnParts[4]);
5128
}
5229
}

‎aws-distro-opentelemetry-node-autoinstrumentation/src/sqs-url-parser.ts‎

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
import { AttributeValue } from '@opentelemetry/api';
5+
import { isAccountId } from './utils';
56

67
const HTTP_SCHEMA: string = 'http://';
78
const HTTPS_SCHEMA: string = 'https://';
@@ -22,9 +23,9 @@ export class SqsUrlParser {
2223
if (typeof url !== 'string') {
2324
return undefined;
2425
}
25-
url = url.replace(HTTP_SCHEMA, '').replace(HTTPS_SCHEMA, '');
26-
const splitUrl: string[] = url.split('/');
27-
if (splitUrl.length === 3 && this.isAccountId(splitUrl[1]) && this.isValidQueueName(splitUrl[2])) {
26+
const urlWithoutProtocol = this.removeProtocol(url);
27+
const splitUrl: string[] = urlWithoutProtocol.split('/');
28+
if (splitUrl.length === 3 && isAccountId(splitUrl[1]) && this.isValidQueueName(splitUrl[2])) {
2829
return splitUrl[2];
2930
}
3031
return undefined;
@@ -38,9 +39,9 @@ export class SqsUrlParser {
3839
return undefined;
3940
}
4041

41-
url = url.replace(HTTP_SCHEMA, '').replace(HTTPS_SCHEMA, '');
4242
if (this.isValidSqsUrl(url)) {
43-
const splitUrl: string[] = url.split('/');
43+
const urlWithoutProtocol = this.removeProtocol(url);
44+
const splitUrl: string[] = urlWithoutProtocol.split('/');
4445
return splitUrl[1];
4546
}
4647

@@ -55,9 +56,9 @@ export class SqsUrlParser {
5556
return undefined;
5657
}
5758

58-
url = url.replace(HTTP_SCHEMA, '').replace(HTTPS_SCHEMA, '');
5959
if (this.isValidSqsUrl(url)) {
60-
const splitUrl: string[] = url.split('/');
60+
const urlWithoutProtocol = this.removeProtocol(url);
61+
const splitUrl: string[] = urlWithoutProtocol.split('/');
6162
const domain: string = splitUrl[0];
6263
const domainParts: string[] = domain.split('.');
6364
if (domainParts.length === 4) {
@@ -68,35 +69,24 @@ export class SqsUrlParser {
6869
return undefined;
6970
}
7071

72+
private static removeProtocol(url: string): string {
73+
return url.replace(HTTP_SCHEMA, '').replace(HTTPS_SCHEMA, '');
74+
}
75+
7176
/**
7277
* Checks if the URL is a valid SQS URL.
7378
*/
7479
private static isValidSqsUrl(url: string): boolean {
75-
const splitUrl: string[] = url.split('/');
80+
const urlWithoutProtocol = this.removeProtocol(url);
81+
const splitUrl: string[] = urlWithoutProtocol.split('/');
7682
return (
7783
splitUrl.length === 3 &&
7884
splitUrl[0].toLowerCase().startsWith('sqs') &&
79-
this.isAccountId(splitUrl[1]) &&
85+
isAccountId(splitUrl[1]) &&
8086
this.isValidQueueName(splitUrl[2])
8187
);
8288
}
8389

84-
private static isAccountId(input: string): boolean {
85-
if (input == null || input.length !== 12) {
86-
return false;
87-
}
88-
89-
if (!this._checkDigits(input)) {
90-
return false;
91-
}
92-
93-
return true;
94-
}
95-
96-
private static _checkDigits(str: string): boolean {
97-
return /^\d+$/.test(str);
98-
}
99-
10090
private static isValidQueueName(input: string): boolean {
10191
if (input == null || input.length === 0 || input.length > 80) {
10292
return false;

‎aws-distro-opentelemetry-node-autoinstrumentation/src/utils.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,19 @@ export const getAwsRegionFromEnvironment = (): string | undefined => {
5555

5656
return undefined;
5757
};
58+
59+
export const checkDigits = (str: string): boolean => {
60+
return /^\d+$/.test(str);
61+
};
62+
63+
export const isAccountId = (input: string): boolean => {
64+
if (input == null || input.length !== 12) {
65+
return false;
66+
}
67+
68+
if (!checkDigits(input)) {
69+
return false;
70+
}
71+
72+
return true;
73+
};

0 commit comments

Comments
 (0)