Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(instrumentation-aws-sdk): upstream aws-sdk instrumentation from ext-js #678

Merged
merged 37 commits into from
Oct 19, 2021
Merged
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
1350cb0
feat(instrumentation-aws-sdk): upstream aws-sdk instrumentation to co…
Sep 26, 2021
71dd599
fix(instrumentation-aws-sdk): test-all-versions script
Sep 26, 2021
e565ab2
Merge remote-tracking branch 'upstream/main' into aws-sdk-plugin
Sep 26, 2021
12c4c2e
chore(instrumentation-aws-sdk): lint
Sep 26, 2021
27acd0c
chore(instrumentation-aws-sdk): pin api package
Sep 26, 2021
28ca896
fix(instrumentation-aws-sdk): bump testing-utils to link in repo
Sep 26, 2021
aa786dc
fix: use otel api that match peer deps
Sep 26, 2021
30ac70a
docs(instrumentation-aws-sdk): remove provider in README
Sep 29, 2021
542e585
fix(aws-sdk): fix package name to otel org prefix
Sep 29, 2021
de0c4bf
chore(instrumentation-aws-sdk): downgrade mocha for node v8 support
Sep 29, 2021
b9255f3
Merge remote-tracking branch 'upstream/main' into aws-sdk-plugin
Oct 4, 2021
2e9b31d
fix: use hook info interface for config hooks
Oct 4, 2021
6b9deeb
build: downgrade expect package to support node8
Oct 4, 2021
09ee529
fix: don't run v3 tests on unsupported node8
Oct 5, 2021
a1198cf
Revert "fix: don't run v3 tests on unsupported node8"
Oct 5, 2021
07fbcfc
ci: exclude aws-sdk from node8 tests
Oct 6, 2021
39b82be
ci: move lerna ignore to the right place
Oct 6, 2021
c881137
feat: add propagation utils package for messaging systems
Oct 7, 2021
4159994
chore(propagation-utils): resolve all lint issues and add types
Oct 7, 2021
6d558e5
chore(instrumentaiton-aws-sdk): lint
Oct 7, 2021
71f620e
Merge remote-tracking branch 'upstream/main' into aws-sdk-plugin
Oct 10, 2021
e921e50
style(instrumentation-aws-sdk): fix lint issues
Oct 10, 2021
cdc9119
fix(propagation-utils): remove codecov from package
Oct 10, 2021
44c121a
Merge branch 'main' into aws-sdk-plugin
rauno56 Oct 13, 2021
04c6cb5
chore(aws-sdk): docs fixes from code review
Oct 18, 2021
024c44c
Merge remote-tracking branch 'upstream/main' into aws-sdk-plugin
Oct 18, 2021
3048ab5
Merge branch 'aws-sdk-plugin' of https://github.com/blumamir/opentele…
Oct 18, 2021
a29a8a0
chore: add aws-sdk to release please config
Oct 18, 2021
3fe26fd
chore(aws-sdk): upgrade sdk to v1.0.0/v0.26.0
Oct 18, 2021
555c243
fix(aws-sdk): support for ^3.36.0 which changed dist folder name
Oct 18, 2021
e18f9d3
fix(aws-sdk): use correct dist folder for v3.35.0
Oct 18, 2021
2944752
chore(aws-sdk): test less versions with tav
Oct 18, 2021
f96a2f3
chore(aws-sdk): test with latest versions of package
Oct 18, 2021
d221111
chore(aws-sdk): update support of v2 to ^2.308.0
Oct 18, 2021
d111081
fix(aws-sdk): don't crash if service name is missing
Oct 18, 2021
e8c9a50
chore: lint fix
Oct 18, 2021
a57cd4a
docs(aws-sdk): add link to GH issue for the receive operation and sem…
Oct 19, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: use hook info interface for config hooks
Amir Blum committed Oct 4, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit 2e9b31da3fe147248310fd102d28d8c69e2be1b3
11 changes: 8 additions & 3 deletions plugins/node/opentelemetry-instrumentation-aws-sdk/README.md
Original file line number Diff line number Diff line change
@@ -45,9 +45,6 @@ aws-sdk instrumentation has few options available to choose from. You can set th
| `responseHook` | `AwsSdkResponseCustomAttributeFunction` | Hook for adding custom attributes when response is received from aws. |
| `sqsProcessHook` | `AwsSdkSqsProcessCustomAttributeFunction` | Hook called after starting sqs `process` span (for each sqs received message), which allow to add custom attributes to it. |
| `suppressInternalInstrumentation` | `boolean` | Most aws operation use http requests under the hood. Set this to `true` to hide all underlying http spans. |
| `moduleVersionAttributeName` | `string` | If passed, a span attribute will be added to all spans with key of the provided `moduleVersionAttributeName` and value of the patched module version |



## Span Attributes
Both V2 and V3 instrumentations are collecting the following attributes:
@@ -99,6 +96,14 @@ This instrumentation is a work in progress. We implemented some of the specific
## Potential Side Effects
The instrumentation is doing best effort to support the trace specification of open telemetry. For SQS, it involves defining new attributes on the `Messages` array, as well as on the manipulated types generated from this array (to set correct trace context for a single SQS message operation). Those properties are defined as [non-enumerable](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Enumerability_and_ownership_of_properties) properties, so they have minimum side effect on the app. They will, however, show when using the `Object.getOwnPropertyDescriptors` and `Reflect.ownKeys` functions on SQS `Messages` array and for each `Message` in the array.

## Migration From opentelemetry-instrumentation-aws-sdk
This instrumentation was originally published under the name `"opentelemetry-instrumentation-aws-sdk"` in [this repo](https://github.com/aspecto-io/opentelemetry-ext-js). Few breaking changes were made during porting to the contrib repo to align with conventions:

### Hook Info
The instrumentation's config `preRequestHook`, `responseHook` and `sqsProcessHook` functions signature changed, so the second function parameter is info object, containing the relevant hook data.

### `moduleVersionAttributeName` config option
The `moduleVersionAttributeName` config option is removed. To add the aws-sdk package version to spans, use the `moduleVersion` attribute in hook info for `preRequestHook` and `responseHook` functions.
## Useful links

- For more information on OpenTelemetry, visit: <https://opentelemetry.io/>
40 changes: 15 additions & 25 deletions plugins/node/opentelemetry-instrumentation-aws-sdk/src/aws-sdk.ts
Original file line number Diff line number Diff line change
@@ -28,6 +28,8 @@ import { AttributeNames } from './enums';
import { ServicesExtensions } from './services';
import {
AwsSdkInstrumentationConfig,
AwsSdkRequestHookInformation,
AwsSdkResponseHookInformation,
NormalizedRequest,
NormalizedResponse,
} from './types';
@@ -197,7 +199,6 @@ export class AwsInstrumentation extends InstrumentationBase<typeof AWS> {
private _startAwsV3Span(
normalizedRequest: NormalizedRequest,
metadata: RequestMetadata,
moduleVersion: string | undefined
): Span {
const name =
metadata.spanName ??
@@ -210,21 +211,13 @@ export class AwsInstrumentation extends InstrumentationBase<typeof AWS> {
},
});

if (this._config.moduleVersionAttributeName && moduleVersion) {
newSpan.setAttribute(
this._config.moduleVersionAttributeName,
moduleVersion
);
}

return newSpan;
}

private _startAwsV2Span(
request: AWS.Request<any, any>,
metadata: RequestMetadata,
normalizedRequest: NormalizedRequest,
moduleVersion: string | undefined
): Span {
const operation = (request as any).operation;
const service = (request as any).service;
@@ -247,20 +240,17 @@ export class AwsInstrumentation extends InstrumentationBase<typeof AWS> {
},
});

if (this._config.moduleVersionAttributeName && moduleVersion) {
newSpan.setAttribute(
this._config.moduleVersionAttributeName,
moduleVersion
);
}

return newSpan;
}

private _callUserPreRequestHook(span: Span, request: NormalizedRequest) {
private _callUserPreRequestHook(span: Span, request: NormalizedRequest, moduleVersion: string | undefined) {
if (this._config?.preRequestHook) {
const requestInfo: AwsSdkRequestHookInformation = {
moduleVersion,
request,
}
safeExecuteInTheMiddle(
() => this._config.preRequestHook!(span, request),
() => this._config.preRequestHook!(span, requestInfo),
(e: Error | undefined) => {
if (e)
diag.error(
@@ -277,8 +267,11 @@ export class AwsInstrumentation extends InstrumentationBase<typeof AWS> {
const responseHook = this._config?.responseHook;
if (!responseHook) return;

const responseInfo: AwsSdkResponseHookInformation = {
response,
};
safeExecuteInTheMiddle(
() => responseHook(span, response),
() => responseHook(span, responseInfo),
(e: Error | undefined) => {
if (e)
diag.error(
@@ -433,7 +426,6 @@ export class AwsInstrumentation extends InstrumentationBase<typeof AWS> {
const span = self._startAwsV3Span(
normalizedRequest,
requestMetadata,
moduleVersion
);
const activeContextWithSpan = trace.setSpan(context.active(), span);

@@ -451,7 +443,7 @@ export class AwsInstrumentation extends InstrumentationBase<typeof AWS> {
);
}

self._callUserPreRequestHook(span, normalizedRequest);
self._callUserPreRequestHook(span, normalizedRequest, moduleVersion);
const resultPromise = context.with(activeContextWithSpan, () => {
self.servicesExtensions.requestPostSpanHook(normalizedRequest);
return self._callOriginalFunction(() =>
@@ -549,13 +541,12 @@ export class AwsInstrumentation extends InstrumentationBase<typeof AWS> {
this,
requestMetadata,
normalizedRequest,
moduleVersion
);
this[REQUEST_SPAN_KEY] = span;
const activeContextWithSpan = trace.setSpan(context.active(), span);
const callbackWithContext = context.bind(activeContextWithSpan, callback);

self._callUserPreRequestHook(span, normalizedRequest);
self._callUserPreRequestHook(span, normalizedRequest, moduleVersion);
self._registerV2CompletedEvent(
span,
this,
@@ -590,12 +581,11 @@ export class AwsInstrumentation extends InstrumentationBase<typeof AWS> {
this,
requestMetadata,
normalizedRequest,
moduleVersion
);
this[REQUEST_SPAN_KEY] = span;

const activeContextWithSpan = trace.setSpan(context.active(), span);
self._callUserPreRequestHook(span, normalizedRequest);
self._callUserPreRequestHook(span, normalizedRequest, moduleVersion);
self._registerV2CompletedEvent(
span,
this,
Original file line number Diff line number Diff line change
@@ -178,7 +178,7 @@ export class SqsServiceExtension implements ServiceExtension {
},
}),
processHook: (span: Span, message: SQS.Message) =>
config.sqsProcessHook?.(span, message),
config.sqsProcessHook?.(span, { message }),
});

pubsubPropagation.patchArrayForProcessSpans(
23 changes: 14 additions & 9 deletions plugins/node/opentelemetry-instrumentation-aws-sdk/src/types.ts
Original file line number Diff line number Diff line change
@@ -33,21 +33,32 @@ export interface NormalizedResponse {
request: NormalizedRequest;
}

export interface AwsSdkRequestHookInformation {
moduleVersion?: string;
request: NormalizedRequest;
}
export interface AwsSdkRequestCustomAttributeFunction {
(span: Span, request: NormalizedRequest): void;
(span: Span, requestInfo: AwsSdkRequestHookInformation): void;
}

export interface AwsSdkResponseHookInformation {
moduleVersion?: string;
response: NormalizedResponse;
}
/**
* span can be used to add custom attributes, or for any other need.
* response is the object that is returned to the user calling the aws-sdk operation.
* The response type and attributes on the response are client-specific.
*/
export interface AwsSdkResponseCustomAttributeFunction {
(span: Span, response: NormalizedResponse): void;
(span: Span, responseInfo: AwsSdkResponseHookInformation): void;
}

export interface AwsSdkSqsProcessHookInformation {
message: AWS.SQS.Message;
}
export interface AwsSdkSqsProcessCustomAttributeFunction {
(span: Span, message: AWS.SQS.Message): void;
(span: Span, sqsProcessInfo: AwsSdkSqsProcessHookInformation): void;
}

export interface AwsSdkInstrumentationConfig extends InstrumentationConfig {
@@ -69,10 +80,4 @@ export interface AwsSdkInstrumentationConfig extends InstrumentationConfig {
* effectively causing those http spans to be non-recordable.
*/
suppressInternalInstrumentation?: boolean;

/**
* If passed, a span attribute will be added to all spans with key of the provided "moduleVersionAttributeName"
* and value of the module version.
*/
moduleVersionAttributeName?: string;
}
Original file line number Diff line number Diff line change
@@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { AwsInstrumentation } from '../src';
import { AwsInstrumentation, AwsSdkRequestHookInformation, AwsSdkResponseHookInformation } from '../src';
import {
getTestSpans,
registerInstrumentationTesting,
@@ -263,10 +263,10 @@ describe('instrumentation-aws-sdk-v2', () => {
it('preRequestHook called and add request attribute to span', done => {
mockV2AwsSend(responseMockSuccess, 'data returned from operation');
const config = {
preRequestHook: (span: Span, request: any) => {
preRequestHook: (span: Span, requestInfo: AwsSdkRequestHookInformation) => {
span.setAttribute(
'attribute from hook',
request.commandInput['Bucket']
requestInfo.request.commandInput['Bucket'],
);
},
};
@@ -314,8 +314,8 @@ describe('instrumentation-aws-sdk-v2', () => {
it('responseHook called and add response attribute to span', done => {
mockV2AwsSend(responseMockSuccess, 'data returned from operation');
const config = {
responseHook: (span: Span, response: any) => {
span.setAttribute('attribute from response hook', response['data']);
responseHook: (span: Span, responseInfo: AwsSdkResponseHookInformation) => {
span.setAttribute('attribute from response hook', responseInfo.response['data']);
},
};

@@ -371,27 +371,5 @@ describe('instrumentation-aws-sdk-v2', () => {
const awsSpans = getAwsSpans();
expect(awsSpans.length).toBe(1);
});

it('setting moduleVersionAttributeName is adding module version', async () => {
mockV2AwsSend(responseMockSuccess, 'data returned from operation', true);
const config = {
moduleVersionAttributeName: 'module.version',
suppressInternalInstrumentation: true,
};

instrumentation.disable();
instrumentation.setConfig(config);
instrumentation.enable();

const s3 = new AWS.S3();

await s3.createBucket({ Bucket: 'aws-test-bucket' }).promise();
const awsSpans = getAwsSpans();
expect(awsSpans.length).toBe(1);

expect(awsSpans[0].attributes['module.version']).toMatch(
/2.\d{1,4}\.\d{1,5}.*/
);
});
});
});
Original file line number Diff line number Diff line change
@@ -15,8 +15,8 @@
*/
import {
AwsInstrumentation,
NormalizedRequest,
NormalizedResponse,
AwsSdkRequestHookInformation,
AwsSdkResponseHookInformation,
} from '../src';
import {
getTestSpans,
@@ -179,14 +179,14 @@ describe('instrumentation-aws-sdk-v3', () => {
it('verify request and response hooks are called with right params', async () => {
instrumentation.disable();
instrumentation.setConfig({
preRequestHook: (span: Span, request: NormalizedRequest) => {
preRequestHook: (span: Span, requestInfo: AwsSdkRequestHookInformation) => {
span.setAttribute(
'attribute.from.request.hook',
request.commandInput.Bucket
requestInfo.request.commandInput.Bucket
);
},

responseHook: (span: Span, response: NormalizedResponse) => {
responseHook: (span: Span, responseInfo: AwsSdkResponseHookInformation) => {
span.setAttribute(
'attribute.from.response.hook',
'data from response hook'
@@ -222,15 +222,15 @@ describe('instrumentation-aws-sdk-v3', () => {
it('handle throw in request and response hooks', async () => {
instrumentation.disable();
instrumentation.setConfig({
preRequestHook: (span: Span, request: NormalizedRequest) => {
preRequestHook: (span: Span, requestInfo: AwsSdkRequestHookInformation) => {
span.setAttribute(
'attribute.from.request.hook',
request.commandInput.Bucket
requestInfo.request.commandInput.Bucket
);
throw new Error('error from request hook in unittests');
},

responseHook: (span: Span, response: NormalizedResponse) => {
responseHook: (span: Span, responseInfo: AwsSdkResponseHookInformation) => {
throw new Error('error from response hook in unittests');
},

@@ -258,35 +258,6 @@ describe('instrumentation-aws-sdk-v3', () => {
});
});

describe('moduleVersionAttributeName', () => {
it('setting moduleVersionAttributeName is adding module version', async () => {
instrumentation.disable();
instrumentation.setConfig({
moduleVersionAttributeName: 'module.version',
suppressInternalInstrumentation: true,
});
instrumentation.enable();

nock(`https://ot-demo-test.s3.${region}.amazonaws.com/`)
.put('/aws-ot-s3-test-object.txt?x-id=PutObject')
.reply(
200,
fs.readFileSync('./test/mock-responses/s3-put-object.xml', 'utf8')
);

const params = {
Bucket: 'ot-demo-test',
Key: 'aws-ot-s3-test-object.txt',
};
await s3Client.putObject(params);
expect(getTestSpans().length).toBe(1);
const [span] = getTestSpans();

expect(span.attributes['module.version']).toMatch(
/3.\d{1,4}\.\d{1,5}.*/
);
});
});
});

describe('custom service behavior', () => {
Original file line number Diff line number Diff line change
@@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { AwsInstrumentation } from '../src';
import { AwsInstrumentation, AwsSdkSqsProcessHookInformation } from '../src';
import {
getTestSpans,
registerInstrumentationTesting,
@@ -349,8 +349,8 @@ describe('SQS', () => {
describe('hooks', () => {
it('sqsProcessHook called and add message attribute to span', async () => {
const config = {
sqsProcessHook: (span: Span, message: AWS.SQS.Message) => {
span.setAttribute('attribute from sqs process hook', message.Body!);
sqsProcessHook: (span: Span, sqsProcessInfo: AwsSdkSqsProcessHookInformation) => {
span.setAttribute('attribute from sqs process hook', sqsProcessInfo.message.Body!);
},
};

@@ -396,7 +396,7 @@ describe('SQS', () => {

it('sqsProcessHook throws does not fail span', async () => {
const config = {
sqsProcessHook: (span: Span, message: AWS.SQS.Message) => {
sqsProcessHook: (span: Span, sqsProcessInfo: AwsSdkSqsProcessHookInformation) => {
throw new Error('error from sqsProcessHook hook');
},
};