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

fix(instrumentation-aws-sdk): remove un-sanitised db.statement span attribute from DynamoDB spans #1748

Merged
merged 8 commits into from
Dec 7, 2023
25 changes: 13 additions & 12 deletions plugins/node/opentelemetry-instrumentation-aws-sdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ npm install --save @opentelemetry/instrumentation-aws-sdk
For further automatic instrumentation instruction see the [@opentelemetry/instrumentation](https://www.npmjs.com/package/@opentelemetry/instrumentation) package.

```js
const { NodeTracerProvider } = require("@opentelemetry/sdk-trace-node");
const { registerInstrumentations } = require("@opentelemetry/instrumentation");
const { NodeTracerProvider } = require('@opentelemetry/sdk-trace-node');
const { registerInstrumentations } = require('@opentelemetry/instrumentation');
const {
AwsInstrumentation,
} = require("@opentelemetry/instrumentation-aws-sdk");
} = require('@opentelemetry/instrumentation-aws-sdk');

const provider = new NodeTracerProvider();
provider.register();
Expand All @@ -42,13 +42,14 @@ registerInstrumentations({

aws-sdk instrumentation has few options available to choose from. You can set the following:

| Options | Type | Description |
| --------------------------------- | ----------------------------------------- | -------------------------------------------------------------------------------------------------------------------------- |
| `preRequestHook` | `AwsSdkRequestCustomAttributeFunction` | Hook called before request send, which allow to add custom attributes to span. |
| `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. |
| `sqsExtractContextPropagationFromPayload` | `boolean` | Will parse and extract context propagation headers from SQS Payload, false by default. [When should it be used?](./doc/sns.md#integration-with-sqs)|
| Options | Type | Description |
| ----------------------------------------- | ----------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `preRequestHook` | `AwsSdkRequestCustomAttributeFunction` | Hook called before request send, which allow to add custom attributes to span. |
| `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. |
| `sqsExtractContextPropagationFromPayload` | `boolean` | Will parse and extract context propagation headers from SQS Payload, false by default. [When should it be used?](./doc/sns.md#integration-with-sqs) |
| `dynamoDBStatementSerializer` | `AwsSdkDynamoDBStatementSerializer` | AWS SDK instrumentation will serialize DynamoDB commands to the `db.statement` attribute using the specified function. Defaults to using a serializer that returns `undefined`. |

## Span Attributes

Expand Down Expand Up @@ -82,8 +83,8 @@ Usage example:
```js
awsInstrumentationConfig = {
preRequestHook: (span, request) => {
if (span.serviceName === "s3") {
span.setAttribute("s3.bucket.name", request.commandInput["Bucket"]);
if (span.serviceName === 's3') {
span.setAttribute('s3.bucket.name', request.commandInput['Bucket']);
}
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -475,8 +475,11 @@ export class AwsInstrumentation extends InstrumentationBase<any> {
command.input,
undefined
);
const requestMetadata =
self.servicesExtensions.requestPreSpanHook(normalizedRequest);
const requestMetadata = self.servicesExtensions.requestPreSpanHook(
normalizedRequest,
self._config,
self._diag
);
const span = self._startAwsV3Span(normalizedRequest, requestMetadata);
const activeContextWithSpan = trace.setSpan(context.active(), span);

Expand Down Expand Up @@ -602,8 +605,11 @@ export class AwsInstrumentation extends InstrumentationBase<any> {
}

const normalizedRequest = normalizeV2Request(this);
const requestMetadata =
self.servicesExtensions.requestPreSpanHook(normalizedRequest);
const requestMetadata = self.servicesExtensions.requestPreSpanHook(
normalizedRequest,
self._config,
self._diag
);
const span = self._startAwsV2Span(
this,
requestMetadata,
Expand Down Expand Up @@ -642,8 +648,11 @@ export class AwsInstrumentation extends InstrumentationBase<any> {
}

const normalizedRequest = normalizeV2Request(this);
const requestMetadata =
self.servicesExtensions.requestPreSpanHook(normalizedRequest);
const requestMetadata = self.servicesExtensions.requestPreSpanHook(
normalizedRequest,
self._config,
self._diag
);
const span = self._startAwsV2Span(
this,
requestMetadata,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,13 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { Span, SpanAttributes, SpanKind, Tracer } from '@opentelemetry/api';
import {
DiagLogger,
Span,
SpanAttributes,
SpanKind,
Tracer,
} from '@opentelemetry/api';
import {
AwsSdkInstrumentationConfig,
NormalizedRequest,
Expand All @@ -30,7 +36,11 @@ export interface RequestMetadata {

export interface ServiceExtension {
// called before request is sent, and before span is started
requestPreSpanHook: (request: NormalizedRequest) => RequestMetadata;
requestPreSpanHook: (
request: NormalizedRequest,
config: AwsSdkInstrumentationConfig,
diag: DiagLogger
) => RequestMetadata;

// called before request is sent, and after span is started
requestPostSpanHook?: (request: NormalizedRequest) => void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { Tracer, Span } from '@opentelemetry/api';
import { Tracer, Span, DiagLogger } from '@opentelemetry/api';
import { ServiceExtension, RequestMetadata } from './ServiceExtension';
import { SqsServiceExtension } from './sqs';
import {
Expand All @@ -35,13 +35,17 @@ export class ServicesExtensions implements ServiceExtension {
this.services.set('Lambda', new LambdaServiceExtension());
}

requestPreSpanHook(request: NormalizedRequest): RequestMetadata {
requestPreSpanHook(
request: NormalizedRequest,
config: AwsSdkInstrumentationConfig,
diag: DiagLogger
): RequestMetadata {
const serviceExtension = this.services.get(request.serviceName);
if (!serviceExtension)
return {
isIncoming: false,
};
return serviceExtension.requestPreSpanHook(request);
return serviceExtension.requestPreSpanHook(request, config, diag);
}

requestPostSpanHook(request: NormalizedRequest) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { Span, SpanKind, Tracer } from '@opentelemetry/api';
import { DiagLogger, Span, SpanKind, Tracer } from '@opentelemetry/api';
import { RequestMetadata, ServiceExtension } from './ServiceExtension';
import {
DbSystemValues,
Expand All @@ -30,7 +30,11 @@ export class DynamodbServiceExtension implements ServiceExtension {
return Array.isArray(values) ? values : [values];
}

requestPreSpanHook(normalizedRequest: NormalizedRequest): RequestMetadata {
requestPreSpanHook(
normalizedRequest: NormalizedRequest,
config: AwsSdkInstrumentationConfig,
diag: DiagLogger
): RequestMetadata {
const spanKind: SpanKind = SpanKind.CLIENT;
let spanName: string | undefined;
const isIncoming = false;
Expand All @@ -40,11 +44,23 @@ export class DynamodbServiceExtension implements ServiceExtension {
[SemanticAttributes.DB_SYSTEM]: DbSystemValues.DYNAMODB,
[SemanticAttributes.DB_NAME]: normalizedRequest.commandInput?.TableName,
[SemanticAttributes.DB_OPERATION]: operation,
[SemanticAttributes.DB_STATEMENT]: JSON.stringify(
normalizedRequest.commandInput
),
};

if (config.dynamoDBStatementSerializer) {
try {
const sanitizedStatement = config.dynamoDBStatementSerializer(
operation,
normalizedRequest.commandInput
);

if (typeof sanitizedStatement === 'string') {
spanAttributes[SemanticAttributes.DB_STATEMENT] = sanitizedStatement;
}
} catch (err) {
diag.error('failed to sanitize DynamoDB statement', err);
}
}

// normalizedRequest.commandInput.RequestItems) is undefined when no table names are returned
// keys in this object are the table names
if (normalizedRequest.commandInput?.TableName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ class LambdaCommands {
}

export class LambdaServiceExtension implements ServiceExtension {
requestPreSpanHook(request: NormalizedRequest): RequestMetadata {
requestPreSpanHook(
request: NormalizedRequest,
_config: AwsSdkInstrumentationConfig
): RequestMetadata {
const functionName = this.extractFunctionName(request.commandInput);

let spanAttributes: SpanAttributes = {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ import { injectPropagationContext } from './MessageAttributes';
import { RequestMetadata, ServiceExtension } from './ServiceExtension';

export class SnsServiceExtension implements ServiceExtension {
requestPreSpanHook(request: NormalizedRequest): RequestMetadata {
requestPreSpanHook(
request: NormalizedRequest,
_config: AwsSdkInstrumentationConfig
): RequestMetadata {
let spanKind: SpanKind = SpanKind.CLIENT;
let spanName = `SNS ${request.commandName}`;
const spanAttributes = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ import {
} from './MessageAttributes';

export class SqsServiceExtension implements ServiceExtension {
requestPreSpanHook(request: NormalizedRequest): RequestMetadata {
requestPreSpanHook(
request: NormalizedRequest,
_config: AwsSdkInstrumentationConfig
): RequestMetadata {
const queueUrl = this.extractQueueUrl(request.commandInput);
const queueName = this.extractQueueNameFromUrl(queueUrl);
let spanKind: SpanKind = SpanKind.CLIENT;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import { Span } from '@opentelemetry/api';
import { InstrumentationConfig } from '@opentelemetry/instrumentation';
import { SQS } from './aws-sdk.types';

export type CommandInput = Record<string, any>;

/**
* These are normalized request and response, which are used by both sdk v2 and v3.
* They organize the relevant data in one interface which can be processed in a
Expand All @@ -25,7 +27,7 @@ import { SQS } from './aws-sdk.types';
export interface NormalizedRequest {
serviceName: string;
commandName: string;
commandInput: Record<string, any>;
commandInput: CommandInput;
region?: string;
}
export interface NormalizedResponse {
Expand Down Expand Up @@ -62,6 +64,11 @@ export interface AwsSdkSqsProcessCustomAttributeFunction {
(span: Span, sqsProcessInfo: AwsSdkSqsProcessHookInformation): void;
}

export type AwsSdkDynamoDBStatementSerializer = (
operation: string,
commandInput: CommandInput
) => string | undefined;

export interface AwsSdkInstrumentationConfig extends InstrumentationConfig {
/** hook for adding custom attributes before request is sent to aws */
preRequestHook?: AwsSdkRequestCustomAttributeFunction;
Expand All @@ -72,6 +79,9 @@ export interface AwsSdkInstrumentationConfig extends InstrumentationConfig {
/** hook for adding custom attribute when an sqs process span is started */
sqsProcessHook?: AwsSdkSqsProcessCustomAttributeFunction;

/** custom serializer function for the db.statement attribute in DynamoDB spans */
dynamoDBStatementSerializer?: AwsSdkDynamoDBStatementSerializer;

/**
* Most aws operation use http request under the hood.
* if http instrumentation is enabled, each aws operation will also create
Expand Down
Loading