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

Pass new user agent prefix for Amplify calls #271

Closed
palpatim opened this issue Dec 19, 2019 · 10 comments
Closed

Pass new user agent prefix for Amplify calls #271

palpatim opened this issue Dec 19, 2019 · 10 comments
Assignees

Comments

@palpatim
Copy link
Member

Currently the API category uses a user agent specified in a constants file (see #75). We need to update this:

@lawmicha
Copy link
Contributor

lawmicha commented Dec 20, 2019

@royjit 's previous work #166 .

  • PR is closed and not merged (was pending some decision that i can't remember what it was right now.
  • Implementation uses a subclass of AWSServiceConfiguration to override the default useragent

@wooj2
Copy link
Contributor

wooj2 commented Dec 20, 2019

I did a deep dive last week of the user-agents we have been sending out.

Currently in Production:

AWS SDK iOS

Constructed as part of AWSCore, the user agent looks like the following:

    "aws-sdk-iOS/<version> iOS/<iosVersion> <locale>"

(Note that there can be some variations if you look at line 179 of AWSService.m)

Amplify Storage/Predictions & other categories using the AWS SDK iOS (BUT NOT Amplify API/DataStore)

Since Jithin’s PR has not been pushed to mainline yet, the user agent should be using the same user agent as the AWS iOS SDK, so therefore, it should be:

    "aws-sdk-iOS/<version> iOS/<iosVersion> <locale>"

Amplify API (and Amplify DataStore):

When using IAM or userPools, the user agent is:

    "amplify-ios/0.9.0 Amplify"

When using an APIkey, we are not setting the user agent (this is a bug, and we need to fix this)

    Currently a bug, and will need to get fixed

AppSync

Based on this link: https://github.com/awslabs/aws-mobile-appsync-sdk-ios/blob/master/AWSAppSyncClient/AWSAppSyncHTTPNetworkTransport.swift#L114
We seem to be sending out a user agent of:

    “aws-sdk-ios/3.0.0 AppSyncClient"

In the short term (prior to P31016485 getting resolved -- e.g. backend parsing being completed):

We recommend two immediate changes.

  1. Change Amplify API user agent from:
“amplify-ios/0.9.0 Amplify”

To:

"aws-sdk-iOS/2.12.2 iOS/<iosVersion> <locale>"

We believe that by making this change, the metrics team will be able to provide us some basic usage metrics (despite not being able to differentiate between actual SDK usage vs amplify usage)
2. Fix the bug in Amplify API to set the user-agent when using an API key.

Longer term (but hopefully not too long)

Sit down with the metrics team to agree on a format for the user agent we send from the amplify clients to the backend services so that we can clearly attribute calls from amplify vs calls from AWS SDK iOS.
Make appropriate changes & test

@wooj2
Copy link
Contributor

wooj2 commented Dec 21, 2019

Sync'ed with Mohit from the product side, we have decided that

  1. We will not be doing a "short term fix"
  2. the desired user agent on iOS will be in the following format:
amplify-iOS/<version> <systemName>/<systemVersion> <locale>

@wooj2 wooj2 self-assigned this Dec 21, 2019
@wooj2
Copy link
Contributor

wooj2 commented Dec 21, 2019

PR here: #273

@wooj2
Copy link
Contributor

wooj2 commented Dec 24, 2019

Verification of building amplify into a sample app and making a GraphQL call via data store.
VerifyViagraphqlRequest

@wooj2
Copy link
Contributor

wooj2 commented Dec 24, 2019

Verification of building storage api into a sample app and making a storage call
verifyStorage

One thing i did notice however is that when we authenticate via codigo, we are continuning to use the aws-sdk-ios user agent from that call, but i suppose that is expected.

@wooj2
Copy link
Contributor

wooj2 commented Dec 24, 2019

Three more items we should verify w/ charles proxy prior to closing this issue:

  • API-REST calls
  • Predictions
  • Pinpoint

@wooj2
Copy link
Contributor

wooj2 commented Dec 24, 2019

Verification of predictions calls:
verifyComprehend
verifyTextTranslate
VerifyRecognition

@wooj2
Copy link
Contributor

wooj2 commented Dec 24, 2019

Verified with api w/ apikey... didn't verify other mechanisms, but based on how the code is structured w/ interceptors, we should be good to go on the API category.
Verify_API_REST_APIKEY

@wooj2
Copy link
Contributor

wooj2 commented Dec 24, 2019

Verified pinpoint:
verifypinpoint

manually verified each of the categories, lgtm!

@wooj2 wooj2 closed this as completed Dec 24, 2019
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

No branches or pull requests

3 participants