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

Added a custom user agent for calls made to sts and iam #9

Merged
merged 1 commit into from
Apr 5, 2019

Conversation

PettitWesley
Copy link
Contributor

Basically just copied from the following ECS CLI code:
https://github.com/aws/amazon-ecs-cli/blob/master/ecs-cli/modules/clients/aws/ecs/client.go#L77
https://github.com/aws/amazon-ecs-cli/blob/master/ecs-cli/modules/clients/user_agent.go

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@efekarakus efekarakus left a comment

Choose a reason for hiding this comment

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

Looks good to me! 🚀 Just a small nitpick.

Also is there a way of testing this change?

)

// UserAgentHeader is the header for a User Agent
const UserAgentHeader = "User-Agent"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: looks like this can be private since noone else is using it.

@PettitWesley
Copy link
Contributor Author

@efekarakus I don't think there's a way to view the request details to check the header since its made over HTTPS. I'm not super sure about this though.

make test and make integ pass.

@efekarakus
Copy link
Contributor

@PettitWesley I meant for example can we use the tool locally and see some metric increment in the backend?

// CustomUserAgentHandler returns a http request handler that sets a custom user agent to all aws requests
func CustomUserAgentHandler() request.NamedHandler {
return request.NamedHandler{
Name: "ECSCLIUserAgentHandler",
Copy link
Contributor

Choose a reason for hiding this comment

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

C/P leftovers? We should rename this "ECSLocalEndpointsAgentHandler", yes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ooops

@PettitWesley PettitWesley force-pushed the user-agent branch 2 times, most recently from 3681abb to 2ad10f1 Compare April 4, 2019 23:57
go.sum Outdated
@@ -1,6 +1,7 @@
github.com/Microsoft/go-winio v0.4.12/go.mod h1:VhR8bwka0BXejwEJY73c50VrPtXAaKcyvVC4A4RozmA=
github.com/aws/amazon-ecs-agent v1.26.0 h1:kT/JZYXpCUXSjNaTWrYjlCPayWCV6dXaMQKOYq5JLfo=
github.com/aws/amazon-ecs-agent v1.26.0/go.mod h1:as/yyU3OZwCtKXO0Bv9dieYPupNo7jnNNZRNQTu9Row=
github.com/aws/amazon-ecs-cli v1.13.0 h1:cOOwtpChp2JhDK7PkIoHN5OKNhCPSQhaL7n3Pz7nIYk=
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be removed? I don't see anywhere new that's using it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

huh.... this is autogenerated.. not sure why it didn't get auto-removed when I re-built after removing it from go.mod

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@PettitWesley PettitWesley merged commit e881243 into master Apr 5, 2019
@PettitWesley PettitWesley deleted the user-agent branch April 5, 2019 22:21
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.

None yet

3 participants