Merge pull request #13 from aspecto-io/aws-readme

docs(plugin-aws-sdk): add documentation for attributes and sqs

Author: Amir Blum

packages/plugin-aws-sdk/README.md

@@ -32,4 +32,31 @@ aws-sdk plugin has few options available to choose from. You can set the followi
 
 | Options        | Type                                   | Description                                                                                     |
 | -------------- | -------------------------------------- | ----------------------------------------------------------------------------------------------- |
-| `preRequestHook` | `AwsSdkRequestCustomAttributeFunction` | Hook called before request send, which allow to add custom attributes to span. |
+| `preRequestHook` | `AwsSdkRequestCustomAttributeFunction` | Hook called before request send, which allow to add custom attributes to span. |
+
+
+## Span Attributes
+This plugin patch the internal `Request` object, which means that each sdk operation will create a single span with attributes from 3 sources:
+
+### Default attributes
+Each span will have the following attributes:
+| Attribute Name | Type | Description | Example |
+| -------------- | ---- | ----------- | ------- |
+| "component" | string | Always equals "aws-sdk" | "aws-sdk" |
+| "aws.operation" | string | The method name for the request. | for `SQS.sendMessage(...)` the operation is "sendMessage" |
+| "aws.signature.version" | string | Aws version of authentication signature on the request. | "v4" |
+| "aws.region" | string | Region name for the request | "eu-west-1" |
+| "aws.service.api" | string | The sdk class name for the service | "SQS" |
+| "aws.service.identifier" | string | Identifier for the service in the sdk | "sqs" |
+| "aws.service.name" | string | Abbreviation name for the service | "Amazon SQS" |
+| "aws.request.id" | uuid | Request unique id, as returned from aws on response | "01234567-89ab-cdef-0123-456789abcdef" |
+| "aws.error" | string | information about a service or networking error, as returned from aws | "UriParameterError: Expected uri parameter to have length >= 1, but found "" for params.Bucket" |
+
+### Custom User Attributes
+The plugin user can configure a hook function which will be called before each request, with the request object and the relevant span. This hook can be used to add custom attributes to the span with any logic. For example, user can add interesting attributes from the `request.params`, and write custom logic based on the service and operation.
+
+### Specific Service Logic
+AWS contains dozens of services accessible with the JS SDK. For many services, the default attributes specified above are enough, but other services have specific [trace semantic conventions](https://github.com/open-telemetry/opentelemetry-specification/tree/master/specification/trace/semantic_conventions), or need to inject/extract intra-process context, or set intra-process context correctly.
+
+This plugin is a work in progress. We implemented some of the specific trace semantics for some of the services, and strive to support more services and extend the already supported services in the future. You can [Open an Issue](https://github.com/aspecto-io/opentelemetry-ext-js/issues), or [Submit a Pull Request](https://github.com/aspecto-io/opentelemetry-ext-js/pulls) if you want to contribute.
+

packages/plugin-aws-sdk/docs/sqs.md

@@ -0,0 +1,39 @@
+# SQS
+SQS is amazon's managed message queue. Thus, it should follow the [Open Telemetry specification for Messaging systems](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/messaging.md).
+
+## Specific trace semantic
+Following methods needs specific attention:
+
+### sendMessage / sendMessageBatch
+- Add [message attributes](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/messaging.md#messaging-attributes) to span in addition to the default attributes. These attributes are covered by the library according to the spec.
+- Inject trace context as SQS MessageAttributes, so the service receiving the message can link cascading spans to the trace which created the message. This is not implemented yet.
+
+### receiveMessage
+- Add [message attributes](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/messaging.md#messaging-attributes) to span in addition to the default attributes. These attributes are covered by the library according to the spec.
+- Create additional "processing spans" for each message received by the application. So if an application called `receiveMessage`, and got back 10 messages, a single `messaging.operation` = `receive` span will be created for the `receiveMessage` operation, and 10 `messaging.operation` = `process` spans will be created, one for each message. Those processing spans are created by the library. This behavior is partially implemented, [See discussion below](#processing-spans).
+- Set the inter process context correctly, so that additional spans created from message receiving and message processing will be linked to parent spans correctly. This behavior is partially implemented, [See discussion below](#processing-spans).
+- Extract trace context from SQS MessageAttributes, and set span's `parent` and `links` correctly according to the spec. This is not implemented yet.
+
+#### Processing spans
+According to open telemetry specification (and to reasonable expectation for trace structure), user of this library would expect to see one span for the operation of receiving messages batch from sqs, and then, for each message, a span with it's own sub-tree for the processing of this specific message. 
+
+For example, if a `receiveMessages` returned 2 messages: msg1 is storing something to a DB, and msg2 is calling an external http endpoint, we should link the db span under msg1, and the http span under msg2, instead of mixing all those operations under the single `receive` span, or start a new trace for each of them.
+
+Unfortunately, this is not so easy to implement in JS:
+1. The SDK is calling a single callback for the messages batch, and it's not straight forward to understand when each individual message processing starts and ends (and set the context correctly for cascading spans).
+2. If async/await is used, context can be lost when returning data from async functions, for example:
+```js
+async function asyncRecv() {
+  const data = await sqs.receiveMessage(recvParams).promise();
+  // context of receiveMessage is set here
+  return data;
+}
+
+async function poll() {
+    const result = await asyncRecv();
+    // context is lost when asyncRecv returns. following spans are created with root context.
+    await Promise.all(result.Messages.map((message) => this.processMessage(message)));
+}
+```
+
+Current implementation partially solves this issue by patching the `map` \ `forEach` functions on the `Messages` array of `receiveMessage` result. This handles issues like the one above, but will not handle situations where the processing is done in other patterns (multiple map\forEach calls, index access to the array, other array operations, etc). This is currently an open issue in the plugin.