Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[743] send custom page load response metrics #144

Merged
merged 5 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 86 additions & 0 deletions src/server/common/controller/generic-page-controller/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import {
createMetricsLogger,
Unit,
StorageResolution
} from 'aws-embedded-metrics'
import { config } from '~/src/config/config.js'
import { createLogger } from '../../helpers/logging/logger.js'
import { NotImplementedError } from '../../helpers/not-implemented-error.js'

/**
* @import {Page} from '../../model/page/page-model.js'
*/

/**
* export @interface PageController {
* handleGet: (req: Request, h: ResponseToolkit) => ResponseObject
* }
*
* export @typedef {{
* request ?: boolean;
* response ?: boolean;
* }} Metrics
*
* export @typedef {{
* get ?: Metrics;
* post ?: Metrics;
* }} MetricReports
*/

export default class GenericPageController {
metrics = createMetricsLogger()
logger = createLogger()

/**
* @param {Page} page
*/
constructor(page) {
this.page = page
}

getHandler(req, h) {
this.sendMetric('get', 'request')
const result = this.handleGet(req, h)
this.sendMetric('get', 'response')
return result
}

postHandler(req, h) {
this.sendMetric('post', 'request')
const result = this.handlePost(req, h)
this.sendMetric('post', 'response')
return result
}

/**
*
* @param {string} method
* @param {string} event
*/
sendMetric(method, event) {
if (!config.get('isProduction')) {
return
}

const sendMetric = this.page.reportMetrics?.[method]?.[event]

if (sendMetric) {
this.metrics.putMetric(
`${method}::${event}-${this.page.urlPath}`,
1,
Unit.Count,
StorageResolution.Standard
)
}
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
handleGet(_req, _h) {
throw new NotImplementedError()
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
handlePost(_req, _h) {
throw new NotImplementedError()
}
}
95 changes: 95 additions & 0 deletions src/server/common/controller/generic-page-controller/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import { config } from '~/src/config/config.js'
import { Page } from '../../model/page/page-model.js'
import GenericPageController from './index.js'
import { NotImplementedError } from '../../helpers/not-implemented-error.js'

class TestPage extends Page {
reportMetrics = {
get: {
request: false,
response: true
},
post: {
request: false,
response: true
}
}
}

class TestGenericController extends GenericPageController {
handleGet() {
return 'get'
}

handlePost() {
return 'post'
}
}

describe('#GenericPageController', () => {
let controller

beforeEach(() => {
config.set('isProduction', true)
})

beforeEach(() => {
const page = new TestPage()
controller = new TestGenericController(page)
})

it('should throw not implemented error', () => {
const defaultController = new GenericPageController(new Page())
expect(() => defaultController.handleGet()).toThrow(NotImplementedError)
expect(() => defaultController.handlePost()).toThrow(NotImplementedError)
})

it('should output error on getHandler', () => {
jest.spyOn(controller, 'handleGet').mockImplementation(() => {
throw new Error('test error')
})

jest.spyOn(controller.logger, 'error').mockImplementation(() => {
throw new Error('test error')
})
expect(() => controller.getHandler()).toThrow()
})

it('should output error on postHandler', () => {
jest.spyOn(controller, 'handlePost').mockImplementation(() => {
throw new Error('test error')
})

jest.spyOn(controller.logger, 'error').mockImplementation(() => {
throw new Error('test error')
})
expect(() => controller.postHandler()).toThrow()
})

it('should send metric on getHandler', () => {
jest.spyOn(controller, 'sendMetric')
jest.spyOn(controller, 'handleGet').mockImplementation(() => {
return 'get success'
})
const metricSpy = jest.spyOn(controller.metrics, 'putMetric')
controller.getHandler()
expect(controller.sendMetric).toHaveBeenCalledWith('get', 'response')
expect(metricSpy).toHaveBeenCalledTimes(1)
})

it('should send metric on postHandler', () => {
jest.spyOn(controller, 'sendMetric')
jest.spyOn(controller, 'handlePost').mockImplementation(() => {
return 'get success'
})
const metricSpy = jest.spyOn(controller.metrics, 'putMetric')
controller.postHandler()
expect(controller.sendMetric).toHaveBeenCalledWith('post', 'response')
expect(metricSpy).toHaveBeenCalledTimes(1)
})

afterEach(() => {
jest.clearAllMocks()
config.set('isProduction', false)
})
})
11 changes: 8 additions & 3 deletions src/server/common/controller/page-controller/page-controller.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { calculateNextPage } from '../../helpers/next-page.js'
import { ExitPage } from '../../model/page/exit-page-model.js'
import GenericPageController from '../generic-page-controller/index.js'
/** @import { Server, ServerRegisterPluginObject, ServerRoute, ReqRefDefaults, RouteDefMethods } from '@hapi/hapi' */
/** @import { NextPage } from '../../helpers/next-page.js' */
/** @import { Page } from '../../model/page/page-model.js' */
Expand All @@ -15,13 +16,17 @@ const defaultControllerOptions = {
methods: ['GET', 'POST']
}

export class PageController {
/**
* @satisfies {PageController}
*/
export class PageController extends GenericPageController {
options
/**
* @param {Page} page
* @param {ControllerOptions} opts
*/
constructor(page, opts = defaultControllerOptions) {
super(page)
this.page = page

this.options = opts
Expand Down Expand Up @@ -50,7 +55,7 @@ export class PageController {
}
}

getHandler(req, h) {
handleGet(req, h) {
return h.view(this.page.view, {
nextPage: req.query.redirect_uri,
pageTitle: this.page.title,
Expand All @@ -59,7 +64,7 @@ export class PageController {
})
}

postHandler(req, h) {
handlePost(req, h) {
const payload = /** @type {NextPage} */ (req.payload)
const nextPage = this.page.nextPage()

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import { calculateNextPage } from '../../helpers/next-page.js'
import { ExitPage } from '../../model/page/exit-page-model.js'
import GenericPageController from '../generic-page-controller/index.js'
/** @import { Server, ServerRegisterPluginObject } from '@hapi/hapi' */
/** @import { NextPage } from '../../helpers/next-page.js' */
/** @import { RawPayload } from '../../model/answer/answer-model.js' */
/** @import { QuestionPage } from '../../model/page/question-page-model.js' */

export class QuestionPageController {
export class QuestionPageController extends GenericPageController {
/**
* @param {QuestionPage} page
*/
constructor(page) {
super(page)
this.page = page
}

Expand Down Expand Up @@ -38,7 +40,7 @@ export class QuestionPageController {
}
}

getHandler(req, h) {
handleGet(req, h) {
const sectionState = req.yar.get(this.page.sectionKey)
const answer = this.page.Answer.fromState(
sectionState?.[this.page.questionKey]
Expand All @@ -53,7 +55,7 @@ export class QuestionPageController {
})
}

postHandler(req, h) {
handlePost(req, h) {
const payload = /** @type {NextPage} */ (req.payload)
const Answer = this.page.Answer
const answer = new Answer(/** @type {RawPayload} */ (payload))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
/** @import { Server, ServerRegisterPluginObject } from '@hapi/hapi' */

import { sectionToSummary } from '../../templates/macros/create-summary.js'
import GenericPageController from '../generic-page-controller/index.js'

export class SummaryPageController {
export class SummaryPageController extends GenericPageController {
/** @type {string} */
indexView = 'common/controller/summary-page-controller/index.njk'

Expand All @@ -17,6 +18,7 @@ export class SummaryPageController {
* @param {SummaryPage} page
*/
constructor(page) {
super(page)
this.page = page
}

Expand Down Expand Up @@ -49,7 +51,7 @@ export class SummaryPageController {
}
}

getHandler(req, res) {
handleGet(req, res) {
const section = this.page.sectionFactory(req.yar.get(this.page.sectionKey))

const { isValid, firstInvalidPage } = section.validate()
Expand All @@ -66,7 +68,7 @@ export class SummaryPageController {
})
}

postHandler(_req, res) {
handlePost(_req, res) {
return res.redirect('/task-list')
}
}
13 changes: 13 additions & 0 deletions src/server/common/model/page/page-model.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,20 @@
import { NotImplementedError } from '../../helpers/not-implemented-error.js'
/** @import { AnswerModel } from '../answer/answer-model.js' */
/** @import { MetricReports, Metrics } from '../../controller/generic-page-controller/index.js' */

export class Page {
/** @type {MetricReports} */
reportMetrics = {
get: {
request: false,
response: false
},
post: {
request: false,
response: false
}
}

/** @type {string} */
urlPath

Expand Down Expand Up @@ -39,13 +52,13 @@
/**
* @param {import('@hapi/hapi').Request} _req
* @returns {Record<string, unknown>}
* */

Check warning on line 55 in src/server/common/model/page/page-model.js

View workflow job for this annotation

GitHub Actions / Run Pull Request Checks

Should be no multiple asterisks on end lines

Check warning on line 55 in src/server/common/model/page/page-model.js

View workflow job for this annotation

GitHub Actions / Run Pull Request Checks

Should be no multiple asterisks on end lines
// eslint-disable-next-line @typescript-eslint/no-unused-vars
viewProps(_req) {
return {}
}

/**

Check warning on line 61 in src/server/common/model/page/page-model.js

View workflow job for this annotation

GitHub Actions / Run Pull Request Checks

JSDoc @returns declaration present but return expression not available in function

Check warning on line 61 in src/server/common/model/page/page-model.js

View workflow job for this annotation

GitHub Actions / Run Pull Request Checks

JSDoc @returns declaration present but return expression not available in function
* @param {AnswerModel} [_answer]
* @returns {Page }
*/
Expand Down
2 changes: 1 addition & 1 deletion src/server/destination/summary/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import SummaryPage from '../../common/model/page/summary-page/SummaryPageModel.js'
import { SummaryPageController } from '../../common/controller/summary-page-controller/SummaryPageController.js'
import { SummaryPageController } from '../../common/controller/summary-page-controller/summary-page-controller.js'

import { DestinationSection } from '~/src/server/common/model/section/destination/destination.js'

Expand Down
2 changes: 1 addition & 1 deletion src/server/licence/check-answers/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import SummaryPage from '../../common/model/page/summary-page/SummaryPageModel.js'
import { SummaryPageController } from '../../common/controller/summary-page-controller/SummaryPageController.js'
import { SummaryPageController } from '../../common/controller/summary-page-controller/summary-page-controller.js'

import { LicenceSection } from '~/src/server/common/model/section/licence/licence.js'

Expand Down
2 changes: 1 addition & 1 deletion src/server/origin/summary/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import SummaryPage from '../../common/model/page/summary-page/SummaryPageModel.js'
import { SummaryPageController } from '../../common/controller/summary-page-controller/SummaryPageController.js'
import { SummaryPageController } from '../../common/controller/summary-page-controller/summary-page-controller.js'

import { OriginSection } from '~/src/server/common/model/section/origin/origin.js'

Expand Down
8 changes: 8 additions & 0 deletions src/server/submit/confirmation/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ export class ConfirmationPage extends Page {
urlPath = '/submit/confirmation'
view = 'submit/confirmation/index'

// sends metric that the page was served
reportMetrics = {
get: {
request: false,
response: true
}
}

pageTitle = title
pageHeading = title
}
Expand Down
Loading