Skip to content

Commit

Permalink
Addressing review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
alesstimec committed Jun 13, 2024
1 parent f391186 commit 45c64d8
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 19 deletions.
1 change: 0 additions & 1 deletion internal/jujuapi/websocket.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ func controllerConnectionFunc(s modelProxyServer, jwtGenerator *jimm.JWTGenerato
return jimmRPC.WebsocketConnectionWithMetadata{
Conn: controllerConn,
ControllerUUID: m.Controller.UUID,
ModelUUID: m.UUID.String,
ModelName: fullModelName,
}, nil
}
Expand Down
4 changes: 2 additions & 2 deletions internal/jujuclient/dial.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,9 @@ func (c *Connection) redial(ctx context.Context, requiredPermissions map[string]
// the server and waits for the response to be returned or the context to
// be canceled.
func (c *Connection) Call(ctx context.Context, facade string, version int, id, method string, args, resp interface{}) (err error) {
labels := []string{facade, method, "", c.mt.Id()}
labels := []string{facade, method, ""}
if c.ctl != nil {
labels = []string{facade, method, c.ctl.UUID, c.mt.Id()}
labels = []string{facade, method, c.ctl.UUID}
}
durationObserver := servermon.DurationObserver(servermon.JujuCallDurationHistogram, labels...)
defer durationObserver()
Expand Down
21 changes: 7 additions & 14 deletions internal/rpc/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ type WebsocketConnection interface {
type WebsocketConnectionWithMetadata struct {
Conn WebsocketConnection
ControllerUUID string
ModelUUID string
ModelName string
}

Expand Down Expand Up @@ -170,8 +169,7 @@ func (c *writeLockConn) sendMessage(responseObject any, request *message) {
}

type inflightMsgs struct {
controlerUUID string
modelUUID string
controllerUUID string

mu sync.Mutex
loginMessage *message
Expand Down Expand Up @@ -205,8 +203,7 @@ func (msgs *inflightMsgs) removeMessage(msg *message) {
servermon.JujuCallDurationHistogram.WithLabelValues(
msg.Type,
msg.Request,
msgs.controlerUUID,
msgs.modelUUID,
msgs.controllerUUID,
).Observe(time.Since(msg.start).Seconds())

msgs.mu.Lock()
Expand Down Expand Up @@ -248,7 +245,7 @@ func (p *modelProxy) sendError(socket *writeLockConn, req *message, err error) {
socket.writeJson(msg)
}
// An error message is a response back to the client.
servermon.JujuCallErrorCount.WithLabelValues(req.Type, req.Request, p.msgs.controlerUUID, p.msgs.modelUUID)
servermon.JujuCallErrorCount.WithLabelValues(req.Type, req.Request, p.msgs.controllerUUID)
p.auditLogMessage(msg, true)
}

Expand Down Expand Up @@ -316,15 +313,13 @@ func (p *clientProxy) start(ctx context.Context) error {
p.dst.conn.Close()
}
}()

for {
zapctx.Debug(ctx, "Reading on client connection")
msg := new(message)
if err := p.src.readJson(&msg); err != nil {
// Error reading on the socket implies it is closed, simply return.
return err
}

zapctx.Debug(ctx, "Read message from client", zap.Any("message", msg))
err := p.makeControllerConnection(ctx)
if err != nil {
Expand All @@ -340,7 +335,7 @@ func (p *clientProxy) start(ctx context.Context) error {
toClient, toController, err := p.handleAdminFacade(ctx, msg)
if err != nil {
p.sendError(p.src, msg, err)
return nil
continue
}
if toClient != nil {
p.src.sendMessage(nil, toClient)
Expand All @@ -353,16 +348,15 @@ func (p *clientProxy) start(ctx context.Context) error {
zapctx.Error(ctx, "Invalid request ID 0")
err := errors.E(op, "Invalid request ID 0")
p.sendError(p.src, msg, err)
return nil
continue
}
p.msgs.addMessage(msg)
zapctx.Debug(ctx, "Writing to controller")

if err := p.dst.writeJson(msg); err != nil {
zapctx.Error(ctx, "clientProxy error writing to dst", zap.Error(err))
p.sendError(p.src, msg, err)
p.msgs.removeMessage(msg)
return nil
continue
}
}
}
Expand All @@ -386,8 +380,7 @@ func (p *clientProxy) makeControllerConnection(ctx context.Context) error {
return err
}

p.msgs.controlerUUID = connWithMetadata.ControllerUUID
p.msgs.modelUUID = connWithMetadata.ModelUUID
p.msgs.controllerUUID = connWithMetadata.ControllerUUID

p.modelName = connWithMetadata.ModelName
p.dst = &writeLockConn{conn: connWithMetadata.Conn}
Expand Down
1 change: 0 additions & 1 deletion internal/rpc/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ func TestProxySocketsAdminFacade(t *testing.T) {
return rpc.WebsocketConnectionWithMetadata{
Conn: controllerWebsocket,
ModelName: "test model",
ModelUUID: uuid.NewString(),
ControllerUUID: uuid.NewString(),
}, nil
},
Expand Down
2 changes: 1 addition & 1 deletion internal/servermon/monitoring.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ var (
Name: "call_duration_seconds",
Help: "Histogram of juju call time in seconds",
Buckets: []float64{.005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5, 10},
}, []string{"facade", "method", "controller", "model"})
}, []string{"facade", "method", "controller"})
JujuCallErrorCount = promauto.NewCounterVec(prometheus.CounterOpts{
Namespace: "jimm",
Subsystem: "juju",
Expand Down

0 comments on commit 45c64d8

Please sign in to comment.