API CHANGE: NewClient(clientName, clientVersion) (#21)

* API CHANGE: NewClient(clientName, clientVersion)

This commit changes the NewClient API to take two arguments, the
clientName and the clientVersion, instead of a userAgent.

We didn't specify how userAgent was made. In theory we expected it
to be composed of `${clientName}/${clientVersion}` but that was not
enforced in any way. Per #16, we want the client to identify with
the server also using the query string.

This is for consistency with what the JavaScript client does.

While there, also provide information on the OS and the architecture
of the client, again as part of the query string.

Closes #16.

cmd/ndt7-client/main.go

@@ -86,7 +86,8 @@ import (
 )
 
 const (
-	userAgent      = "ndt7-client-go/0.1.0"
+	clientName     = "ndt7-client-go-cmd"
+	clientVersion  = "0.1.0"
 	defaultTimeout = 55 * time.Second
 )
 
@@ -163,7 +164,7 @@ func main() {
 	ctx, cancel := context.WithTimeout(context.Background(), *flagTimeout)
 	defer cancel()
 	var r runner
-	r.client = ndt7.NewClient(userAgent)
+	r.client = ndt7.NewClient(clientName, clientVersion)
 	r.client.Dialer.TLSClientConfig = &tls.Config{
 		InsecureSkipVerify: *flagNoVerify,
 	}

cmd/ndt7-client/main_test.go

@@ -103,7 +103,7 @@ func (me mockedEmitter) OnComplete(subtest string) error {
 // the emitter.OnStarting function fails.
 func TestRunSubtestOnStartingError(t *testing.T) {
 	runner := runner{
-		client: ndt7.NewClient(userAgent),
+		client: ndt7.NewClient(clientName, clientVersion),
 		emitter: mockedEmitter{
 			StartingError: errors.New("mocked error"),
 		},
@@ -129,7 +129,7 @@ func TestRunSubtestOnStartingError(t *testing.T) {
 // the emitter.OnConnected function fails.
 func TestRunSubtestOnConnectedError(t *testing.T) {
 	runner := runner{
-		client: ndt7.NewClient(userAgent),
+		client: ndt7.NewClient(clientName, clientVersion),
 		emitter: mockedEmitter{
 			ConnectedError: errors.New("mocked error"),
 		},
@@ -155,7 +155,7 @@ func TestRunSubtestOnConnectedError(t *testing.T) {
 // the emitter.OnComplete function fails.
 func TestRunSubtestOnCompleteError(t *testing.T) {
 	runner := runner{
-		client: ndt7.NewClient(userAgent),
+		client: ndt7.NewClient(clientName, clientVersion),
 		emitter: mockedEmitter{
 			CompleteError: errors.New("mocked error"),
 		},
@@ -181,7 +181,7 @@ func TestRunSubtestOnCompleteError(t *testing.T) {
 // the emitEvent function fails.
 func TestRunSubtestEmitEventError(t *testing.T) {
 	runner := runner{
-		client:  ndt7.NewClient(userAgent),
+		client:  ndt7.NewClient(clientName, clientVersion),
 		emitter: mockedEmitter{},
 	}
 	code := runner.runSubtest(
@@ -213,7 +213,7 @@ func TestBatchEmitterEventsOrderNormal(t *testing.T) {
 	}
 	writer := &mocks.SavingWriter{}
 	runner := runner{
-		client:  ndt7.NewClient(userAgent),
+		client:  ndt7.NewClient(clientName, clientVersion),
 		emitter: emitter.Batch{Writer: writer},
 	}
 	code := runner.runSubtest(
@@ -269,7 +269,7 @@ func TestBatchEmitterEventsOrderFailure(t *testing.T) {
 	}
 	writer := &mocks.SavingWriter{}
 	runner := runner{
-		client:  ndt7.NewClient(userAgent),
+		client:  ndt7.NewClient(clientName, clientVersion),
 		emitter: emitter.Batch{Writer: writer},
 	}
 	runner.client.MLabNSClient.BaseURL = "\t" // URL parser error

example_test.go

@@ -12,7 +12,7 @@ import (
 func Example() {
 	ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
 	defer cancel()
-	client := ndt7.NewClient("example/v0.1.0")
+	client := ndt7.NewClient("ndt7-client-go-example", "0.1.0")
 	ch, err := client.StartDownload(ctx)
 	if err != nil {
 		log.Fatal(err)

ndt7.go

@@ -13,6 +13,7 @@ import (
 	"context"
 	"net/http"
 	"net/url"
+	"runtime"
 	"time"
 
 	"github.com/gorilla/websocket"
@@ -24,6 +25,14 @@ import (
 	"github.com/m-lab/ndt7-client-go/spec"
 )
 
+const (
+	// libraryName is the name of this library
+	libraryName = "ndt7-client-go"
+
+	// libraryVersion is the version of this library
+	libraryVersion = "0.1.0"
+)
+
 // locateFn is the type of function used to locate a server.
 type locateFn = func(ctx context.Context, client *mlabns.Client) (string, error)
 
@@ -58,9 +67,13 @@ type Client struct {
 	// defaults in NewClient and you may override it.
 	MLabNSClient *mlabns.Client
 
-	// UserAgent is the user-agent that will be used. It's set by
+	// ClientName is the name of the software running ndt7 tests. It's set by
 	// NewClient; you may want to change this value.
-	UserAgent string
+	ClientName string
+
+	// ClientVersion is the version of the software running ndt7 tests. It's
+	// set by NewClient; you may want to change this value.
+	ClientVersion string
 
 	// connect is the function for connecting a specific
 	// websocket cnnection. It's set to its default value by
@@ -80,11 +93,18 @@ type Client struct {
 	upload subtestFn
 }
 
-// NewClient creates a new client instance identified by the
-// specified user agent. M-Lab services may reject requests coming
-// from clients with empty user agents in the future.
-func NewClient(userAgent string) *Client {
+// makeUserAgent creates the user agent string
+func makeUserAgent(clientName, clientVersion string) string {
+	return clientName + "/" + clientVersion + " " + libraryName + "/" + libraryVersion
+}
+
+// NewClient creates a new client instance identified by the specified
+// clientName and clientVersion. M-Lab services may reject requests coming
+// from clients that do not identify themselves properly.
+func NewClient(clientName, clientVersion string) *Client {
 	return &Client{
+		ClientName:    clientName,
+		ClientVersion: clientVersion,
 		connect: func(
 			dialer websocket.Dialer, ctx context.Context, urlStr string,
 			requestHeader http.Header) (*websocket.Conn, *http.Response, error,
@@ -98,9 +118,10 @@ func NewClient(userAgent string) *Client {
 		locate: func(ctx context.Context, c *mlabns.Client) (string, error) {
 			return c.Query(ctx)
 		},
-		MLabNSClient: mlabns.NewClient("ndt7", userAgent),
-		upload:       upload.Run,
-		UserAgent:    userAgent,
+		MLabNSClient: mlabns.NewClient(
+			"ndt7", makeUserAgent(clientName, clientVersion),
+		),
+		upload: upload.Run,
 	}
 }
 
@@ -115,9 +136,17 @@ func (c *Client) doConnect(ctx context.Context, URLPath string) (*websocket.Conn
 	URL.Scheme = "wss"
 	URL.Host = c.FQDN
 	URL.Path = URLPath
+	q := URL.Query()
+	q.Set("client_arch", runtime.GOARCH)
+	q.Set("client_library_name", libraryName)
+	q.Set("client_library_version", libraryVersion)
+	q.Set("client_name", c.ClientName)
+	q.Set("client_os", runtime.GOOS)
+	q.Set("client_version", c.ClientVersion)
+	URL.RawQuery = q.Encode()
 	headers := http.Header{}
 	headers.Add("Sec-WebSocket-Protocol", params.SecWebSocketProtocol)
-	headers.Add("User-Agent", c.UserAgent)
+	headers.Add("User-Agent", makeUserAgent(c.ClientName, c.ClientVersion))
 	conn, _, err := c.connect(c.Dialer, ctx, URL.String(), headers)
 	return conn, err
 }

ndt7_test.go

@@ -11,13 +11,15 @@ import (
 	"github.com/m-lab/ndt7-client-go/spec"
 )
 
-// userAgent is the user agent used throughout this file
-const userAgent = "ndt7-client-go/0.1.0"
+const (
+	clientName    = "ndt7-client-go-tests"
+	clientVersion = "0.1.0"
+)
 
 // newMockedClient returns a mocked client that does nothing
 // except pretending it is doing something.
 func newMockedClient(ctx context.Context) *Client {
-	client := NewClient(userAgent)
+	client := NewClient(clientName, clientVersion)
 	// Override locate to return a fake IP address
 	client.locate = func(ctx context.Context, c *mlabns.Client) (string, error) {
 		return "127.0.0.1", nil
@@ -72,7 +74,7 @@ func TestUploadCase(t *testing.T) {
 // with an error when discovering a server.
 func TestStartDiscoverServerError(t *testing.T) {
 	ctx := context.Background()
-	client := NewClient(userAgent)
+	client := NewClient(clientName, clientVersion)
 	client.MLabNSClient.BaseURL = "\t" // cause URL parse to fail
 	_, err := client.start(ctx, nil, "")
 	if err == nil {
@@ -84,7 +86,7 @@ func TestStartDiscoverServerError(t *testing.T) {
 // with an error when connecting.
 func TestStartConnectError(t *testing.T) {
 	ctx := context.Background()
-	client := NewClient(userAgent)
+	client := NewClient(clientName, clientVersion)
 	client.FQDN = "\t" // cause URL parse to fail
 	_, err := client.start(ctx, nil, "")
 	if err == nil {
@@ -97,7 +99,7 @@ func TestIntegrationDownload(t *testing.T) {
 	if testing.Short() {
 		t.Skip("Skipping test in short mode")
 	}
-	client := NewClient(userAgent)
+	client := NewClient(clientName, clientVersion)
 	ch, err := client.StartDownload(context.Background())
 	if err != nil {
 		t.Fatal(err)
@@ -139,7 +141,7 @@ func TestIntegrationUpload(t *testing.T) {
 	if testing.Short() {
 		t.Skip("Skipping test in short mode")
 	}
-	client := NewClient(userAgent)
+	client := NewClient(clientName, clientVersion)
 	ch, err := client.StartUpload(context.Background())
 	if err != nil {
 		t.Fatal(err)