Merge branch 'master' of github.com:aspecto-io/opentelemetry-ext-js into request-hook-refactor

Author: Amir Blum

packages/plugin-aws-sdk/README.md

@@ -1,4 +1,5 @@
 # OpenTelemetry aws-sdk Instrumentation for Node.js
+![NPM version](https://img.shields.io/npm/v/opentelemetry-plugin-aws-sdk.svg)
 
 This module provides automatic instrumentation for [`aws-sdk`](https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/).
 
@@ -10,10 +11,10 @@ npm install --save opentelemetry-plugin-aws-sdk
 
 ## Usage
 
-To load a specific plugin (**aws-sdk** in this case), specify it in the Node Tracer's configuration
+To load this plugin, specify it in the Node Tracer's configuration:
 
 ```js
-const { NodeTracerProvider } = require("opentelemetry-plugin-aws-sdk");
+const { NodeTracerProvider } = require("@opentelemetry/node");
 
 const provider = new NodeTracerProvider({
   plugins: {
@@ -42,21 +43,40 @@ This plugin patch the internal `Request` object, which means that each sdk opera
 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" |
+| `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.
+The plugin user can configure a `preRequestHook` function which will be called before each request, with the request object and the corrosponding 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.
+Usage example:
+```js
+awsPluginConfig = {
+  enabled: true,
+  path: "opentelemetry-plugin-aws-sdk",
+  preRequestHook: (span, request) => {
+    if (span.attributes["aws.service.api"] === 's3') {
+      span.setAttribute("s3.bucket.name", request.params["Bucket"]);
+    }
+  }
+};
+```
 
 ### 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.
 
+Specific service logic currently implemented for:
+* [SQS](./docs/sqs.md)
+
+---
+
 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

@@ -2,22 +2,29 @@
 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:
+The following methods are automatically enhanced:
 
 ### 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.
+- [Message Attributes](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/messaging.md#messaging-attributes) are added by this plugin according to the spec.
+- TODO: Inject trace context as SQS MessageAttributes, so the service receiving the message can link cascading spans to the trace which created the message. 
 
 ### 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.
+- [Message Attributes](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/messaging.md#messaging-attributes) are added by this plugin according to the spec.
+- Additional "processing spans" are created for each message received by the application.   
+If an application invoked `receiveMessage`, and received a 10 messages batch, 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).
+- Sets the inter process context correctly, so that additional spans created through the process will be linked to parent spans correctly.  
+This behavior is partially implemented, [See discussion below](#processing-spans).
+- TODO: Extract trace context from SQS MessageAttributes, and set span's `parent` and `links` correctly according to the spec.
 
-#### 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. 
+#### 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.
+For example, if a `receiveMessages` returned 2 messages: 
+* `msg1` resulting in storing something to a DB.
+* `msg2` resulting in calling an external HTTP endpoint.  
+
+This will result in a creating a DB span that would be linked under `msg1` process, and an HTTP span that would be under `msg2` (in opposed to 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).
@@ -36,4 +43,4 @@ async function poll() {
 }
 ```
 
-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.
+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.