From 930a7516fe07d0c93444b68127265e6f8402d2b4 Mon Sep 17 00:00:00 2001 From: Jon Shipley Date: Fri, 6 Dec 2024 11:30:35 +0000 Subject: [PATCH] Bug/65582 init app insights error logging asap (#2984) * Bump rexml from 3.3.4 to 3.3.9 in /test/admin-hpa Bumps [rexml](https://github.com/ruby/rexml) from 3.3.4 to 3.3.9. - [Release notes](https://github.com/ruby/rexml/releases) - [Changelog](https://github.com/ruby/rexml/blob/master/NEWS.md) - [Commits](https://github.com/ruby/rexml/compare/v3.3.4...v3.3.9) --- updated-dependencies: - dependency-name: rexml dependency-type: indirect ... Signed-off-by: dependabot[bot] * fix: upgrade ramda from 0.28.0 to 0.30.1 Snyk has created this PR to upgrade ramda from 0.28.0 to 0.30.1. See this package in npm: ramda See this project in Snyk: https://app.snyk.io/org/mtc/project/f2cd044f-cd02-4438-8249-c21552435905?utm_source=github&utm_medium=referral&page=upgrade-pr * fix: upgrade sass from 1.79.3 to 1.80.4 Snyk has created this PR to upgrade sass from 1.79.3 to 1.80.4. See this package in yarn: sass See this project in Snyk: https://app.snyk.io/org/mtc/project/86296478-3a5b-43ab-9fc6-4b4c5f4833ba?utm_source=github&utm_medium=referral&page=upgrade-pr * fix: upgrade @babel/runtime from 7.25.6 to 7.26.0 Snyk has created this PR to upgrade @babel/runtime from 7.25.6 to 7.26.0. See this package in yarn: @babel/runtime See this project in Snyk: https://app.snyk.io/org/activemq/project/cc9ba2aa-367d-4704-a7a4-c5455f64251f?utm_source=github&utm_medium=referral&page=upgrade-pr * Bump cross-spawn from 7.0.3 to 7.0.6 in /tslib Bumps [cross-spawn](https://github.com/moxystudio/node-cross-spawn) from 7.0.3 to 7.0.6. - [Changelog](https://github.com/moxystudio/node-cross-spawn/blob/master/CHANGELOG.md) - [Commits](https://github.com/moxystudio/node-cross-spawn/compare/v7.0.3...v7.0.6) --- updated-dependencies: - dependency-name: cross-spawn dependency-type: indirect ... Signed-off-by: dependabot[bot] * [admin] appInsights mod Mod appinsights to log early, e.g. due to bad Redis config. * fix: admin-assets/Dockerfile to reduce vulnerabilities The following vulnerabilities are fixed with an upgrade: - https://snyk.io/vuln/SNYK-ALPINE319-EXPAT-7908400 - https://snyk.io/vuln/SNYK-ALPINE319-EXPAT-7908409 - https://snyk.io/vuln/SNYK-ALPINE319-CURL-7567383 - https://snyk.io/vuln/SNYK-ALPINE319-CURL-7567383 - https://snyk.io/vuln/SNYK-ALPINE319-EXPAT-7908399 * Fix to pass tests * Fix logic error * Log pupil-api redis errors to App Insights * refactor name * whitespace --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: snyk-bot --- admin-assets/Dockerfile | 2 +- admin/models/logger.js | 4 +-- admin/package.json | 2 +- .../data-access/redis-cache.service.js | 26 ++++++++++++------- admin/yarn.lock | 2 +- load-test/package.json | 2 +- pupil-spa/package.json | 2 +- pupil-spa/yarn.lock | 2 +- test/admin-hpa/Gemfile.lock | 4 +-- tslib/src/pupil-api/helpers/app-insights.ts | 8 +++++- tslib/src/pupil-api/services/log.service.ts | 17 +++++------- tslib/src/pupil-api/services/redis.service.ts | 1 + tslib/yarn.lock | 6 ++--- 13 files changed, 42 insertions(+), 36 deletions(-) diff --git a/admin-assets/Dockerfile b/admin-assets/Dockerfile index 3bc98c728f..de3cdb24cd 100644 --- a/admin-assets/Dockerfile +++ b/admin-assets/Dockerfile @@ -1,5 +1,5 @@ -FROM nginx:1.26.1-alpine +FROM nginx:1.26.2-alpine ## Copy our default nginx config COPY nginx/default.conf /etc/nginx/conf.d/ diff --git a/admin/models/logger.js b/admin/models/logger.js index 6725deefae..87edf924d2 100644 --- a/admin/models/logger.js +++ b/admin/models/logger.js @@ -18,8 +18,8 @@ class Logger { console.info('Log level is set to `%s`', this.level) let format - if (config.Logging.SendToAppInsights) { - format = winston.format.simple() + if (config.Monitoring.ApplicationInsights.ConnectionString) { + format = winston.format.json() } else { format = winston.format.combine( winston.format.colorize({ all: true }), diff --git a/admin/package.json b/admin/package.json index a9ed555b47..96bf38e8c5 100644 --- a/admin/package.json +++ b/admin/package.json @@ -90,7 +90,7 @@ "ramda": "^0.30.1", "ramda-adjunct": "^5.1.0", "random-number-csprng": "^1.0.2", - "sass": "^1.80.3", + "sass": "^1.80.4", "simplemde": "^1.11.2", "to-bool": "^0.0.1", "uuid": "^10.0.0", diff --git a/admin/services/data-access/redis-cache.service.js b/admin/services/data-access/redis-cache.service.js index cc6aa17c36..76faf1c14e 100644 --- a/admin/services/data-access/redis-cache.service.js +++ b/admin/services/data-access/redis-cache.service.js @@ -17,9 +17,15 @@ if (config.Redis.useTLS) { let redis -const redisConnect = () => { - if (!redis) { - redis = new Redis(redisConfig) +const redisConnect = async () => { + try { + if (!redis) { + redis = new Redis(redisConfig) + // Make a call to check the config is valid. This is only done once. + await redis.info() + } + } catch (error) { + logger.alert('ALERT: REDIS CONNECTION ERROR', error) } } @@ -31,7 +37,7 @@ const redisCacheService = {} * @returns {Promise} */ redisCacheService.get = async key => { - redisConnect() + await redisConnect() try { logger.debug(`REDIS (get): retrieving ${key}`) const cacheEntry = await redis.get(key) @@ -50,7 +56,7 @@ redisCacheService.get = async key => { * @returns {Promise} */ redisCacheService.set = async (key, value, ttl = undefined) => { - redisConnect() + await redisConnect() try { logger.debug(`REDIS (set): adding ${key} ttl:${ttl}`) const storageItemString = prepareCacheEntry(value) @@ -74,7 +80,7 @@ redisCacheService.drop = async (cacheKeys = []) => { if (Array.isArray(cacheKeys) && cacheKeys.length === 0) { return false } - redisConnect() + await redisConnect() if (typeof cacheKeys === 'string') { cacheKeys = [cacheKeys] } @@ -103,7 +109,7 @@ redisCacheService.setMany = async (items) => { if (!Array.isArray(items)) { throw new Error('items is not an array') } - redisConnect() + await redisConnect() const multi = redis.multi() for (let index = 0; index < items.length; index++) { const item = items[index] @@ -137,7 +143,7 @@ redisCacheService.getMany = async (keys) => { if (!Array.isArray(keys)) { throw new Error('keys is not an array') } - redisConnect() + await redisConnect() const rawData = await redis.mget(...keys) const data = rawData.map(raw => unwrap(raw)) logger.debug(`(redis) getMany ${keys.join(', ')}`) @@ -157,7 +163,7 @@ redisCacheService.getTtl = async (key) => { if (key.length === 0) { throw new Error('Invalid key length') } - redisConnect() + await redisConnect() return await redis.ttl(key) } @@ -168,7 +174,7 @@ redisCacheService.getTtl = async (key) => { * @return {Promise} */ redisCacheService.dropByPrefix = async (prefix) => { - redisConnect() + await redisConnect() const cmd = `for i, name in ipairs(redis.call('KEYS', '${prefix}*')) do redis.call('DEL', name); end` await redis.eval(cmd, 0) } diff --git a/admin/yarn.lock b/admin/yarn.lock index ab580bd847..2a6a998b76 100644 --- a/admin/yarn.lock +++ b/admin/yarn.lock @@ -10933,7 +10933,7 @@ safe-stable-stringify@^2.3.1: resolved "https://registry.yarnpkg.com/safer-buffer/-/safer-buffer-2.1.2.tgz#44fa161b0187b9549dd84bb91802f9bd8385cd6a" integrity sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg== -sass@^1.80.3: +sass@^1.80.4: version "1.81.0" resolved "https://registry.yarnpkg.com/sass/-/sass-1.81.0.tgz#a9010c0599867909dfdbad057e4a6fbdd5eec941" integrity sha512-Q4fOxRfhmv3sqCLoGfvrC9pRV8btc0UtqL9mN6Yrv6Qi9ScL55CVH1vlPP863ISLEEMNLLuu9P+enCeGHlnzhA== diff --git a/load-test/package.json b/load-test/package.json index 50cb9eb2c6..527e2ba287 100644 --- a/load-test/package.json +++ b/load-test/package.json @@ -26,7 +26,7 @@ "fs-extra": "^11.2.0", "ioredis": "^5.4.1", "number-to-words": "^1.2.3", - "ramda": "^0.28.0", + "ramda": "^0.30.1", "tedious": "^18.1.0", "uuid": "^9.0.1", "winston": "^3.15.0" diff --git a/pupil-spa/package.json b/pupil-spa/package.json index 356a93b055..fb5306d382 100644 --- a/pupil-spa/package.json +++ b/pupil-spa/package.json @@ -35,7 +35,7 @@ "@angular/platform-browser-dynamic": "^14.2.10", "@angular/platform-server": "^14.2.10", "@angular/router": "^14.2.10", - "@babel/runtime": "^7.25.7", + "@babel/runtime": "^7.26.0", "@microsoft/applicationinsights-web": "^3.3.3", "core-js": "2.6.12", "dotenv": "^16.0.3", diff --git a/pupil-spa/yarn.lock b/pupil-spa/yarn.lock index 4edd81ed45..b8c1f62b0f 100644 --- a/pupil-spa/yarn.lock +++ b/pupil-spa/yarn.lock @@ -1301,7 +1301,7 @@ dependencies: regenerator-runtime "^0.13.4" -"@babel/runtime@^7.25.7": +"@babel/runtime@^7.26.0": version "7.26.0" resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.26.0.tgz#8600c2f595f277c60815256418b85356a65173c1" integrity sha512-FDSOghenHTiToteC/QRlv2q3DhPZ/oOXTBoirfWNx1Cx3TMVcGWQtMMmQcSvb/JjpNeGzx8Pq/b4fKEJuWm1sw== diff --git a/test/admin-hpa/Gemfile.lock b/test/admin-hpa/Gemfile.lock index 8fef5202e4..2fffd2f4ab 100644 --- a/test/admin-hpa/Gemfile.lock +++ b/test/admin-hpa/Gemfile.lock @@ -159,8 +159,7 @@ GEM connection_pool regexp_parser (2.9.2) require_all (3.0.0) - rexml (3.3.4) - strscan + rexml (3.3.9) rspec (3.13.0) rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) @@ -189,7 +188,6 @@ GEM capybara (~> 3.32) site_prism-all_there (> 2, < 5) site_prism-all_there (3.0.5) - strscan (3.1.0) sys-uname (1.3.0) ffi (~> 1.1) tiny_tds (2.1.7) diff --git a/tslib/src/pupil-api/helpers/app-insights.ts b/tslib/src/pupil-api/helpers/app-insights.ts index 9b373abb1f..d11488a9ef 100644 --- a/tslib/src/pupil-api/helpers/app-insights.ts +++ b/tslib/src/pupil-api/helpers/app-insights.ts @@ -1,9 +1,10 @@ import { PingService } from '../services/ping.service' import config from '../config' import { isNil } from 'ramda' -const appInsights = require('applicationinsights') +const appInsights = require('applicationinsights') const cloudRoleName = 'Pupil-API' +let isAppInsightsSetup = false const connectionString = config.Logging.ApplicationInsights.ConnectionString if (isNil(connectionString)) { @@ -25,12 +26,17 @@ if (isNil(connectionString)) { .setInternalLogging(false, true) .setDistributedTracingMode(appInsights.DistributedTracingModes.AI_AND_W3C) .enableWebInstrumentation(false) + + isAppInsightsSetup = true appInsights.defaultClient.context.tags[appInsights.defaultClient.context.keys.cloudRole] = cloudRoleName appInsights.defaultClient.context.tags[appInsights.defaultClient.context.keys.cloudRoleInstance] = config.Logging.ApplicationInsights.InstanceId } const appInsightsHelper = { startInsightsIfConfigured: async () => { + if (!isAppInsightsSetup) { + return + } appInsights.start() console.log('Application insights: started') const pingService = new PingService() diff --git a/tslib/src/pupil-api/services/log.service.ts b/tslib/src/pupil-api/services/log.service.ts index ddbf100ada..984d908f70 100644 --- a/tslib/src/pupil-api/services/log.service.ts +++ b/tslib/src/pupil-api/services/log.service.ts @@ -31,41 +31,36 @@ export class Logger { this.logger = winston.createLogger(baseLogOptions) } - log (level: LogLevel, msg: string, exception?: any): void { + log (level: LogLevel, msg: string, exception?: Error | null | undefined): void { this.logger.log(level, msg, exception) } /** * AI -> critical - * @param {string} msg */ - alert (msg: string, exception = null): void { this.log('alert', msg, exception) } + alert (msg: string, exception: Error | null | undefined = null): void { this.log('alert', msg, exception) } /** * AI -> error - * @param {string} msg */ - error (msg: string, exception = null): void { this.log('error', msg, exception) } + error (msg: string, exception: Error | null | undefined = null): void { this.log('error', msg, exception) } /** * AI -> warning - * @param {string} msg */ - warn (msg: string, exception = null): void { this.log('warning', msg, exception) } + warn (msg: string, exception: Error | null | undefined = null): void { this.log('warning', msg, exception) } /** * AI -> notice - * @param {string} msg */ - info (msg: string, exception = null): void { + info (msg: string, exception: Error | null | undefined = null): void { this.log('info', msg, exception) } /** * AI -> verbose - * @param {string} msg */ - debug (msg: string, exception = null): void { this.log('debug', msg, exception) } + debug (msg: string, exception: Error | null | undefined = null): void { this.log('debug', msg, exception) } /** * Return the underlying `winston` logger diff --git a/tslib/src/pupil-api/services/redis.service.ts b/tslib/src/pupil-api/services/redis.service.ts index 8d9d413818..3a1a8cfd1e 100644 --- a/tslib/src/pupil-api/services/redis.service.ts +++ b/tslib/src/pupil-api/services/redis.service.ts @@ -65,6 +65,7 @@ export class RedisService implements IRedisService { } } this.redis = new Redis(options) + this.redis.on('error', (error) => { this.logger.error(`ERROR: [pupil-api]: redis error: ${error.message}`, error) }) this.logger = new Logger() } diff --git a/tslib/yarn.lock b/tslib/yarn.lock index 288d3325fe..185c9ba453 100644 --- a/tslib/yarn.lock +++ b/tslib/yarn.lock @@ -2434,9 +2434,9 @@ create-jest@^29.7.0: prompts "^2.0.1" cross-spawn@^7.0.2, cross-spawn@^7.0.3: - version "7.0.3" - resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-7.0.3.tgz#f73a85b9d5d41d045551c177e2882d4ac85728a6" - integrity sha512-iRDPJKUPVEND7dHPO8rkbOnPpyDygcDFtWjpeWNCgy8WP2rXcxXL8TskReQl6OrB2G7+UJrags1q15Fudc7G6w== + version "7.0.6" + resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-7.0.6.tgz#8a58fe78f00dcd70c370451759dfbfaf03e8ee9f" + integrity sha512-uV2QOWP2nWzsy2aMp8aRibhi9dlzF5Hgh5SHaB9OiTGEyDTiJJyx0uy51QXdyWbtAHNua4XJzUKca3OzKUd3vA== dependencies: path-key "^3.1.0" shebang-command "^2.0.0"