Skip to content

Commit

Permalink
feat(cb2-14315): ngrx based approach for internal logging system
Browse files Browse the repository at this point in the history
  • Loading branch information
matthew2564 committed Oct 9, 2024
1 parent 4c6eca6 commit 97276ad
Show file tree
Hide file tree
Showing 24 changed files with 694 additions and 20 deletions.
33 changes: 17 additions & 16 deletions setenv.ts
Original file line number Diff line number Diff line change
@@ -1,30 +1,31 @@
const { writeFile } = require('fs');
const { writeFile } = require('node:fs');
const { argv } = require('yargs');
const path = require('path');
const envFile = path.join(__dirname, `.env${argv.env ? '.' + argv.env : ''}`);
const path = require('node:path');
const envFile = path.join(__dirname, `.env${argv.env ? `.${argv.env}` : ''}`);

// read environment variables from .env file
require('dotenv').config({
path: path.resolve(envFile)
path: path.resolve(envFile),
});

// read the command line arguments passed with yargs
const environment = argv.environment;
const isProduction = environment === 'prod';
const targetPath = isProduction ? `./src/environments/environment.prod.ts` : `./src/environments/environment.deploy.ts`;
const targetPath = isProduction ? './src/environments/environment.prod.ts' : './src/environments/environment.deploy.ts';

// we have access to our environment variables
// in the process.env object thanks to dotenv
const environmentFileContent = `export const environment = {
production: ${isProduction},
TARGET_ENV: '${process.env["TARGET_ENV"]}',
TARGET_ENV: '${process.env['TARGET_ENV']}',
RemoveAADFullAccessRole: ${process.env['RemoveAADFullAccessRole']},
EnableDevTools: ${process.env['EnableDevTools']},
VTM_CLIENT_ID: "${process.env['VTM_CLIENT_ID']}",
VTM_AUTHORITY_ID: "${process.env['VTM_AUTHORITY_ID']}",
VTM_REDIRECT_URI: "${process.env['VTM_REDIRECT_URI']}",
VTM_API_URI: "${process.env['VTM_API_URI']}",
VTM_API_CLIENT_ID: "${process.env['VTM_API_CLIENT_ID']}",
LOGS_API_KEY: "${process.env['LOGS_API_KEY']}",
DOCUMENT_RETRIEVAL_API_KEY: "${process.env['DOCUMENT_RETRIEVAL_API_KEY']}",
FEEDBACK_URI: "${process.env['FEEDBACK_URI']}",
SENTRY_DSN: "${process.env['SENTRY_DSN']}",
Expand All @@ -34,18 +35,18 @@ const environmentFileContent = `export const environment = {
`;

const filesToWrite = [
{
path: targetPath,
contents: environmentFileContent
}
{
path: targetPath,
contents: environmentFileContent,
},
];

// write the content to the respective file
filesToWrite.forEach(({ path, contents }) => {
writeFile(path, contents, (err: string) => {
if (err) {
console.log(err);
}
console.log(`Wrote variables to ${path}`);
});
writeFile(path, contents, (err: string) => {
if (err) {
console.log(err);
}
console.log(`Wrote variables to ${path}`);
});
});
5 changes: 5 additions & 0 deletions src/app/app.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { Store, select } from '@ngrx/store';
import * as Sentry from '@sentry/angular';
import { LoadingService } from '@services/loading/loading.service';
import { UserService } from '@services/user-service/user-service';
import { startSendingLogs } from '@store/logs/logs.actions';
import { selectRouteData } from '@store/router/router.selectors';
// eslint-disable-next-line import/no-extraneous-dependencies
import { GoogleTagManagerService } from 'angular-google-tag-manager';
Expand Down Expand Up @@ -33,6 +34,9 @@ export class AppComponent implements OnInit, OnDestroy {

async ngOnInit() {
this.startSentry();

this.store.dispatch(startSendingLogs());

this.router.events.pipe(takeUntil(this.destroy$)).subscribe((event: Event) => {
if (event instanceof NavigationEnd) {
const gtmTag = {
Expand All @@ -42,6 +46,7 @@ export class AppComponent implements OnInit, OnDestroy {
void this.gtmService.pushTag(gtmTag);
}
});

await this.gtmService.addGtmToDom();
initAll();
}
Expand Down
6 changes: 6 additions & 0 deletions src/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
InteractionType,
PublicClientApplication,
} from '@azure/msal-browser';
import { ResponseLoggerInterceptor } from '@interceptors/response-logger/response-logger.interceptor';
import * as Sentry from '@sentry/angular';
import { FeatureToggleService } from '@services/feature-toggle-service/feature-toggle-service';
// eslint-disable-next-line import/no-extraneous-dependencies
Expand Down Expand Up @@ -94,6 +95,11 @@ const featureFactory = (featureFlagsService: FeatureToggleService) => () => feat
useClass: MsalInterceptor,
multi: true,
},
{
provide: HTTP_INTERCEPTORS,
useClass: ResponseLoggerInterceptor,
multi: true,
},
{
provide: MSAL_INSTANCE,
useFactory: MSALInstanceFactory,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import { HttpErrorResponse, HttpHandler, HttpRequest, HttpResponse } from '@angular/common/http';
import { TestBed } from '@angular/core/testing';
import { ResponseLoggerInterceptor } from '@interceptors/response-logger/response-logger.interceptor';
import { LogType } from '@models/logs/logs.model';
import { MockStore, provideMockStore } from '@ngrx/store/testing';
import { LogsProvider } from '@services/logs/logs.service';
import { id, initialState } from '@store/user/user-service.reducer';
import { of, throwError } from 'rxjs';

describe('Interceptor: ResponseLoggerInterceptor', () => {
let interceptor: ResponseLoggerInterceptor;
let logsProvider: jest.Mocked<LogsProvider>;
let mockStore: MockStore;
const mockThreshold = 10000;
const mockReq = new HttpRequest('GET', 'https://example.com');
const mockNext = {
handle: jest.fn(),
} as jest.Mocked<HttpHandler>;

beforeEach(() => {
const logsProviderMock = {
dispatchLog: jest.fn(),
};

TestBed.configureTestingModule({
providers: [
ResponseLoggerInterceptor,
{ provide: LogsProvider, useValue: logsProviderMock },
provideMockStore({ initialState }),
],
});

interceptor = TestBed.inject(ResponseLoggerInterceptor);
logsProvider = TestBed.inject(LogsProvider) as jest.Mocked<LogsProvider>;
mockStore = TestBed.inject(MockStore);

jest.spyOn(interceptor, 'threshold', 'get').mockReturnValue(mockThreshold);

mockStore.overrideSelector(id, 'test-oid');
});

it('should be created', () => {
expect(interceptor).toBeTruthy();
});

it('should calculate request duration', () => {
expect(interceptor.getRequestDuration(11000, 10000)).toEqual(1000);
});

it('should not log when request is for local assets', (done) => {
const localReq = new HttpRequest('GET', 'assets/test.json');
mockNext.handle.mockReturnValue(of(new HttpResponse()));

interceptor.intercept(localReq, mockNext).subscribe(() => {
expect(logsProvider.dispatchLog).not.toHaveBeenCalled();
done();
});
});

it('should log successful responses', (done) => {
const mockResponse = new HttpResponse({ status: 200, statusText: 'OK', url: 'https://example.com' });
mockNext.handle.mockReturnValue(of(mockResponse));

interceptor.intercept(mockReq, mockNext).subscribe(() => {
expect(logsProvider.dispatchLog).toHaveBeenCalledWith({
type: LogType.INFO,
message: 'test-oid - 200 OK for API call to https://example.com',
timestamp: expect.any(Number),
});
done();
});
});

it('should log when request is slower than threshold', (done) => {
const mockResponse = new HttpResponse({ status: 200, statusText: 'OK', url: 'https://example.com' });
mockNext.handle.mockReturnValue(of(mockResponse));

jest.spyOn(interceptor as any, 'getRequestDuration').mockReturnValue(mockThreshold + 1);

interceptor.intercept(mockReq, mockNext).subscribe(() => {
expect(logsProvider.dispatchLog).toHaveBeenCalledWith({
timestamp: expect.any(Number),
oid: 'test-oid',
type: LogType.WARN,
detail: 'Long Request',
message: 'Request to https://example.com is taking longer than 10 seconds',
requestDurationInMs: mockThreshold + 1,
});
done();
});
});

it('should log errors', (done) => {
const mockError = new HttpErrorResponse({ status: 404, statusText: 'Not Found', url: 'https://example.com' });
mockNext.handle.mockReturnValue(throwError(() => mockError));

interceptor.intercept(mockReq, mockNext).subscribe({
error: () => {
expect(logsProvider.dispatchLog).toHaveBeenCalledWith({
type: LogType.ERROR,
message: 'test-oid - Http failure response for https://example.com: 404 Not Found',
status: 404,
errors: undefined,
timestamp: expect.any(Number),
});
done();
},
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import {
HttpErrorResponse,
HttpEvent,
HttpHandler,
HttpInterceptor,
HttpRequest,
HttpResponse,
} from '@angular/common/http';
import { Injectable, inject } from '@angular/core';
import { LogType } from '@models/logs/logs.model';
import { Store } from '@ngrx/store';
import { LogsProvider } from '@services/logs/logs.service';
import { id } from '@store/user/user-service.reducer';
import { get } from 'lodash';
import { Observable, catchError, tap, throwError } from 'rxjs';

@Injectable()
export class ResponseLoggerInterceptor implements HttpInterceptor {
private logsProvider = inject(LogsProvider);
private oid = inject(Store).selectSignal(id);

intercept<T>(request: HttpRequest<T>, next: HttpHandler): Observable<HttpEvent<unknown>> {
const start = Date.now();

return next.handle(request).pipe(
tap((event) => {
// skip logging for local files
if (request.url.includes('assets/') && request.url.endsWith('.json')) return;

const finish = Date.now();

if (event instanceof HttpResponse) {
this.logsProvider.dispatchLog({
type: LogType.INFO,
message: `${this.oid()} - ${event.status} ${event.statusText} for API call to ${event.url}`,
timestamp: Date.now(),
});
}

const requestDuration = this.getRequestDuration(finish, start);

// if the request took longer than the threshold, log the request
if (requestDuration > this.threshold) {
this.logsProvider.dispatchLog({
timestamp: Date.now(),
oid: this.oid(),
type: LogType.WARN,
detail: 'Long Request',
message: `Request to ${request.url} is taking longer than ${this.threshold / 1000} seconds`,
requestDurationInMs: requestDuration,
});
}
}),
catchError((err) => {
const status = err instanceof HttpErrorResponse ? err.status : 0;

const message = err instanceof HttpErrorResponse || err instanceof Error ? err.message : JSON.stringify(err);

this.logsProvider.dispatchLog({
type: LogType.ERROR,
message: `${this.oid()} - ${message}`,
status,
errors: get(err, 'error.errors', undefined),
timestamp: Date.now(),
});

return throwError(() => err);
})
);
}

get threshold(): number {
return 10_000; // 10 seconds in milliseconds
}

// separated out to allow simplified spies in tests
getRequestDuration(finish: number, start: number): number {
return finish - start;
}
}
15 changes: 15 additions & 0 deletions src/app/models/logs/logs.model.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
export type LogsModel = Log[];

export type Log = {
type: string;
message: string;
timestamp: number;
[propName: string]: unknown;
};

export enum LogType {
DEBUG = 'debug',
INFO = 'info',
WARN = 'warn',
ERROR = 'error',
}
12 changes: 12 additions & 0 deletions src/app/services/http/http.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,4 +380,16 @@ describe('HttpService', () => {
expect(req.request.body).toHaveProperty('secondaryVrms');
});
});

describe('sendLogs', () => {
it('should send a logs array and report back a success', async () => {
httpService.sendLogs([]).then((response) => {
expect(response.body).toEqual([]);
});

const req = httpTestingController.expectOne(`${environment.VTM_API_URI}/log`);
expect(req.request.method).toBe('POST');
req.flush([], { status: 200, statusText: 'OK' });
});
});
});
18 changes: 17 additions & 1 deletion src/app/services/http/http.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { TechRecordSearchSchema } from '@dvsa/cvs-type-definitions/types/v3/tech
import { TechRecordType } from '@dvsa/cvs-type-definitions/types/v3/tech-record/tech-record-verb';
import { environment } from '@environments/environment';
import { Defect } from '@models/defects/defect.model';
import { Log } from '@models/logs/logs.model';
import {
DeleteItem,
ReferenceDataApiResponse,
Expand All @@ -25,10 +26,12 @@ import { TechRecordArchiveAndProvisionalPayload } from '@models/vehicle/techReco
import { TechRecordPOST } from '@models/vehicle/techRecordPOST';
import { TechRecordPUT } from '@models/vehicle/techRecordPUT';
import { cloneDeep } from 'lodash';
import { lastValueFrom, timeout } from 'rxjs';

@Injectable({ providedIn: 'root' })
export class HttpService {
http = inject(HttpClient);
private http = inject(HttpClient);
private static readonly TIMEOUT = 30000;

addProvisionalTechRecord(body: TechRecordArchiveAndProvisionalPayload, systemNumber: string) {
if (body === null || body === undefined) {
Expand Down Expand Up @@ -638,4 +641,17 @@ export class HttpService {
}
);
}

sendLogs = async (logs: Log[]) => {
const headers = new HttpHeaders().set('x-api-key', environment.LOGS_API_KEY);

return lastValueFrom(
this.http
.post(`${environment.VTM_API_URI}/log`, logs, {
headers,
observe: 'response',
})
.pipe(timeout(HttpService.TIMEOUT))
);
};
}
Loading

0 comments on commit 97276ad

Please sign in to comment.