Merge lp:~thumper/juju-core/debug-log-api-client into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Add the debug-log client API

The client connection now caches the x509 pool so it can
be passed along with the websocket connection for the debug
log call.

Add the debug-log client api

This involved a little more work than I expected.
The client connection now caches the x509 pool so it can
be passed along with the websocket connection for the debug
log call.

A key point of interest, is that if the remote api server
does not have a websocket listener on '/log', the dial config
just blocks and never returns, so we can't wait for an error
on that to determine that the remote api server doesn't support
the api yet. I thought it would be good to be able to ask
the api server what version it was running, so in future, if we
hit this again, we can at least have a version check. The mere
existance of the version call is enough for us to determine that
the remote server supports debug-log, so we don't care about
the actual value.

Initially I used a bufio scanner to read the first line of the
response, however that reads 4k into a buffer, which killed all
the tests. I now read a byte at a time to get the json encoded
error. It would probably be much better to use the websocket
methods to read messages, but not sure how that would impact the
gui.

https://codereview.appspot.com/85850043/

[Tim Penhey (thumper)]

Reviewers: mp+214875_code.launchpad.net,

Message:
Please take a look.

Description:
Add the debug-log client api

This involved a little more work than I expected.
The client connection now caches the x509 pool so it can
be passed along with the websocket connection for the debug
log call.

A key point of interest, is that if the remote api server
does not have a websocket listener on '/log', the dial config
just blocks and never returns, so we can't wait for an error
on that to determine that the remote api server doesn't support
the api yet. I thought it would be good to be able to ask
the api server what version it was running, so in future, if we
hit this again, we can at least have a version check. The mere
existance of the version call is enough for us to determine that
the remote server supports debug-log, so we don't care about
the actual value.

Initially I used a bufio scanner to read the first line of the
response, however that reads 4k into a buffer, which killed all
the tests. I now read a byte at a time to get the json encoded
error. It would probably be much better to use the websocket
methods to read messages, but not sure how that would impact the
gui.

https://code.launchpad.net/~thumper/juju-core/debug-log-api-client/+merge/214875

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/85850043/

Affected files (+322, -4 lines):
   A [revision details]
   M state/api/apiclient.go
   M state/api/client.go
   M state/api/client_test.go
   M state/api/export_test.go
   M state/apiserver/client/client.go
   M state/apiserver/client/client_test.go
   M state/apiserver/debuglog_test.go
   M utils/http.go
   M utils/http_test.go

[Tim Penhey (thumper)]

Please take a look.

https://codereview.appspot.com/85850043/

[Roger Peppe (rogpeppe)]

Looks great in general. Quite a few minor points but nothing too
substantial.

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go
File state/api/client.go (right):

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode715
state/api/client.go:715: func (c *Client) Version() (version.Number,
error) {
I think I'd call this AgentVersion.
We might have different ideas of API version later.

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode720
state/api/client.go:720: if result.Error != nil {
This isn't possible. The Error field is only there for bulk version
results.
I'd either ignore the field, or make a new type in params
(e.g. type AgentVersionResult {Version version.Number})

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode726
state/api/client.go:726: // Allow overriding in tests.
I'd be tempted to name this websocketDialConfig, because that's what
we're
mocking.

// websocketDialConfig is called instead of websocket.DialConfig
// so we can override it in tests.
?

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode735
state/api/client.go:735: // IsConnectionError returns true if the error
is a connection error.
// IsConnectionError reports whether the error is a connection
// error.

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode741
state/api/client.go:741: // Params for WatchDebugLog
Please can we document this type and its fields?
This is the public face of the Go Juju API.

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode755
state/api/client.go:755: // machines or units. The watching is started
the given number of
The last sentence doesn't seem to fit here.

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode756
state/api/client.go:756: // matching lines back in history.

// It returns an error that satisfies IsConnectionError
// when the connection cannot be made.

?

I'm not quite sure why we want to distinguish a connection
error from the other kinds of errors though.

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode759
state/api/client.go:759: // end point (not sure why). So do a version
check, as version was added
The reason that the websocket connection hangs is that
the server is serving the API on every URL (that was
a mistake that I should have fixed earlier, sorry),
so you're waiting for an RPC reply without sending
any request.

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode768
state/api/client.go:768: attrs["replay"] =
[]string{strconv.FormatBool(args.Replay)}
attrs.Set(fmt.Sprint(args.Replay))

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode771
state/api/client.go:771: attrs["maxLines"] =
[]string{fmt.Sprint(args.Limit)}
attrs.Set(fmt.Sprint(args.Limit))
etc

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode780
state/api/client.go:780: if len(arg) > 0 {
Is this actually necessary. If there's a zero-length slice in the
attributes, does it matter if it's nil or empty?

I think just
attrs["inc...

[John A Meinel (jameinel)]

I believe old servers respond to plain API requests on any URL (so /log
is just another API endpoint). So I think you *could* issue a request
there and see if it responds? Or try to log and then if we haven't
gotten any data after X seconds, issue a request.

https://codereview.appspot.com/85850043/

[Tim Penhey (thumper)]

Please take a look.

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go
File state/api/client.go (right):

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode715
state/api/client.go:715: func (c *Client) Version() (version.Number,
error) {
On 2014/04/09 10:28:40, rog wrote:
> I think I'd call this AgentVersion.
> We might have different ideas of API version later.

Done.

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode720
state/api/client.go:720: if result.Error != nil {
On 2014/04/09 10:28:40, rog wrote:
> This isn't possible. The Error field is only there for bulk version
results.
> I'd either ignore the field, or make a new type in params
> (e.g. type AgentVersionResult {Version version.Number})

I don't like ignoring it, so I'll change the response type.

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode726
state/api/client.go:726: // Allow overriding in tests.
On 2014/04/09 10:28:40, rog wrote:
> I'd be tempted to name this websocketDialConfig, because that's what
we're
> mocking.

> // websocketDialConfig is called instead of websocket.DialConfig
> // so we can override it in tests.
> ?

Done.

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode735
state/api/client.go:735: // IsConnectionError returns true if the error
is a connection error.
On 2014/04/09 10:28:40, rog wrote:
> // IsConnectionError reports whether the error is a connection
> // error.

Done.

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode741
state/api/client.go:741: // Params for WatchDebugLog
On 2014/04/09 10:28:40, rog wrote:
> Please can we document this type and its fields?
> This is the public face of the Go Juju API.

Done.

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode755
state/api/client.go:755: // machines or units. The watching is started
the given number of
On 2014/04/09 10:28:40, rog wrote:
> The last sentence doesn't seem to fit here.

Rewritten

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode756
state/api/client.go:756: // matching lines back in history.
On 2014/04/09 10:28:40, rog wrote:

> // It returns an error that satisfies IsConnectionError
> // when the connection cannot be made.

> ?

> I'm not quite sure why we want to distinguish a connection
> error from the other kinds of errors though.

The next branch uses the connection error to determine whether to just
report the error and finish, or to try tailing the debug log file over
ssh like we did before as a fallback for older servers.

https://codereview.appspot.com/85850043/diff/20001/state/api/client.go#newcode759
state/api/client.go:759: // end point (not sure why). So do a version
check, as version was added
On 2014/04/09 10:28:40, rog wrote:
> The reason that the websocket connection hangs is that
> the server is serving the API on every URL (that was
> a mistake that I should have fixed earlier, sorry),
> so you're waiting for an RPC reply without sending
> any request.

Ah.. thanks for the explanation. I had guessed something like this but
wasn't entirely sure.

h...

[Tim Penhey (thumper)]

Please take a look.

https://codereview.appspot.com/85850043/

[Roger Peppe (rogpeppe)]

LGTM with a few trivials.
Thanks!

https://codereview.appspot.com/85850043/diff/20001/state/api/client_test.go
File state/api/client_test.go (right):

https://codereview.appspot.com/85850043/diff/20001/state/api/client_test.go#newcode211
state/api/client_test.go:211: func echoUrl(c *gc.C)
func(*websocket.Config) (io.ReadCloser, error) {
On 2014/04/10 03:59:50, thumper wrote:
> On 2014/04/09 10:28:40, rog wrote:
> > Here's the source of your buffering problem.
> > This fixes it:
> >
> > func echoUrl(c *gc.C) func(*websocket.Config) (io.ReadCloser, error)
{
> > message, err := json.Marshal(&params.ErrorResult{})
> > c.Assert(err, gc.IsNil)
> > return func(config *websocket.Config) (io.ReadCloser, error) {
> > pr, pw := io.Pipe()
> > go func() {
> > fmt.Fprintf(pw, "%s\n", message)
> > fmt.Fprintf(pw, "%s\n", config.Location)
> > }()
> > return pr, nil
> > }
> > }

> That is pretty nice. I take it io.Pipe also does the reading message
boundaries?

yup, io.Pipe does no buffering at all - each write corresponds exactly
to each read.

https://codereview.appspot.com/85850043/diff/60001/state/api/client.go
File state/api/client.go (right):

https://codereview.appspot.com/85850043/diff/60001/state/api/client.go#newcode738
state/api/client.go:738: // Params for WatchDebugLog controls the
filtering of the log messages. If the
// DebugLogParams holds parameters for WatchDebugLog that
// control the filtering of the log messages.

?

Thanks a lot for documenting these, BTW. It makes a big difference.

https://codereview.appspot.com/85850043/diff/60001/state/api/client.go#newcode747
state/api/client.go:747: // are set all modules are considered
included.
s/ / /

Does the inclusion of a logging module here imply all its sub-modules
too?

https://codereview.appspot.com/85850043/diff/60001/state/api/client.go#newcode752
state/api/client.go:752: // ExcludeModule lists logging modules to
exclude from the resposne.
ditto

https://codereview.appspot.com/85850043/diff/60001/state/api/client.go#newcode755
state/api/client.go:755: // have been sent, the socket is closed.
// If zero, there is no limit.
?

https://codereview.appspot.com/85850043/diff/60001/state/api/client.go#newcode763
state/api/client.go:763: // than the end. If replay is true, backlog is
ignored.
Nice. I started commenting on Backlog to say "but what if I want no
lines of backlog?" - then I saw this.

https://codereview.appspot.com/85850043/diff/60001/state/api/client.go#newcode821
state/api/client.go:821: logger.Debugf("initial line: %s", line)
Just realised - we probably want %q here so that we don't get a blank
line in the log.

https://codereview.appspot.com/85850043/diff/60001/state/api/client_test.go
File state/api/client_test.go (right):

https://codereview.appspot.com/85850043/diff/60001/state/api/client_test.go#newcode140
state/api/client_test.go:140: return &badReader{ioutil.NopCloser(junk),
err}, nil
or ioutil.NopCloser(&badReader{err})

then you can lose the ReadCloser field inside badReader.

https://codereview.appspot.com/85850043/diff/60001/state/api/client_test.go#newcode195
state/api/client_test.go:195: func echoUrl(c *gc.C)
func(*websocket.Config) (io.ReadClos...

[Tim Penhey (thumper)]

Please take a look.

https://codereview.appspot.com/85850043/diff/60001/state/api/client.go
File state/api/client.go (right):

https://codereview.appspot.com/85850043/diff/60001/state/api/client.go#newcode738
state/api/client.go:738: // Params for WatchDebugLog controls the
filtering of the log messages. If the
On 2014/04/10 07:04:18, rog wrote:
> // DebugLogParams holds parameters for WatchDebugLog that
> // control the filtering of the log messages.

> ?

> Thanks a lot for documenting these, BTW. It makes a big difference.

Done.

https://codereview.appspot.com/85850043/diff/60001/state/api/client.go#newcode747
state/api/client.go:747: // are set all modules are considered
included.
On 2014/04/10 07:04:18, rog wrote:
> s/ / /

> Does the inclusion of a logging module here imply all its sub-modules
too?

Yes, updated.

https://codereview.appspot.com/85850043/diff/60001/state/api/client.go#newcode755
state/api/client.go:755: // have been sent, the socket is closed.
On 2014/04/10 07:04:18, rog wrote:
> // If zero, there is no limit.
> ?

Updated.

https://codereview.appspot.com/85850043/diff/60001/state/api/client.go#newcode821
state/api/client.go:821: logger.Debugf("initial line: %s", line)
On 2014/04/10 07:04:18, rog wrote:
> Just realised - we probably want %q here so that we don't get a blank
line in
> the log.

Done.

https://codereview.appspot.com/85850043/diff/60001/state/api/client_test.go
File state/api/client_test.go (right):

https://codereview.appspot.com/85850043/diff/60001/state/api/client_test.go#newcode140
state/api/client_test.go:140: return &badReader{ioutil.NopCloser(junk),
err}, nil
On 2014/04/10 07:04:18, rog wrote:
> or ioutil.NopCloser(&badReader{err})

> then you can lose the ReadCloser field inside badReader.

Good call.

https://codereview.appspot.com/85850043/diff/60001/state/api/client_test.go#newcode195
state/api/client_test.go:195: func echoUrl(c *gc.C)
func(*websocket.Config) (io.ReadCloser, error) {
On 2014/04/10 07:04:18, rog wrote:
> s/echoUrl/echoURL/

> (Go convention is to capitalise either all or none of an acronym)

Done.

https://codereview.appspot.com/85850043/diff/60001/state/api/params/internal.go
File state/api/params/internal.go (right):

https://codereview.appspot.com/85850043/diff/60001/state/api/params/internal.go#newcode563
state/api/params/internal.go:563: // apiserver.
On 2014/04/10 07:04:18, rog wrote:
> s/apiserver/agent running the API server./
> ?

Done.

https://codereview.appspot.com/85850043/diff/60001/state/apiserver/client/client.go
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/85850043/diff/60001/state/apiserver/client/client.go#newcode785
state/apiserver/client/client.go:785: // AgentVersion returns the
current version that the api server is running.
On 2014/04/10 07:04:18, rog wrote:
> s/api/API/

Done.

https://codereview.appspot.com/85850043/

=== modified file 'state/api/apiclient.go'
--- state/api/apiclient.go	2014-04-09 15:17:04 +0000
+++ state/api/apiclient.go	2014-04-10 09:38:03 +0000
@@ -56,6 +56,10 @@
 	// serverRoot holds the cached API server address and port we used
 	// to login, with a https:// prefix.
 	serverRoot string
+
+	// certPool holds the cert pool that is used to authenticate the tls
+	// connections to the API.
+	certPool *x509.CertPool
 }
 
 // Info encapsulates information about a server holding juju state and
@@ -151,6 +155,7 @@
 		serverRoot: "https://" + conn.Config().Location.Host,
 		tag:        info.Tag,
 		password:   info.Password,
+		certPool:   pool,
 	}
 	if info.Tag != "" || info.Password != "" {
 		if err := st.Login(info.Tag, info.Password, info.Nonce); err != nil {

=== modified file 'state/api/client.go'
--- state/api/client.go	2014-04-10 08:47:38 +0000
+++ state/api/client.go	2014-04-10 09:38:03 +0000
@@ -4,14 +4,20 @@
 package api
 
 import (
+	"crypto/tls"
 	"encoding/json"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net/http"
+	"net/url"
 	"os"
 	"strings"
 	"time"
 
+	"code.google.com/p/go.net/websocket"
+	"github.com/juju/loggo"
+
 	"launchpad.net/juju-core/charm"
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/instance"
@@ -703,3 +709,128 @@
 	}
 	return c.call("EnsureAvailability", args, nil)
 }
+
+// AgentVersion reports the version number of the api server.
+func (c *Client) AgentVersion() (version.Number, error) {
+	var result params.AgentVersionResult
+	if err := c.call("AgentVersion", nil, &result); err != nil {
+		return version.Number{}, err
+	}
+	return result.Version, nil
+}
+
+// websocketDialConfig is called instead of websocket.DialConfig so we can
+// override it in tests.
+var websocketDialConfig = func(config *websocket.Config) (io.ReadCloser, error) {
+	return websocket.DialConfig(config)
+}
+
+type connectionError struct {
+	error
+}
+
+// IsConnectionError reports whether the error is a connection error.
+func IsConnectionError(err error) bool {
+	_, ok := err.(*connectionError)
+	return ok
+}
+
+// DebugLogParams holds parameters for WatchDebugLog that control the
+// filtering of the log messages. If the structure is zero initialized, the
+// entire log file is sent back starting from the end, and until the user
+// closes the connection.
+type DebugLogParams struct {
+	// IncludeEntity lists entity tags to include in the response. Tags may
+	// finish with a '*' to match a prefix e.g.: unit-mysql-*, machine-2. If
+	// none are set, then all lines are considered included.
+	IncludeEntity []string
+	// IncludeModule lists logging modules to include in the response. If none
+	// are set all modules are considered included.  If a module is specified,
+	// all the submodules also match.
+	IncludeModule []string
+	// ExcludeEntity lists entity tags to exclude from the response. As with
+	// IncludeEntity the values may finish with a '*'.
+	ExcludeEntity []string
+	// ExcludeModule lists logging modules to exclude from the resposne. If a
+	// module is specified, all the submodules are also excluded.
+	ExcludeModule []string
+	// Limit defines the maximum number of lines to return. Once this many
+	// have been sent, the socket is closed.  If zero, all filtered lines are
+	// sent down the connection until the client closes the connection.
+	Limit uint
+	// Backlog tells the server to try to go back this many lines before
+	// starting filtering. If backlog is zero and replay is false, then there
+	// may be an initial delay until the next matching log message is written.
+	Backlog uint
+	// Level specifies the minimum logging level to be sent back in the response.
+	Level loggo.Level
+	// Replay tells the server to start at the start of the log file rather
+	// than the end. If replay is true, backlog is ignored.
+	Replay bool
+}
+
+// WatchDebugLog returns a ReadCloser that the caller can read the log lines
+// from. Only log lines that match the filtering specified in the
+// DebugLogParams are returned. It returns an error that satisfies
+// IsConnectionError when the connection cannot be made.
+func (c *Client) WatchDebugLog(args DebugLogParams) (io.ReadCloser, error) {
+	// The websocket connection just hangs if the server doesn't have the log
+	// end point. So do a version check, as version was added at the same time
+	// as the remote end point.
+	_, err := c.AgentVersion()
+	if err != nil {
+		return nil, &connectionError{fmt.Errorf("server doesn't support debug log websocket")}
+	}
+	// Prepare URL.
+	attrs := url.Values{}
+	if args.Replay {
+		attrs.Set("replay", fmt.Sprint(args.Replay))
+	}
+	if args.Limit > 0 {
+		attrs.Set("maxLines", fmt.Sprint(args.Limit))
+	}
+	if args.Backlog > 0 {
+		attrs.Set("backlog", fmt.Sprint(args.Backlog))
+	}
+	if args.Level != loggo.UNSPECIFIED {
+		attrs.Set("level", fmt.Sprint(args.Level))
+	}
+	attrs["includeEntity"] = args.IncludeEntity
+	attrs["includeModule"] = args.IncludeModule
+	attrs["excludeEntity"] = args.ExcludeEntity
+	attrs["excludeModule"] = args.ExcludeModule
+
+	target := url.URL{
+		Scheme:   "wss",
+		Host:     c.st.addr,
+		Path:     "/log",
+		RawQuery: attrs.Encode(),
+	}
+	cfg, err := websocket.NewConfig(target.String(), "http://localhost/")
+	cfg.Header = utils.BasicAuthHeader(c.st.tag, c.st.password)
+	cfg.TlsConfig = &tls.Config{RootCAs: c.st.certPool, ServerName: "anything"}
+	connection, err := websocketDialConfig(cfg)
+	if err != nil {
+		return nil, &connectionError{err}
+	}
+	// Read the initial error and translate to a real error.
+	// Read up to the first new line character. We can't use bufio here as it
+	// reads too much from the reader.
+	line := make([]byte, 4096)
+	n, err := connection.Read(line)
+	if err != nil {
+		return nil, fmt.Errorf("unable to read initial response: %v", err)
+	}
+	line = line[0:n]
+
+	logger.Debugf("initial line: %q", line)
+	var errResult params.ErrorResult
+	err = json.Unmarshal(line, &errResult)
+	if err != nil {
+		return nil, fmt.Errorf("unable to unmarshal initial response: %v", err)
+	}
+	if errResult.Error != nil {
+		return nil, errResult.Error
+	}
+	return connection, nil
+}

=== modified file 'state/api/client_test.go'
--- state/api/client_test.go	2014-04-01 15:28:49 +0000
+++ state/api/client_test.go	2014-04-10 09:38:03 +0000
@@ -4,10 +4,19 @@
 package api_test
 
 import (
+	"bufio"
+	"bytes"
+	"encoding/json"
 	"fmt"
+	"io"
+	"io/ioutil"
 	"net"
 	"net/http"
+	"net/url"
+	"strings"
 
+	"code.google.com/p/go.net/websocket"
+	"github.com/juju/loggo"
 	jc "github.com/juju/testing/checkers"
 	gc "launchpad.net/gocheck"
 
@@ -83,3 +92,114 @@
 	_, err = client.AddLocalCharm(curl, charmArchive)
 	c.Assert(err, jc.Satisfies, params.IsCodeNotImplemented)
 }
+
+func (s *clientSuite) TestWatchDebugLogConnected(c *gc.C) {
+	// Shows both the unmarshalling of a real error, and
+	// that the api server is connected.
+	client := s.APIState.Client()
+	reader, err := client.WatchDebugLog(api.DebugLogParams{})
+	c.Assert(err, gc.ErrorMatches, "cannot open log file: .*")
+	c.Assert(reader, gc.IsNil)
+}
+
+func (s *clientSuite) TestConnectionErrorBadConnection(c *gc.C) {
+	s.PatchValue(api.WebsocketDialConfig, func(_ *websocket.Config) (io.ReadCloser, error) {
+		return nil, fmt.Errorf("bad connection")
+	})
+	client := s.APIState.Client()
+	reader, err := client.WatchDebugLog(api.DebugLogParams{})
+	c.Assert(err, gc.ErrorMatches, "bad connection")
+	c.Assert(reader, gc.IsNil)
+}
+
+func (s *clientSuite) TestConnectionErrorNoData(c *gc.C) {
+	s.PatchValue(api.WebsocketDialConfig, func(_ *websocket.Config) (io.ReadCloser, error) {
+		return ioutil.NopCloser(&bytes.Buffer{}), nil
+	})
+	client := s.APIState.Client()
+	reader, err := client.WatchDebugLog(api.DebugLogParams{})
+	c.Assert(err, gc.ErrorMatches, "unable to read initial response: EOF")
+	c.Assert(reader, gc.IsNil)
+}
+
+func (s *clientSuite) TestConnectionErrorBadData(c *gc.C) {
+	s.PatchValue(api.WebsocketDialConfig, func(_ *websocket.Config) (io.ReadCloser, error) {
+		junk := strings.NewReader("junk\n")
+		return ioutil.NopCloser(junk), nil
+	})
+	client := s.APIState.Client()
+	reader, err := client.WatchDebugLog(api.DebugLogParams{})
+	c.Assert(err, gc.ErrorMatches, "unable to unmarshal initial response: .*")
+	c.Assert(reader, gc.IsNil)
+}
+
+func (s *clientSuite) TestConnectionErrorReadError(c *gc.C) {
+	s.PatchValue(api.WebsocketDialConfig, func(_ *websocket.Config) (io.ReadCloser, error) {
+		err := fmt.Errorf("bad read")
+		return ioutil.NopCloser(&badReader{err}), nil
+	})
+	client := s.APIState.Client()
+	reader, err := client.WatchDebugLog(api.DebugLogParams{})
+	c.Assert(err, gc.ErrorMatches, "unable to read initial response: bad read")
+	c.Assert(reader, gc.IsNil)
+}
+
+func (s *clientSuite) TestParamsEncoded(c *gc.C) {
+	s.PatchValue(api.WebsocketDialConfig, echoURL(c))
+
+	params := api.DebugLogParams{
+		IncludeEntity: []string{"a", "b"},
+		IncludeModule: []string{"c", "d"},
+		ExcludeEntity: []string{"e", "f"},
+		ExcludeModule: []string{"g", "h"},
+		Limit:         100,
+		Backlog:       200,
+		Level:         loggo.ERROR,
+		Replay:        true,
+	}
+
+	client := s.APIState.Client()
+	reader, err := client.WatchDebugLog(params)
+	c.Assert(err, gc.IsNil)
+
+	bufReader := bufio.NewReader(reader)
+	location, err := bufReader.ReadString('\n')
+	c.Assert(err, gc.IsNil)
+	connectUrl, err := url.Parse(strings.TrimSpace(location))
+	c.Assert(err, gc.IsNil)
+
+	values := connectUrl.Query()
+	c.Assert(values, jc.DeepEquals, url.Values{
+		"includeEntity": params.IncludeEntity,
+		"includeModule": params.IncludeModule,
+		"excludeEntity": params.ExcludeEntity,
+		"excludeModule": params.ExcludeModule,
+		"maxLines":      {"100"},
+		"backlog":       {"200"},
+		"level":         {"ERROR"},
+		"replay":        {"true"},
+	})
+}
+
+// badReader raises err when Read is called.
+type badReader struct {
+	err error
+}
+
+func (r *badReader) Read(p []byte) (n int, err error) {
+	return 0, r.err
+}
+
+func echoURL(c *gc.C) func(*websocket.Config) (io.ReadCloser, error) {
+	response := &params.ErrorResult{}
+	message, err := json.Marshal(response)
+	c.Assert(err, gc.IsNil)
+	return func(config *websocket.Config) (io.ReadCloser, error) {
+		pr, pw := io.Pipe()
+		go func() {
+			fmt.Fprintf(pw, "%s\n", message)
+			fmt.Fprintf(pw, "%s\n", config.Location)
+		}()
+		return pr, nil
+	}
+}

=== modified file 'state/api/export_test.go'
--- state/api/export_test.go	2014-04-01 03:13:49 +0000
+++ state/api/export_test.go	2014-04-10 09:38:03 +0000
@@ -5,6 +5,8 @@
 
 var (
 	NewWebsocketDialer = newWebsocketDialer
+
+	WebsocketDialConfig = &websocketDialConfig
 )
 
 // SetServerRoot allows changing the URL to the internal API server

=== modified file 'state/api/params/internal.go'
--- state/api/params/internal.go	2014-04-09 15:08:51 +0000
+++ state/api/params/internal.go	2014-04-10 09:38:03 +0000
@@ -572,3 +572,9 @@
 type RunResults struct {
 	Results []RunResult
 }
+
+// AgentVersionResult is used to return the current version number of the
+// agent running the API server.
+type AgentVersionResult struct {
+	Version version.Number
+}

=== modified file 'state/apiserver/client/client.go'
--- state/apiserver/client/client.go	2014-04-10 08:47:38 +0000
+++ state/apiserver/client/client.go	2014-04-10 09:38:03 +0000
@@ -28,6 +28,7 @@
 	"launchpad.net/juju-core/state/statecmd"
 	coretools "launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/utils"
+	"launchpad.net/juju-core/version"
 )
 
 var logger = loggo.GetLogger("juju.state.apiserver.client")
@@ -782,6 +783,11 @@
 	return changes, nil
 }
 
+// AgentVersion returns the current version that the API server is running.
+func (c *Client) AgentVersion() (params.AgentVersionResult, error) {
+	return params.AgentVersionResult{Version: version.Current.Number}, nil
+}
+
 // EnvironmentGet implements the server-side part of the
 // get-environment CLI command.
 func (c *Client) EnvironmentGet() (params.EnvironmentGetResults, error) {

=== modified file 'state/apiserver/client/client_test.go'
--- state/apiserver/client/client_test.go	2014-04-10 08:47:38 +0000
+++ state/apiserver/client/client_test.go	2014-04-10 09:38:03 +0000
@@ -2211,3 +2211,11 @@
 	c.Assert(err, gc.IsNil)
 	c.Assert(apiHostPorts, gc.DeepEquals, stateAPIHostPorts)
 }
+
+func (s *clientSuite) TestClientAgentVersion(c *gc.C) {
+	current := version.MustParse("1.2.0")
+	s.PatchValue(&version.Current.Number, current)
+	result, err := s.APIState.Client().AgentVersion()
+	c.Assert(err, gc.IsNil)
+	c.Assert(result, gc.Equals, current)
+}

=== modified file 'state/apiserver/debuglog.go'
--- state/apiserver/debuglog.go	2014-04-07 04:58:59 +0000
+++ state/apiserver/debuglog.go	2014-04-10 09:38:03 +0000
@@ -41,7 +41,7 @@
 //      - as with include, it may finish with a '*'
 //   excludeModule -> []string - lists logging modules to exclude from the response
 //   limit -> uint - show *at most* this many lines
-//   backtrack -> uint
+//   backlog -> uint
 //      - go back this many lines from the end before starting to filter
 //      - has no meaning if 'replay' is true
 //   level -> string one of [TRACE, DEBUG, INFO, WARNING, ERROR]

=== modified file 'state/apiserver/debuglog_test.go'
--- state/apiserver/debuglog_test.go	2014-04-07 04:33:18 +0000
+++ state/apiserver/debuglog_test.go	2014-04-10 09:38:03 +0000
@@ -7,7 +7,6 @@
 	"bufio"
 	"crypto/tls"
 	"crypto/x509"
-	"encoding/base64"
 	"encoding/json"
 	"io"
 	"net/http"
@@ -22,6 +21,7 @@
 
 	"launchpad.net/juju-core/state/api/params"
 	"launchpad.net/juju-core/testing"
+	"launchpad.net/juju-core/utils"
 )
 
 type debugLogSuite struct {
@@ -191,9 +191,7 @@
 }
 
 func (s *debugLogSuite) dialWebsocket(c *gc.C, queryParams url.Values) (*websocket.Conn, error) {
-	header := http.Header{
-		"Authorization": {"Basic " + base64.StdEncoding.EncodeToString([]byte(s.userTag+":"+s.password))},
-	}
+	header := utils.BasicAuthHeader(s.userTag, s.password)
 	return s.dialWebsocketInternal(c, queryParams, header)
 }
 

=== modified file 'utils/http.go'
--- utils/http.go	2014-03-24 03:24:00 +0000
+++ utils/http.go	2014-04-10 09:38:03 +0000
@@ -5,6 +5,7 @@
 
 import (
 	"crypto/tls"
+	"encoding/base64"
 	"net/http"
 	"sync"
 )
@@ -83,3 +84,18 @@
 	registerFileProtocol(transport)
 	return transport
 }
+
+// BasicAuthHeader creates a header that contains just the "Authorization"
+// entry.  The implementation was originally taked from net/http but this is
+// needed externally from the http request object in order to use this with
+// our websockets. See 2 (end of page 4) http://www.ietf.org/rfc/rfc2617.txt
+// "To receive authorization, the client sends the userid and password,
+// separated by a single colon (":") character, within a base64 encoded string
+// in the credentials."
+func BasicAuthHeader(username, password string) http.Header {
+	auth := username + ":" + password
+	encoded := "Basic " + base64.StdEncoding.EncodeToString([]byte(auth))
+	return http.Header{
+		"Authorization": {encoded},
+	}
+}

=== modified file 'utils/http_test.go'
--- utils/http_test.go	2014-03-24 00:23:19 +0000
+++ utils/http_test.go	2014-04-10 09:38:03 +0000
@@ -4,10 +4,12 @@
 package utils_test
 
 import (
+	"encoding/base64"
 	"fmt"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 
 	gc "launchpad.net/gocheck"
 
@@ -79,3 +81,16 @@
 	client2 := utils.GetNonValidatingHTTPClient()
 	c.Check(client1, gc.Equals, client2)
 }
+
+func (s *httpSuite) TestBasicAuthHeader(c *gc.C) {
+	header := utils.BasicAuthHeader("eric", "sekrit")
+	c.Assert(len(header), gc.Equals, 1)
+	auth := header.Get("Authorization")
+	fields := strings.Fields(auth)
+	c.Assert(len(fields), gc.Equals, 2)
+	basic, encoded := fields[0], fields[1]
+	c.Assert(basic, gc.Equals, "Basic")
+	decoded, err := base64.StdEncoding.DecodeString(encoded)
+	c.Assert(err, gc.IsNil)
+	c.Assert(string(decoded), gc.Equals, "eric:sekrit")
+}