Skip to content

Commit ce592e5

Browse files
jj22eejj22ee
committed
address comments
1 parent f3b9422 commit ce592e5

File tree

6 files changed

+115
-66
lines changed

6 files changed

+115
-66
lines changed

‎plugins/node/opentelemetry-instrumentation-aws-sdk/src/enums.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,10 @@ export enum AttributeNames {
2222
AWS_REQUEST_ID = 'aws.request.id',
2323
AWS_REQUEST_EXTENDED_ID = 'aws.request.extended_id',
2424
AWS_SIGNATURE_VERSION = 'aws.signature.version',
25+
26+
// TODO: Add these semantic attributes to:
27+
// - https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-semantic-conventions/src/trace/SemanticAttributes.ts
28+
// For S3, see specification: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/object-stores/s3.md
29+
AWS_S3_BUCKET = 'aws.s3.bucket',
30+
AWS_KINESIS_STREAM_NAME = 'aws.kinesis.stream.name',
2531
}

‎plugins/node/opentelemetry-instrumentation-aws-sdk/src/services/kinesis.ts‎

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -13,29 +13,22 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
import { Attributes, Span, SpanKind, Tracer } from '@opentelemetry/api';
17-
import {
18-
AwsSdkInstrumentationConfig,
19-
NormalizedRequest,
20-
NormalizedResponse,
21-
} from '../types';
22-
import { _AWS_KINESIS_STREAM_NAME } from '../utils';
16+
import { Attributes, SpanKind } from '@opentelemetry/api';
17+
import { AttributeNames } from '../enums';
18+
import { AwsSdkInstrumentationConfig, NormalizedRequest } from '../types';
2319
import { RequestMetadata, ServiceExtension } from './ServiceExtension';
2420

2521
export class KinesisServiceExtension implements ServiceExtension {
2622
requestPreSpanHook(
2723
request: NormalizedRequest,
2824
_config: AwsSdkInstrumentationConfig
2925
): RequestMetadata {
30-
const streamName = request.commandInput.StreamName;
31-
26+
const streamName = request.commandInput?.StreamName;
3227
const spanKind: SpanKind = SpanKind.CLIENT;
33-
let spanName: string | undefined;
34-
3528
const spanAttributes: Attributes = {};
3629

3730
if (streamName) {
38-
spanAttributes[_AWS_KINESIS_STREAM_NAME] = streamName;
31+
spanAttributes[AttributeNames.AWS_KINESIS_STREAM_NAME] = streamName;
3932
}
4033

4134
const isIncoming = false;
@@ -44,16 +37,6 @@ export class KinesisServiceExtension implements ServiceExtension {
4437
isIncoming,
4538
spanAttributes,
4639
spanKind,
47-
spanName,
4840
};
4941
}
50-
51-
requestPostSpanHook = (request: NormalizedRequest) => {};
52-
53-
responseHook = (
54-
response: NormalizedResponse,
55-
span: Span,
56-
tracer: Tracer,
57-
config: AwsSdkInstrumentationConfig
58-
) => {};
5942
}

‎plugins/node/opentelemetry-instrumentation-aws-sdk/src/services/s3.ts‎

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -13,29 +13,22 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
import { Attributes, Span, SpanKind, Tracer } from '@opentelemetry/api';
17-
import {
18-
AwsSdkInstrumentationConfig,
19-
NormalizedRequest,
20-
NormalizedResponse,
21-
} from '../types';
22-
import { _AWS_S3_BUCKET } from '../utils';
16+
import { Attributes, SpanKind } from '@opentelemetry/api';
17+
import { AttributeNames } from '../enums';
18+
import { AwsSdkInstrumentationConfig, NormalizedRequest } from '../types';
2319
import { RequestMetadata, ServiceExtension } from './ServiceExtension';
2420

2521
export class S3ServiceExtension implements ServiceExtension {
2622
requestPreSpanHook(
2723
request: NormalizedRequest,
2824
_config: AwsSdkInstrumentationConfig
2925
): RequestMetadata {
30-
const bucketName = request.commandInput.Bucket;
31-
26+
const bucketName = request.commandInput?.Bucket;
3227
const spanKind: SpanKind = SpanKind.CLIENT;
33-
let spanName: string | undefined;
34-
3528
const spanAttributes: Attributes = {};
3629

3730
if (bucketName) {
38-
spanAttributes[_AWS_S3_BUCKET] = bucketName;
31+
spanAttributes[AttributeNames.AWS_S3_BUCKET] = bucketName;
3932
}
4033

4134
const isIncoming = false;
@@ -44,16 +37,6 @@ export class S3ServiceExtension implements ServiceExtension {
4437
isIncoming,
4538
spanAttributes,
4639
spanKind,
47-
spanName,
4840
};
4941
}
50-
51-
requestPostSpanHook = (request: NormalizedRequest) => {};
52-
53-
responseHook = (
54-
response: NormalizedResponse,
55-
span: Span,
56-
tracer: Tracer,
57-
config: AwsSdkInstrumentationConfig
58-
) => {};
5942
}

‎plugins/node/opentelemetry-instrumentation-aws-sdk/src/utils.ts‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,14 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
import { NormalizedRequest } from './types';
1716
import { Attributes, Context, context } from '@opentelemetry/api';
1817
import {
1918
SEMATTRS_RPC_METHOD,
2019
SEMATTRS_RPC_SERVICE,
2120
SEMATTRS_RPC_SYSTEM,
2221
} from '@opentelemetry/semantic-conventions';
2322
import { AttributeNames } from './enums';
24-
25-
// TODO: Add these semantic attributes to:
26-
// - https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-semantic-conventions/src/trace/SemanticAttributes.ts
27-
// For S3, see specification: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/object-stores/s3.md
28-
export const _AWS_S3_BUCKET = 'aws.s3.bucket';
29-
export const _AWS_KINESIS_STREAM_NAME = 'aws.kinesis.stream.name';
23+
import { NormalizedRequest } from './types';
3024

3125
const toPascalCase = (str: string): string =>
3226
typeof str === 'string' ? str.charAt(0).toUpperCase() + str.slice(1) : str;

‎plugins/node/opentelemetry-instrumentation-aws-sdk/test/kinesis.test.ts‎

Lines changed: 52 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,21 @@ import {
1919
registerInstrumentationTesting,
2020
} from '@opentelemetry/contrib-test-utils';
2121
import { AwsInstrumentation } from '../src';
22+
import { AttributeNames } from '../src/enums';
2223
registerInstrumentationTesting(new AwsInstrumentation());
2324

25+
import { Kinesis } from '@aws-sdk/client-kinesis';
2426
import * as AWS from 'aws-sdk';
2527
import { AWSError } from 'aws-sdk';
2628
import * as nock from 'nock';
2729

2830
import { SpanKind } from '@opentelemetry/api';
2931
import { ReadableSpan } from '@opentelemetry/sdk-trace-base';
3032
import { expect } from 'expect';
31-
import { _AWS_KINESIS_STREAM_NAME } from '../src/utils';
3233

3334
const region = 'us-east-1';
3435

35-
describe('Kinesis', () => {
36+
describe('Kinesis - v2', () => {
3637
let kinesis: AWS.Kinesis;
3738
beforeEach(() => {
3839
AWS.config.credentials = {
@@ -44,7 +45,7 @@ describe('Kinesis', () => {
4445
};
4546
});
4647

47-
describe('CreateStream', () => {
48+
describe('DescribeStream', () => {
4849
it('adds Stream Name', async () => {
4950
kinesis = new AWS.Kinesis({ region: region });
5051
const dummyStreamName = 'dummy-stream-name';
@@ -54,10 +55,9 @@ describe('Kinesis', () => {
5455
.reply(200, 'null');
5556

5657
await kinesis
57-
.createStream(
58+
.describeStream(
5859
{
5960
StreamName: dummyStreamName,
60-
ShardCount: 3,
6161
},
6262
(err: AWSError) => {
6363
expect(err).toBeFalsy();
@@ -66,15 +66,55 @@ describe('Kinesis', () => {
6666
.promise();
6767

6868
const testSpans = getTestSpans();
69-
const creationSpans = testSpans.filter((s: ReadableSpan) => {
70-
return s.name === 'Kinesis.CreateStream';
69+
const describeSpans = testSpans.filter((s: ReadableSpan) => {
70+
return s.name === 'Kinesis.DescribeStream';
7171
});
72-
expect(creationSpans.length).toBe(1);
73-
const creationSpan = creationSpans[0];
74-
expect(creationSpan.attributes[_AWS_KINESIS_STREAM_NAME]).toBe(
75-
dummyStreamName
72+
expect(describeSpans.length).toBe(1);
73+
const describeSpan = describeSpans[0];
74+
expect(
75+
describeSpan.attributes[AttributeNames.AWS_KINESIS_STREAM_NAME]
76+
).toBe(dummyStreamName);
77+
expect(describeSpan.kind).toBe(SpanKind.CLIENT);
78+
});
79+
});
80+
});
81+
82+
describe('Kinesis - v3', () => {
83+
let kinesis: Kinesis;
84+
beforeEach(() => {
85+
kinesis = new Kinesis({
86+
region: region,
87+
credentials: {
88+
accessKeyId: 'abcde',
89+
secretAccessKey: 'abcde',
90+
},
91+
});
92+
});
93+
94+
describe('DescribeStream', () => {
95+
it('adds Stream Name', async () => {
96+
const dummyStreamName = 'dummy-stream-name';
97+
98+
nock(`https://kinesis.${region}.amazonaws.com/`).post('/').reply(200, {});
99+
100+
await kinesis
101+
.describeStream({
102+
StreamName: dummyStreamName,
103+
})
104+
.catch((err: any) => {});
105+
106+
const testSpans: ReadableSpan[] = getTestSpans();
107+
const describeSpans: ReadableSpan[] = testSpans.filter(
108+
(s: ReadableSpan) => {
109+
return s.name === 'Kinesis.DescribeStream';
110+
}
76111
);
77-
expect(creationSpan.kind).toBe(SpanKind.CLIENT);
112+
expect(describeSpans.length).toBe(1);
113+
const describeSpan = describeSpans[0];
114+
expect(
115+
describeSpan.attributes[AttributeNames.AWS_KINESIS_STREAM_NAME]
116+
).toBe(dummyStreamName);
117+
expect(describeSpan.kind).toBe(SpanKind.CLIENT);
78118
});
79119
});
80120
});

‎plugins/node/opentelemetry-instrumentation-aws-sdk/test/s3.test.ts‎

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,21 @@ import {
1919
registerInstrumentationTesting,
2020
} from '@opentelemetry/contrib-test-utils';
2121
import { AwsInstrumentation } from '../src';
22+
import { AttributeNames } from '../src/enums';
2223
registerInstrumentationTesting(new AwsInstrumentation());
2324

25+
import { S3 } from '@aws-sdk/client-s3';
2426
import * as AWS from 'aws-sdk';
2527
import { AWSError } from 'aws-sdk';
2628
import * as nock from 'nock';
2729

2830
import { SpanKind } from '@opentelemetry/api';
2931
import { ReadableSpan } from '@opentelemetry/sdk-trace-base';
3032
import { expect } from 'expect';
31-
import { _AWS_S3_BUCKET } from '../src/utils';
3233

3334
const region = 'us-east-1';
3435

35-
describe('S3', () => {
36+
describe('S3 - v2', () => {
3637
let s3: AWS.S3;
3738
beforeEach(() => {
3839
AWS.config.credentials = {
@@ -68,7 +69,49 @@ describe('S3', () => {
6869
});
6970
expect(listObjectsSpans.length).toBe(1);
7071
const listObjectsSpan = listObjectsSpans[0];
71-
expect(listObjectsSpan.attributes[_AWS_S3_BUCKET]).toBe(dummyBucketName);
72+
expect(listObjectsSpan.attributes[AttributeNames.AWS_S3_BUCKET]).toBe(
73+
dummyBucketName
74+
);
75+
expect(listObjectsSpan.kind).toBe(SpanKind.CLIENT);
76+
});
77+
});
78+
});
79+
80+
describe('S3 - v3', () => {
81+
let s3: S3;
82+
beforeEach(() => {
83+
s3 = new S3({
84+
region: region,
85+
credentials: {
86+
accessKeyId: 'abcde',
87+
secretAccessKey: 'abcde',
88+
},
89+
});
90+
});
91+
92+
describe('ListObjects', () => {
93+
it('adds bucket Name', async () => {
94+
const dummyBucketName = 'dummy-bucket-name';
95+
96+
nock(`https://s3.${region}.amazonaws.com/`).post('/').reply(200, 'null');
97+
98+
await s3
99+
.listObjects({
100+
Bucket: dummyBucketName,
101+
})
102+
.catch((err: any) => {});
103+
104+
const testSpans: ReadableSpan[] = getTestSpans();
105+
const listObjectsSpans: ReadableSpan[] = testSpans.filter(
106+
(s: ReadableSpan) => {
107+
return s.name === 'S3.ListObjects';
108+
}
109+
);
110+
expect(listObjectsSpans.length).toBe(1);
111+
const listObjectsSpan = listObjectsSpans[0];
112+
expect(listObjectsSpan.attributes[AttributeNames.AWS_S3_BUCKET]).toBe(
113+
dummyBucketName
114+
);
72115
expect(listObjectsSpan.kind).toBe(SpanKind.CLIENT);
73116
});
74117
});

0 commit comments

Comments
 (0)