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: implment awk-sdk v2 dynamodb instrumentation #2128

Merged
merged 18 commits into from
Jul 1, 2021
Merged

Conversation

astorm
Copy link
Contributor

@astorm astorm commented Jun 28, 2021

Implements #1953

This PR add instrumentations (and tests for same) for the DynamoDB APIs provided by the v2 of the AWS SDK.

(sorting out some jenkins issues, but 99% sure the problems are not related to the code in the PR)

Checklist

  • Implement code
  • Add tests
  • Update documentation
  • Add CHANGELOG.asciidoc entry
  • Commit message follows commit guidelines

@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Jun 28, 2021
@apmmachine
Copy link
Contributor

apmmachine commented Jun 28, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: astorm commented: jenkins run the tests please

  • Start Time: 2021-07-01T16:32:36.007+0000

  • Duration: 18 min 1 sec

  • Commit: c8b3ecc

Test stats 🧪

Test Results
Failed 0
Passed 19040
Skipped 0
Total 19040

Trends 🧪

Image of Build Times

Image of Tests

@astorm
Copy link
Contributor Author

astorm commented Jun 29, 2021

Jenkins run the tests please

@astorm astorm requested a review from trentm June 29, 2021 23:13
@astorm
Copy link
Contributor Author

astorm commented Jun 29, 2021

jenkins run the tests please

Copy link
Member

@trentm trentm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A first pass with non-substantive feedback. I'll start playing with it tomorrow for real to give feedback on the actual DynamoDB-bits.

.tav.yml Show resolved Hide resolved
.tav.yml Outdated Show resolved Hide resolved
test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js Outdated Show resolved Hide resolved
test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js Outdated Show resolved Hide resolved
.tav.yml Outdated Show resolved Hide resolved
lib/instrumentation/modules/aws-sdk/dynamodb.js Outdated Show resolved Hide resolved
lib/instrumentation/modules/aws-sdk/dynamodb.js Outdated Show resolved Hide resolved
lib/instrumentation/modules/aws-sdk/dynamodb.js Outdated Show resolved Hide resolved
lib/instrumentation/modules/aws-sdk/dynamodb.js Outdated Show resolved Hide resolved
@trentm
Copy link
Member

trentm commented Jun 30, 2021

For the record here is an example generated span for a "client.query()" call:

    {
        "span": {
            "name": "DynamoDB Query trentm-play-music1",
            "type": "db",
            "id": "f856792e537b92f1",
            "transaction_id": "90875b81b15592de",
            "parent_id": "f6db83a9d68c20d0",
            "trace_id": "1d119f65efe37bd607b366739402abd8",
            "subtype": "dynamodb",
            "action": "query",
            "timestamp": 1625089047718089,
            "duration": 43.402,
            "context": {
                "db": {
                    "instance": "us-east-2",
                    "statement": "Genre = :genre",
                    "type": "dynamodb"
                },
                "destination": {
                    "address": "localhost:4566",
                    "port": 4566,
                    "service": {
                        "name": "dynamodb",
                        "type": "db",
                        "resource": "dynamodb"
                    },
                    "cloud": {
                        "region": "us-east-2"
                    }
                }
            },
            "sync": false,
            "outcome": "success",
            "sample_rate": 1
        }
    }

@trentm
Copy link
Member

trentm commented Jun 30, 2021

Here is an example captured error event for a failed conditional PutItem:

    {
        "error": {
            "id": "90894a94779679a8f616be4d9375ab2e",
            "timestamp": 1625092950223000,
            "parent_id": "7445b213ec2b12d8",
            "trace_id": "b8eb787611b104ac14826fb0b6677384",
            "transaction_id": "c955fc327db61502",
            "transaction": {
                "type": null,
                "sampled": true
            },
            "context": {
                "user": {},
                "tags": {},
                "custom": {}
            },
            "exception": {
                "message": "The conditional request failed",
                "type": "ConditionalCheckFailedException",
                "handled": true,
                "code": "ConditionalCheckFailedException",
                "attributes": {
                    "message": "The conditional request failed",
                    "time": "2021-06-30T22:42:30.223Z",
                    "requestId": "b70f4a22-c2b9-4264-a310-20138ac67c30",
                    "statusCode": 400,
                    "retryable": false,
                    "retryDelay": 20.310348847722526
                },
                "module": "aws-sdk",
                "stacktrace": [
                    {
                        "filename": "node_modules/aws-sdk/lib/protocol/json.js",
                        "lineno": 52,
                        "function": "extractError",
                        "library_frame": true,
                        "abs_path": "/Users/trentm/el/apm-agent-nodejs12/node_modules/aws-sdk/lib/protocol/json.js",
                        "pre_context": [
                            "  }",
                            ""
                        ],
                        "context_line": "  resp.error = util.error(new Error(), error);",
                        "post_context": [
                            "}",
                            ""
                        ]
                    },
                    {
                        "filename": "node_modules/aws-sdk/lib/sequential_executor.js",
                        "lineno": 106,
                        "function": "callListeners",
                        "library_frame": true,
                        "abs_path": "/Users/trentm/el/apm-agent-nodejs12/node_modules/aws-sdk/lib/sequential_executor.js",
                        "pre_context": [
                            "      } else { // synchronous listener",
                            "        try {"
                        ],
                        "context_line": "          listener.apply(self, args);",
                        "post_context": [
                            "        } catch (err) {",
                            "          error = AWS.util.error(error || new Error(), err);"
                        ]
                    },
                    {
                        "filename": "node_modules/aws-sdk/lib/sequential_executor.js",
                        "lineno": 78,
                        "function": "emit",
                        "library_frame": true,
                        "abs_path": "/Users/trentm/el/apm-agent-nodejs12/node_modules/aws-sdk/lib/sequential_executor.js",
                        "pre_context": [
                            "    var listeners = this.listeners(eventName);",
                            "    var count = listeners.length;"
                        ],
                        "context_line": "    this.callListeners(listeners, eventArgs, doneCallback);",
                        "post_context": [
                            "    return count > 0;",
                            "  },"
                        ]
                    },
                    {
                        "filename": "node_modules/aws-sdk/lib/request.js",
                        "lineno": 688,
                        "function": "emit",
                        "library_frame": true,
                        "abs_path": "/Users/trentm/el/apm-agent-nodejs12/node_modules/aws-sdk/lib/request.js",
                        "pre_context": [
                            "",
                            "    var origEmit = AWS.SequentialExecutor.prototype.emit;"
                        ],
                        "context_line": "    origEmit.call(this, eventName, args, function (err) {",
                        "post_context": [
                            "      if (err) this.response.error = err;",
                            "      done.call(this, err);"
                        ]
                    }
                ]
            },
            "culprit": "extractError (node_modules/aws-sdk/lib/protocol/json.js)"
        }
    }

That's from this PutItem:

        var params = {
          TableName: TABLE_NAME,
          Item: putItemItemFromObject(
            {
              Artist: 'No One You Know',
              SongTitle: 'Somewhere Down The Road',
              AlbumTitle: 'Somewhat Famous',
              Genre: 'Country',
              Year: 1984
            }
          ),
          ConditionExpression: 'attribute_not_exists(SongTitle)'
        }
        ddbClient.putItem(params, function (err, data) {
          log.info({params, err, data}, 'putItem conditional')
          next(err ? null : new Error('expected conditional PutItem to fail, but it did not'))
        })

@trentm
Copy link
Member

trentm commented Jun 30, 2021

Here is a play-dynamodb.js script (and sample output against localstack): https://gist.github.com/trentm/5f9caf59fff246bc99088cf458e29ae9

@trentm
Copy link
Member

trentm commented Jun 30, 2021

Here is a play-dynamodb.js script

That ran fine against real-AWS as well, and generated spans that I expect. 👍

Copy link
Member

@trentm trentm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming tests pass.

@astorm
Copy link
Contributor Author

astorm commented Jul 1, 2021

jenkins run the tests please

@astorm astorm merged commit 1cf3037 into master Jul 1, 2021
@astorm astorm deleted the astorm/dynamo-db branch July 1, 2021 18:16
dgieselaar pushed a commit to dgieselaar/apm-agent-nodejs that referenced this pull request Sep 10, 2021
* feat: Implements dynamodb instrumentation for aws sdk v2

Implements elastic#1953
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants