From 06a413c8072cec82b2ea46f909d74cc063178b26 Mon Sep 17 00:00:00 2001 From: Michael Barrett Date: Fri, 5 Jan 2024 12:10:39 +0000 Subject: [PATCH] Updated Sentry env to use PRO_ENV when available (#19441) refs [ARCH-33](https://linear.app/tryghost/issue/ARCH-33/fix-sentry-environment) To ensure that we are correctly identifying the environment that data is being sent to Sentry from, we can use the `PRO_ENV` environment variable if it is available. This will be set to `production` in production and `staging` in staging. If `PRO_ENV` is not available, we will fall back to retrieving the environment from config (`env`) --- .../server/services/public-config/site.js | 8 ++- ghost/core/core/shared/sentry.js | 7 +- .../services/public-config/site.test.js | 66 +++++++++++++++++++ ghost/core/test/unit/shared/sentry.test.js | 15 +++++ 4 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 ghost/core/test/unit/server/services/public-config/site.test.js diff --git a/ghost/core/core/server/services/public-config/site.js b/ghost/core/core/server/services/public-config/site.js index b94672af8b..2b43a62987 100644 --- a/ghost/core/core/server/services/public-config/site.js +++ b/ghost/core/core/server/services/public-config/site.js @@ -19,7 +19,13 @@ module.exports = function getSiteProperties() { if (config.get('client_sentry') && !config.get('client_sentry').disabled) { siteProperties.sentry_dsn = config.get('client_sentry').dsn; - siteProperties.sentry_env = config.get('env'); + + let environment = config.get('PRO_ENV'); + if (!environment) { + environment = config.get('env'); + } + + siteProperties.sentry_env = environment; } return siteProperties; diff --git a/ghost/core/core/shared/sentry.js b/ghost/core/core/shared/sentry.js index f267234453..27343fd37e 100644 --- a/ghost/core/core/shared/sentry.js +++ b/ghost/core/core/shared/sentry.js @@ -69,7 +69,12 @@ const beforeSend = function (event, hint) { if (sentryConfig && !sentryConfig.disabled) { const Sentry = require('@sentry/node'); const version = require('@tryghost/version').full; - const environment = config.get('env'); + + let environment = config.get('PRO_ENV'); + if (!environment) { + environment = config.get('env'); + } + const sentryInitConfig = { dsn: sentryConfig.dsn, release: 'ghost@' + version, diff --git a/ghost/core/test/unit/server/services/public-config/site.test.js b/ghost/core/test/unit/server/services/public-config/site.test.js new file mode 100644 index 0000000000..ccd21aed6f --- /dev/null +++ b/ghost/core/test/unit/server/services/public-config/site.test.js @@ -0,0 +1,66 @@ +const assert = require('assert/strict'); +const configUtils = require('../../../../utils/configUtils'); +const getSiteProperties = require('../../../../../core/server/services/public-config/site'); + +describe('Public-config Service', function () { + describe('Site Properties', function () { + describe('Sentry', function () { + const fakeDSN = 'https://aaabbbccc000111222333444555667@sentry.io/1234567'; + + afterEach(async function () { + await configUtils.restore(); + }); + + it('should not include sentry properties if sentry disabled via config', function () { + configUtils.set({ + client_sentry: { + disabled: true + } + }); + + const siteProperties = getSiteProperties(); + + assert.equal(siteProperties.sentry_dsn, undefined); + assert.equal(siteProperties.sentry_env, undefined); + }); + + it('should not include sentry properties if sentry not present in config', function () { + const siteProperties = getSiteProperties(); + + assert.equal(siteProperties.sentry_dsn, undefined); + assert.equal(siteProperties.sentry_env, undefined); + }); + + it('should include sentry properties if sentry not disabled in config', function () { + configUtils.set({ + client_sentry: { + disabled: false, + dsn: fakeDSN + } + }); + + const siteProperties = getSiteProperties(); + + assert.equal(siteProperties.sentry_dsn, fakeDSN); + assert.equal(siteProperties.sentry_env, 'testing'); // testing is the default env + }); + + it('should use PRO_ENV env var for sentry_env property if in config', function () { + const env = 'staging'; + + configUtils.set({ + client_sentry: { + disabled: false, + dsn: fakeDSN + }, + PRO_ENV: env + }); + + const siteProperties = getSiteProperties(); + + assert.equal(siteProperties.sentry_dsn, fakeDSN); + assert.equal(siteProperties.sentry_env, env); + }); + }); + }); +}); diff --git a/ghost/core/test/unit/shared/sentry.test.js b/ghost/core/test/unit/shared/sentry.test.js index 135727413d..05047685a0 100644 --- a/ghost/core/test/unit/shared/sentry.test.js +++ b/ghost/core/test/unit/shared/sentry.test.js @@ -51,6 +51,21 @@ describe('UNIT: sentry', function () { assert.equal(initArgs[0].environment, 'testing', 'should be the testing env'); assert.ok(initArgs[0].hasOwnProperty('beforeSend'), 'should have a beforeSend function'); }); + + it('initialises sentry with the correct environment', function () { + const env = 'staging'; + + configUtils.set({ + PRO_ENV: env + }); + + delete require.cache[require.resolve('../../../core/shared/sentry')]; + require('../../../core/shared/sentry'); + + const initArgs = Sentry.init.getCall(1).args; + + assert.equal(initArgs[0].environment, env, 'should be the correct env'); + }); }); describe('beforeSend', function () {