From df981d8250a8a1b2c82b3a6355f31b4acf924691 Mon Sep 17 00:00:00 2001 From: Wes Date: Fri, 13 Sep 2024 19:06:33 -0700 Subject: [PATCH] fix: address PR comments --- backend/controller/controller.go | 8 +- .../internal/sql/deployment_queries.sql.go | 89 +++++++++++++++++++ .../controller/dal/internal/sql/queries.sql | 40 --------- .../dal/internal/sql/queries.sql.go | 76 ---------------- .../dal/internal/sql/deployment_queries.sql | 39 ++++++++ .../internal/sql/deployment_queries.sql.go | 88 ++++++++++++++++++ .../timeline/dal/internal/sql/querier.go | 2 + backend/controller/timeline/timeline.go | 21 +++-- sqlc.yaml | 3 + 9 files changed, 237 insertions(+), 129 deletions(-) create mode 100644 backend/controller/dal/internal/sql/deployment_queries.sql.go create mode 100644 backend/controller/timeline/dal/internal/sql/deployment_queries.sql create mode 100644 backend/controller/timeline/dal/internal/sql/deployment_queries.sql.go diff --git a/backend/controller/controller.go b/backend/controller/controller.go index 002eb68455..599945e138 100644 --- a/backend/controller/controller.go +++ b/backend/controller/controller.go @@ -1073,12 +1073,13 @@ func (s *Service) callWithRequest( response, err := client.verb.Call(ctx, req) var resp *connect.Response[ftlv1.CallResponse] - var maybeResponse optional.Option[*ftlv1.CallResponse] + var callResponse either.Either[*ftlv1.CallResponse, error] if err == nil { resp = connect.NewResponse(response.Msg) - maybeResponse = optional.Some(resp.Msg) + callResponse = either.LeftOf[error](resp.Msg) observability.Calls.Request(ctx, req.Msg.Verb, start, optional.None[string]()) } else { + callResponse = either.RightOf[*ftlv1.CallResponse](err) observability.Calls.Request(ctx, req.Msg.Verb, start, optional.Some("verb call failed")) } s.timeline.RecordCall(ctx, &timeline.Call{ @@ -1088,9 +1089,8 @@ func (s *Service) callWithRequest( StartTime: start, DestVerb: verbRef, Callers: callers, - CallError: optional.Nil(err), Request: req.Msg, - Response: maybeResponse, + Response: callResponse, }) return resp, err } diff --git a/backend/controller/dal/internal/sql/deployment_queries.sql.go b/backend/controller/dal/internal/sql/deployment_queries.sql.go new file mode 100644 index 0000000000..69e9c25b9b --- /dev/null +++ b/backend/controller/dal/internal/sql/deployment_queries.sql.go @@ -0,0 +1,89 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.27.0 +// source: deployment_queries.sql + +package sql + +import ( + "context" + + "github.com/TBD54566975/ftl/internal/encryption" + "github.com/TBD54566975/ftl/internal/model" +) + +const insertTimelineDeploymentCreatedEvent = `-- name: InsertTimelineDeploymentCreatedEvent :exec +INSERT INTO timeline ( + deployment_id, + type, + custom_key_1, + custom_key_2, + payload +) +VALUES ( + ( + SELECT id + FROM deployments + WHERE deployments.key = $1::deployment_key + ), + 'deployment_created', + $2::TEXT, + $3::TEXT, + $4 +) +` + +type InsertTimelineDeploymentCreatedEventParams struct { + DeploymentKey model.DeploymentKey + Language string + ModuleName string + Payload encryption.EncryptedTimelineColumn +} + +func (q *Queries) InsertTimelineDeploymentCreatedEvent(ctx context.Context, arg InsertTimelineDeploymentCreatedEventParams) error { + _, err := q.db.ExecContext(ctx, insertTimelineDeploymentCreatedEvent, + arg.DeploymentKey, + arg.Language, + arg.ModuleName, + arg.Payload, + ) + return err +} + +const insertTimelineDeploymentUpdatedEvent = `-- name: InsertTimelineDeploymentUpdatedEvent :exec +INSERT INTO timeline ( + deployment_id, + type, + custom_key_1, + custom_key_2, + payload +) +VALUES ( + ( + SELECT id + FROM deployments + WHERE deployments.key = $1::deployment_key + ), + 'deployment_updated', + $2::TEXT, + $3::TEXT, + $4 +) +` + +type InsertTimelineDeploymentUpdatedEventParams struct { + DeploymentKey model.DeploymentKey + Language string + ModuleName string + Payload encryption.EncryptedTimelineColumn +} + +func (q *Queries) InsertTimelineDeploymentUpdatedEvent(ctx context.Context, arg InsertTimelineDeploymentUpdatedEventParams) error { + _, err := q.db.ExecContext(ctx, insertTimelineDeploymentUpdatedEvent, + arg.DeploymentKey, + arg.Language, + arg.ModuleName, + arg.Payload, + ) + return err +} diff --git a/backend/controller/dal/internal/sql/queries.sql b/backend/controller/dal/internal/sql/queries.sql index 38c8f34e73..3adc68db2d 100644 --- a/backend/controller/dal/internal/sql/queries.sql +++ b/backend/controller/dal/internal/sql/queries.sql @@ -205,46 +205,6 @@ FROM runners r WHERE state = 'assigned' AND d.key = sqlc.arg('key')::deployment_key; --- name: InsertTimelineDeploymentCreatedEvent :exec -INSERT INTO timeline ( - deployment_id, - type, - custom_key_1, - custom_key_2, - payload -) -VALUES ( - ( - SELECT id - FROM deployments - WHERE deployments.key = sqlc.arg('deployment_key')::deployment_key - ), - 'deployment_created', - sqlc.arg('language')::TEXT, - sqlc.arg('module_name')::TEXT, - sqlc.arg('payload') -); - --- name: InsertTimelineDeploymentUpdatedEvent :exec -INSERT INTO timeline ( - deployment_id, - type, - custom_key_1, - custom_key_2, - payload -) -VALUES ( - ( - SELECT id - FROM deployments - WHERE deployments.key = sqlc.arg('deployment_key')::deployment_key - ), - 'deployment_updated', - sqlc.arg('language')::TEXT, - sqlc.arg('module_name')::TEXT, - sqlc.arg('payload') -); - -- name: CreateRequest :exec INSERT INTO requests (origin, "key", source_addr) VALUES ($1, $2, $3); diff --git a/backend/controller/dal/internal/sql/queries.sql.go b/backend/controller/dal/internal/sql/queries.sql.go index 93906c4574..cc1e476f93 100644 --- a/backend/controller/dal/internal/sql/queries.sql.go +++ b/backend/controller/dal/internal/sql/queries.sql.go @@ -1757,82 +1757,6 @@ func (q *Queries) InsertSubscriber(ctx context.Context, arg InsertSubscriberPara return err } -const insertTimelineDeploymentCreatedEvent = `-- name: InsertTimelineDeploymentCreatedEvent :exec -INSERT INTO timeline ( - deployment_id, - type, - custom_key_1, - custom_key_2, - payload -) -VALUES ( - ( - SELECT id - FROM deployments - WHERE deployments.key = $1::deployment_key - ), - 'deployment_created', - $2::TEXT, - $3::TEXT, - $4 -) -` - -type InsertTimelineDeploymentCreatedEventParams struct { - DeploymentKey model.DeploymentKey - Language string - ModuleName string - Payload encryption.EncryptedTimelineColumn -} - -func (q *Queries) InsertTimelineDeploymentCreatedEvent(ctx context.Context, arg InsertTimelineDeploymentCreatedEventParams) error { - _, err := q.db.ExecContext(ctx, insertTimelineDeploymentCreatedEvent, - arg.DeploymentKey, - arg.Language, - arg.ModuleName, - arg.Payload, - ) - return err -} - -const insertTimelineDeploymentUpdatedEvent = `-- name: InsertTimelineDeploymentUpdatedEvent :exec -INSERT INTO timeline ( - deployment_id, - type, - custom_key_1, - custom_key_2, - payload -) -VALUES ( - ( - SELECT id - FROM deployments - WHERE deployments.key = $1::deployment_key - ), - 'deployment_updated', - $2::TEXT, - $3::TEXT, - $4 -) -` - -type InsertTimelineDeploymentUpdatedEventParams struct { - DeploymentKey model.DeploymentKey - Language string - ModuleName string - Payload encryption.EncryptedTimelineColumn -} - -func (q *Queries) InsertTimelineDeploymentUpdatedEvent(ctx context.Context, arg InsertTimelineDeploymentUpdatedEventParams) error { - _, err := q.db.ExecContext(ctx, insertTimelineDeploymentUpdatedEvent, - arg.DeploymentKey, - arg.Language, - arg.ModuleName, - arg.Payload, - ) - return err -} - const isCronJobPending = `-- name: IsCronJobPending :one SELECT EXISTS ( SELECT 1 diff --git a/backend/controller/timeline/dal/internal/sql/deployment_queries.sql b/backend/controller/timeline/dal/internal/sql/deployment_queries.sql new file mode 100644 index 0000000000..268a22047e --- /dev/null +++ b/backend/controller/timeline/dal/internal/sql/deployment_queries.sql @@ -0,0 +1,39 @@ +-- name: InsertTimelineDeploymentCreatedEvent :exec +INSERT INTO timeline ( + deployment_id, + type, + custom_key_1, + custom_key_2, + payload +) +VALUES ( + ( + SELECT id + FROM deployments + WHERE deployments.key = sqlc.arg('deployment_key')::deployment_key + ), + 'deployment_created', + sqlc.arg('language')::TEXT, + sqlc.arg('module_name')::TEXT, + sqlc.arg('payload') +); + +-- name: InsertTimelineDeploymentUpdatedEvent :exec +INSERT INTO timeline ( + deployment_id, + type, + custom_key_1, + custom_key_2, + payload +) +VALUES ( + ( + SELECT id + FROM deployments + WHERE deployments.key = sqlc.arg('deployment_key')::deployment_key + ), + 'deployment_updated', + sqlc.arg('language')::TEXT, + sqlc.arg('module_name')::TEXT, + sqlc.arg('payload') +); diff --git a/backend/controller/timeline/dal/internal/sql/deployment_queries.sql.go b/backend/controller/timeline/dal/internal/sql/deployment_queries.sql.go new file mode 100644 index 0000000000..12356896c0 --- /dev/null +++ b/backend/controller/timeline/dal/internal/sql/deployment_queries.sql.go @@ -0,0 +1,88 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.27.0 +// source: deployment_queries.sql + +package sql + +import ( + "context" + + "github.com/TBD54566975/ftl/internal/encryption" +) + +const insertTimelineDeploymentCreatedEvent = `-- name: InsertTimelineDeploymentCreatedEvent :exec +INSERT INTO timeline ( + deployment_id, + type, + custom_key_1, + custom_key_2, + payload +) +VALUES ( + ( + SELECT id + FROM deployments + WHERE deployments.key = $1::deployment_key + ), + 'deployment_created', + $2::TEXT, + $3::TEXT, + $4 +) +` + +type InsertTimelineDeploymentCreatedEventParams struct { + DeploymentKey interface{} + Language string + ModuleName string + Payload encryption.EncryptedTimelineColumn +} + +func (q *Queries) InsertTimelineDeploymentCreatedEvent(ctx context.Context, arg InsertTimelineDeploymentCreatedEventParams) error { + _, err := q.db.ExecContext(ctx, insertTimelineDeploymentCreatedEvent, + arg.DeploymentKey, + arg.Language, + arg.ModuleName, + arg.Payload, + ) + return err +} + +const insertTimelineDeploymentUpdatedEvent = `-- name: InsertTimelineDeploymentUpdatedEvent :exec +INSERT INTO timeline ( + deployment_id, + type, + custom_key_1, + custom_key_2, + payload +) +VALUES ( + ( + SELECT id + FROM deployments + WHERE deployments.key = $1::deployment_key + ), + 'deployment_updated', + $2::TEXT, + $3::TEXT, + $4 +) +` + +type InsertTimelineDeploymentUpdatedEventParams struct { + DeploymentKey interface{} + Language string + ModuleName string + Payload encryption.EncryptedTimelineColumn +} + +func (q *Queries) InsertTimelineDeploymentUpdatedEvent(ctx context.Context, arg InsertTimelineDeploymentUpdatedEventParams) error { + _, err := q.db.ExecContext(ctx, insertTimelineDeploymentUpdatedEvent, + arg.DeploymentKey, + arg.Language, + arg.ModuleName, + arg.Payload, + ) + return err +} diff --git a/backend/controller/timeline/dal/internal/sql/querier.go b/backend/controller/timeline/dal/internal/sql/querier.go index afa6fa3142..9d0bf33988 100644 --- a/backend/controller/timeline/dal/internal/sql/querier.go +++ b/backend/controller/timeline/dal/internal/sql/querier.go @@ -15,6 +15,8 @@ type Querier interface { // This is a dummy query to ensure that the Timeline model is generated. DummyQueryTimeline(ctx context.Context, id int64) (Timeline, error) InsertTimelineCallEvent(ctx context.Context, arg InsertTimelineCallEventParams) error + InsertTimelineDeploymentCreatedEvent(ctx context.Context, arg InsertTimelineDeploymentCreatedEventParams) error + InsertTimelineDeploymentUpdatedEvent(ctx context.Context, arg InsertTimelineDeploymentUpdatedEventParams) error InsertTimelineLogEvent(ctx context.Context, arg InsertTimelineLogEventParams) error } diff --git a/backend/controller/timeline/timeline.go b/backend/controller/timeline/timeline.go index 360399d2ca..8205453c34 100644 --- a/backend/controller/timeline/timeline.go +++ b/backend/controller/timeline/timeline.go @@ -5,6 +5,7 @@ import ( "fmt" "time" + "github.com/alecthomas/types/either" "github.com/alecthomas/types/optional" "github.com/TBD54566975/ftl/backend/controller/encryption" @@ -17,11 +18,11 @@ import ( ) type Service struct { - dal dal.DAL + dal *dal.DAL } func New(ctx context.Context, conn libdal.Connection, encryption *encryption.Service) *Service { - return &Service{dal: *dal.New(conn, encryption)} + return &Service{dal: dal.New(conn, encryption)} } type Log struct { @@ -54,8 +55,7 @@ type Call struct { DestVerb *schema.Ref Callers []*schema.Ref Request *ftlv1.CallRequest - Response optional.Option[*ftlv1.CallResponse] - CallError optional.Option[error] + Response either.Either[*ftlv1.CallResponse, error] } func (s *Service) RecordCall(ctx context.Context, call *Call) { @@ -69,14 +69,17 @@ func (s *Service) RecordCall(ctx context.Context, call *Call) { var stack optional.Option[string] var responseBody []byte - if callError, ok := call.CallError.Get(); ok { - errorStr = optional.Some(callError.Error()) - } else if response, ok := call.Response.Get(); ok { - responseBody = response.GetBody() - if callError := response.GetError(); callError != nil { + switch response := call.Response.(type) { + case either.Left[*ftlv1.CallResponse, error]: + resp := response.Get() + responseBody = resp.GetBody() + if callError := resp.GetError(); callError != nil { errorStr = optional.Some(callError.Message) stack = optional.Ptr(callError.Stack) } + case either.Right[*ftlv1.CallResponse, error]: + callError := response.Get() + errorStr = optional.Some(callError.Error()) } err := s.dal.InsertCallEvent(ctx, &dal.CallEvent{ diff --git a/sqlc.yaml b/sqlc.yaml index 56c5fc86c6..7638aa1f89 100644 --- a/sqlc.yaml +++ b/sqlc.yaml @@ -7,6 +7,8 @@ sql: - backend/controller/dal/internal/sql/async_queries.sql # FIXME: Until we fully decouple cron from the controller, we need to include the cron queries here - backend/controller/cronjobs/dal/internal/sql/queries.sql + # Some of the timeline entries happen within a controller transaction, so we need to include them here + - backend/controller/timeline/dal/internal/sql/deployment_queries.sql schema: "backend/controller/sql/schema" database: uri: postgres://localhost:15432/ftl?sslmode=disable&user=postgres&password=secret @@ -181,6 +183,7 @@ sql: - <<: *daldir queries: - backend/controller/timeline/dal/internal/sql/queries.sql + - backend/controller/timeline/dal/internal/sql/deployment_queries.sql gen: go: <<: *gengo