Skip to content

Conversation

@hamiltop
Copy link
Member

This is a pretty simple dynamodb client that supports apm tracing and takes an an input a map of table descriptions (to be used in testing and in production for table creation).

@bmarini
Copy link
Contributor

bmarini commented May 17, 2018

I just noticed that the aws sdk has two useful looking sub packages that might be able to replace the attribute/query building code in this client.
https://godoc.org/github.com/aws/aws-sdk-go/service/dynamodb/dynamodbattribute
https://godoc.org/github.com/aws/aws-sdk-go/service/dynamodb/expression

return gio.Item == nil
}

func isValidConsumedCapacityLevel(level string) bool {

Choose a reason for hiding this comment

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

Do we care about this?

Endpoint: aws.String(params.LocalDynamoURL),
}
svc := dynamodb.New(session.New(), &config)
//svc.Handlers.Retry.PushFrontNamed(CheckThrottleHandler)

Choose a reason for hiding this comment

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

What about this?

}

req, _ := dt.client.PutItemRequest(input)
err := dt.sendWithTracing(ctx, req)
Copy link
Contributor

Choose a reason for hiding this comment

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

If #109 is merged, then we can just remove all of this and use PutItemWithContext. To add tracing to the client, with the table name included in the spans:

s := contrib.WithTracing(session.New())
c := dynamodb.New(s)
c.Handlers.Send.PushBack(func(r *request.Request) {
        span := opentracing.SpanFromContext(r.Context())
        span.SetTag(dd_opentracing.SpanType, "db")
        span.SetTag("aws.table_name", dt.tableName)
})

@ejholmes
Copy link
Contributor

I rebased this, and added two commits:

  1. Removes sendWithTracing, since Simpler AWS opentracing #109 was merged.
  2. Changes dynamo_client.NewDynamoDBClient to accept a client.ConfigProvider so you can provide a *session.Session wrapped with tracing.

@Kevin-Jonaitis Kevin-Jonaitis marked this pull request as draft February 25, 2021 19:11
@Kevin-Jonaitis
Copy link

Changing to "WIP". If you would like this to be reviewed, please convert back from WIP and we'll take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants