Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

dynamo client#107

Draft
hamiltop wants to merge 3 commits into
master from
dynamo_client
Draft

dynamo client #107
hamiltop wants to merge 3 commits into
master from
dynamo_client

Conversation

@hamiltop

@hamiltop hamiltop commented May 16, 2018

Copy link
Copy Markdown
Contributor

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 commented May 17, 2018

Copy link
Copy Markdown
Contributor

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 {

@benguillet benguillet May 18, 2018

Copy link
Copy Markdown

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?

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

@benguillet benguillet May 18, 2018

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What about this?

Comment thread dynamodb_client/base_table_client.go Outdated
}

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

Copy link
Copy Markdown
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)
})

Copy link
Copy Markdown
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

Copy link
Copy Markdown

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

Reviewers

2 more reviewers
@ejholmes ejholmes ejholmes left review comments
@benguillet benguillet benguillet approved these changes
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /