From 8bdf24ba39367b1f5803ec288a8407bd444112e1 Mon Sep 17 00:00:00 2001 From: Joshua Hawxwell Date: Fri, 25 Oct 2024 11:21:00 +0100 Subject: [PATCH 1/2] Show certificate provider and attorney the contextual LPA --- cypress/e2e/attorney/read-the-lpa.cy.js | 22 +++++--- .../certificate-provider/read-the-lpa.cy.js | 22 ++++---- internal/page/fixtures/attorney.go | 5 +- .../page/fixtures/certificate_provider.go | 4 +- web/template/attorney/read_the_lpa.gohtml | 12 +---- .../certificateprovider/read_the_lpa.gohtml | 52 ++++++++----------- 6 files changed, 54 insertions(+), 63 deletions(-) diff --git a/cypress/e2e/attorney/read-the-lpa.cy.js b/cypress/e2e/attorney/read-the-lpa.cy.js index fc46f8f8bd..9e26330608 100644 --- a/cypress/e2e/attorney/read-the-lpa.cy.js +++ b/cypress/e2e/attorney/read-the-lpa.cy.js @@ -1,13 +1,19 @@ describe('Read the LPA', () => { - it('displays the LPA details with actor specific content', () => { - cy.visit('/fixtures/attorney?redirect=/read-the-lpa'); + it('displays the LPA details with actor specific content', () => { + cy.visit('/fixtures/attorney?redirect=/read-the-lpa'); - cy.contains('dt', "When attorneys can use the LPA") - cy.contains('dt', "Attorney names") - cy.contains('dt', "Replacement attorney names") + cy.contains('Donor: Sam Smith'); + cy.contains('Certificate provider: Charlie Cooper'); + cy.contains('Attorney: Jessie Jones'); + cy.contains('Trust corporation attorney: First Choice Trust Corporation Ltd.'); + cy.contains('Replacement attorney: Blake Buckley'); + cy.contains('Replacement trust corporation attorney: Second Choice Trust Corporation Ltd.'); + cy.contains('Signed by Sam Smith on: 2 January 2023'); + cy.contains('Witnessed by Charlie Cooper on: 2 January 2023'); + cy.contains('Signed by Charlie Cooper on: 2 January 2023'); - cy.contains('Continue').click(); + cy.contains('Continue').click(); - cy.url().should('contain', '/task-list'); - }); + cy.url().should('contain', '/task-list'); + }); }); diff --git a/cypress/e2e/certificate-provider/read-the-lpa.cy.js b/cypress/e2e/certificate-provider/read-the-lpa.cy.js index e97594dfe3..66bfbe0b04 100644 --- a/cypress/e2e/certificate-provider/read-the-lpa.cy.js +++ b/cypress/e2e/certificate-provider/read-the-lpa.cy.js @@ -7,11 +7,14 @@ describe('Read the LPA', () => { it('displays the LPA details and goes to provide certificate', () => { cy.checkA11yApp(); - cy.contains('dt', "When attorneys can use the LPA") - cy.contains('dt', "Their attorneys") - cy.contains('dt', "Their replacement attorneys") + cy.contains('Donor: Sam Smith'); + cy.contains('Certificate provider: Charlie Cooper'); + cy.contains('Attorney: Jessie Jones'); + cy.contains('Attorney: Robin Redcar'); + cy.contains('Signed by Sam Smith on: 2 January 2023'); + cy.contains('Witnessed by Charlie Cooper on: 2 January 2023'); - cy.contains('Continue').click(); + cy.contains('button', 'Continue').click(); cy.url().should('contain', '/what-happens-next'); }); }); @@ -24,12 +27,13 @@ describe('Read the LPA', () => { it('displays the LPA details and goes to task list', () => { cy.checkA11yApp(); - cy.contains('dt', "When attorneys can use the LPA") - cy.contains('dt', "Their attorneys") - cy.contains('dt', "Their replacement attorneys") + cy.contains('Donor: Sam Smith'); + cy.contains('Certificate provider: Charlie Cooper'); + cy.contains('Attorney: Jessie Jones'); + cy.contains('Attorney: Robin Redcar'); - cy.get('button').should('not.contain', 'Continue'); - cy.contains('Return to task list').click(); + cy.contains('button', 'Continue').should('not.exist'); + cy.contains('a', 'Return to task list').click(); cy.url().should('contain', '/task-list'); }); }); diff --git a/internal/page/fixtures/attorney.go b/internal/page/fixtures/attorney.go index 83b339b840..7f50bec03c 100644 --- a/internal/page/fixtures/attorney.go +++ b/internal/page/fixtures/attorney.go @@ -147,8 +147,8 @@ func Attorney( attorneyCtx = appcontext.ContextWithSession(r.Context(), &appcontext.Session{SessionID: attorneySessionID, LpaID: donorDetails.LpaID}) ) - donorDetails.SignedAt = time.Now() - donorDetails.WitnessedByCertificateProviderAt = time.Now() + donorDetails.SignedAt = time.Date(2023, time.January, 2, 3, 4, 5, 6, time.UTC) + donorDetails.WitnessedByCertificateProviderAt = time.Date(2023, time.January, 2, 3, 4, 5, 6, time.UTC) donorDetails.Donor = makeDonor(testEmail) if lpaType == "personal-welfare" && !isTrustCorporation { @@ -221,6 +221,7 @@ func Attorney( } certificateProvider.ContactLanguagePreference = localize.En + certificateProvider.SignedAt = time.Date(2023, time.January, 2, 3, 4, 5, 6, time.UTC) attorney, err := createAttorney( attorneyCtx, diff --git a/internal/page/fixtures/certificate_provider.go b/internal/page/fixtures/certificate_provider.go index 1b45695eca..4b392d00a1 100644 --- a/internal/page/fixtures/certificate_provider.go +++ b/internal/page/fixtures/certificate_provider.go @@ -200,8 +200,8 @@ func CertificateProvider( if progress >= slices.Index(progressValues, "signedByDonor") { donorDetails.Tasks.ConfirmYourIdentityAndSign = task.IdentityStateCompleted - donorDetails.WitnessedByCertificateProviderAt = time.Now() - donorDetails.SignedAt = time.Now() + donorDetails.SignedAt = time.Date(2023, time.January, 2, 3, 4, 5, 6, time.UTC) + donorDetails.WitnessedByCertificateProviderAt = time.Date(2023, time.January, 2, 3, 4, 5, 6, time.UTC) } if progress >= slices.Index(progressValues, "confirmYourDetails") { diff --git a/web/template/attorney/read_the_lpa.gohtml b/web/template/attorney/read_the_lpa.gohtml index 92db007c98..1026fb7f14 100644 --- a/web/template/attorney/read_the_lpa.gohtml +++ b/web/template/attorney/read_the_lpa.gohtml @@ -9,17 +9,7 @@ {{ template "warning" (content .App "youMustReadLpaCarefully") }} -

- {{ tr .App "lpaDecisions" }} -

- - {{ template "lpa-decisions" (lpaDecisions .App .Lpa false) }} - -

- {{ tr .App "peopleNamedOnTheLpa" }} -

- - {{ template "people-named-on-lpa" (lpaDecisions .App .Lpa false) }} + {{ template "contextual-lpa" . }}
{{ template "continue-button" . }} diff --git a/web/template/certificateprovider/read_the_lpa.gohtml b/web/template/certificateprovider/read_the_lpa.gohtml index bc9ad121da..7893c67a7a 100644 --- a/web/template/certificateprovider/read_the_lpa.gohtml +++ b/web/template/certificateprovider/read_the_lpa.gohtml @@ -3,37 +3,27 @@ {{ define "pageTitle" }}{{ trFormat .App "readDonorNameLpa" "DonorFullName" .Lpa.Donor.FullName }}{{ end }} {{ define "main" }} -
-
-

{{ trFormat .App "readDonorNameLpa" "DonorFullName" .Lpa.Donor.FullName }}

- - {{ if and .App.IsCertificateProvider .Lpa.SignedAt.IsZero }} - {{ template "warning" (content .App "youShouldReadLpaCarefully") }} - {{ else }} - {{ template "warning" (content .App "youMustReadLpaCarefully") }} - {{ end }} - -

- {{ tr .App "lpaDecisions" }} -

- - {{ template "lpa-decisions" (lpaDecisions .App .Lpa false) }} - -

- {{ tr .App "peopleNamedOnTheLpa" }} -

- - {{ template "people-named-on-lpa" (lpaDecisions .App .Lpa false) }} - - -
- {{ if not .Lpa.SignedAt.IsZero }} - {{ template "continue-button" . }} - {{ end }} - {{ tr .App "returnToTaskList" }} +
+
+

{{ trFormat .App "readDonorNameLpa" "DonorFullName" .Lpa.Donor.FullName }}

+ + {{ if and .App.IsCertificateProvider .Lpa.SignedAt.IsZero }} + {{ template "warning" (content .App "youShouldReadLpaCarefully") }} + {{ else }} + {{ template "warning" (content .App "youMustReadLpaCarefully") }} + {{ end }} + + {{ template "contextual-lpa" . }} + + + {{ if .Lpa.SignedAt.IsZero }} + {{ template "button" (button .App "returnToTaskList" "link" (global.Paths.CertificateProvider.TaskList.Format .App.LpaID)) }} + {{ else }} + {{ template "buttons" (button .App "continue") }} + {{ end }} + + {{ template "csrf-field" . }} +
- {{ template "csrf-field" . }} -
-
{{ end }} From 615fbab30144e08045a541d67afd12effc0ca213 Mon Sep 17 00:00:00 2001 From: Joshua Hawxwell Date: Fri, 25 Oct 2024 11:54:01 +0100 Subject: [PATCH 2/2] Use existing span when adding xray context to events --- cmd/mlpa/main.go | 1 + internal/event/client.go | 8 ---- internal/telemetry/middleware.go | 32 +++++++++++++ internal/telemetry/slog.go | 48 +++++++++++++++++++ .../{telemetry_test.go => slog_test.go} | 0 internal/telemetry/telemetry.go | 41 ---------------- 6 files changed, 81 insertions(+), 49 deletions(-) create mode 100644 internal/telemetry/middleware.go create mode 100644 internal/telemetry/slog.go rename internal/telemetry/{telemetry_test.go => slog_test.go} (100%) diff --git a/cmd/mlpa/main.go b/cmd/mlpa/main.go index 733c66a944..d7e441b6ff 100644 --- a/cmd/mlpa/main.go +++ b/cmd/mlpa/main.go @@ -205,6 +205,7 @@ func run(ctx context.Context, logger *slog.Logger) error { } otelaws.AppendMiddlewares(&cfg.APIOptions) + telemetry.AppendMiddlewares(&cfg.APIOptions) lpasDynamoClient, err := dynamo.NewClient(cfg, dynamoTableLpas) if err != nil { diff --git a/internal/event/client.go b/internal/event/client.go index 2b2edeb03f..5bf5844b2d 100644 --- a/internal/event/client.go +++ b/internal/event/client.go @@ -9,8 +9,6 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/eventbridge" "github.com/aws/aws-sdk-go-v2/service/eventbridge/types" - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/trace" ) const source = "opg.poas.makeregister" @@ -90,12 +88,6 @@ func send[T any](ctx context.Context, c *Client, detail any) error { return errors.New("event send of unknown type") } - tracer := otel.GetTracerProvider().Tracer("mlpab") - ctx, span := tracer.Start(ctx, detailType, - trace.WithSpanKind(trace.SpanKindInternal), - ) - defer span.End() - v, err := json.Marshal(detail) if err != nil { return err diff --git a/internal/telemetry/middleware.go b/internal/telemetry/middleware.go new file mode 100644 index 0000000000..a86f035c1e --- /dev/null +++ b/internal/telemetry/middleware.go @@ -0,0 +1,32 @@ +package telemetry + +import ( + "context" + + "github.com/aws/aws-sdk-go-v2/service/eventbridge" + "github.com/aws/smithy-go/middleware" + "go.opentelemetry.io/otel/attribute" + semconv "go.opentelemetry.io/otel/semconv/v1.17.0" + "go.opentelemetry.io/otel/trace" +) + +func AppendMiddlewares(list *[]func(*middleware.Stack) error) { + *list = append(*list, func(stack *middleware.Stack) error { + return stack.Initialize.Add(middleware.InitializeMiddlewareFunc("appInitializeMiddlewareAfter", + func(ctx context.Context, in middleware.InitializeInput, next middleware.InitializeHandler) (middleware.InitializeOutput, middleware.Metadata, error) { + span := trace.SpanFromContext(ctx) + + switch v := in.Parameters.(type) { + case *eventbridge.PutEventsInput: + if len(v.Entries) == 1 { + span.SetAttributes(attribute.String("aws.operation", "PutEvents "+*v.Entries[0].DetailType)) + span.SetAttributes(semconv.CloudeventsEventSource(*v.Entries[0].Source)) + span.SetAttributes(semconv.CloudeventsEventType(*v.Entries[0].DetailType)) + } + } + + return next.HandleInitialize(ctx, in) + }, + ), middleware.After) + }) +} diff --git a/internal/telemetry/slog.go b/internal/telemetry/slog.go new file mode 100644 index 0000000000..cb55f06e37 --- /dev/null +++ b/internal/telemetry/slog.go @@ -0,0 +1,48 @@ +package telemetry + +import ( + "context" + "log/slog" + + "github.com/ministryofjustice/opg-modernising-lpa/internal/appcontext" + "go.opentelemetry.io/otel/trace" +) + +type SlogHandler struct { + handler slog.Handler +} + +func NewSlogHandler(h slog.Handler) slog.Handler { + return &SlogHandler{handler: h} +} + +func (h *SlogHandler) Enabled(ctx context.Context, level slog.Level) bool { + return h.handler.Enabled(ctx, level) +} + +func (h *SlogHandler) Handle(ctx context.Context, record slog.Record) error { + spanCtx := trace.SpanContextFromContext(ctx) + if spanCtx.HasTraceID() { + traceID := spanCtx.TraceID() + record.AddAttrs(slog.String("trace_id", traceID.String())) + } + + session, err := appcontext.SessionFromContext(ctx) + if err == nil { + record.AddAttrs(slog.String("session_id", session.SessionID)) + + if session.OrganisationID != "" { + record.AddAttrs(slog.String("organisation_id", session.OrganisationID)) + } + } + + return h.handler.Handle(ctx, record) +} + +func (h *SlogHandler) WithAttrs(attrs []slog.Attr) slog.Handler { + return NewSlogHandler(h.handler.WithAttrs(attrs)) +} + +func (h *SlogHandler) WithGroup(name string) slog.Handler { + return NewSlogHandler(h.handler.WithGroup(name)) +} diff --git a/internal/telemetry/telemetry_test.go b/internal/telemetry/slog_test.go similarity index 100% rename from internal/telemetry/telemetry_test.go rename to internal/telemetry/slog_test.go diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go index fb1749e5ec..12befc3d98 100644 --- a/internal/telemetry/telemetry.go +++ b/internal/telemetry/telemetry.go @@ -5,12 +5,10 @@ package telemetry import ( "context" "fmt" - "log/slog" "net/http" "strings" "github.com/felixge/httpsnoop" - "github.com/ministryofjustice/opg-modernising-lpa/internal/appcontext" "go.opentelemetry.io/contrib/propagators/aws/xray" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -77,42 +75,3 @@ func WrapHandler(handler http.Handler) http.HandlerFunc { span.SetAttributes(semconv.HTTPResponseContentLengthKey.Int64(m.Written)) } } - -type SlogHandler struct { - handler slog.Handler -} - -func NewSlogHandler(h slog.Handler) slog.Handler { - return &SlogHandler{handler: h} -} - -func (h *SlogHandler) Enabled(ctx context.Context, level slog.Level) bool { - return h.handler.Enabled(ctx, level) -} - -func (h *SlogHandler) Handle(ctx context.Context, record slog.Record) error { - spanCtx := trace.SpanContextFromContext(ctx) - if spanCtx.HasTraceID() { - traceID := spanCtx.TraceID() - record.AddAttrs(slog.String("trace_id", traceID.String())) - } - - session, err := appcontext.SessionFromContext(ctx) - if err == nil { - record.AddAttrs(slog.String("session_id", session.SessionID)) - - if session.OrganisationID != "" { - record.AddAttrs(slog.String("organisation_id", session.OrganisationID)) - } - } - - return h.handler.Handle(ctx, record) -} - -func (h *SlogHandler) WithAttrs(attrs []slog.Attr) slog.Handler { - return NewSlogHandler(h.handler.WithAttrs(attrs)) -} - -func (h *SlogHandler) WithGroup(name string) slog.Handler { - return NewSlogHandler(h.handler.WithGroup(name)) -}