0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-27 22:49:56 -05:00

Added Sentry instrumentation for get helpers (#19576)

no issue

- To help debug ABORTED_GET_HELPER errors, this PR adds Sentry
instrumentation to the get helpers
- It also adds the homepage, any pages/posts, the tag page, and the
author page to the list of transactions that will send to Sentry
This commit is contained in:
Joe Grigg 2024-01-25 15:16:48 +00:00
parent 922af6defe
commit 3d39b35f46
3 changed files with 89 additions and 22 deletions

View file

@ -7,6 +7,7 @@ const {hbs} = require('../services/handlebars');
const logging = require('@tryghost/logging');
const errors = require('@tryghost/errors');
const tpl = require('@tryghost/tpl');
const Sentry = require('@sentry/node');
const _ = require('lodash');
const jsonpath = require('jsonpath');
@ -206,32 +207,47 @@ module.exports = async function get(resource, options) {
// Parse the options we're going to pass to the API
apiOptions = parseOptions(ghostGlobals, this, apiOptions);
let apiOptionsString = Object.entries(apiOptions)
.map(([key, value]) => ` ${key}="${value}"`)
.join('');
apiOptions.context = {member: data.member};
try {
const response = await makeAPICall(resource, controllerName, action, apiOptions);
const spanName = `{{#get "${resource}"${apiOptionsString}}} ${data.member ? 'member' : 'public'}`;
const result = await Sentry.startSpan({
op: 'frontend.helpers.get',
name: spanName,
tags: {
resource,
...apiOptions,
context: data.member ? 'member' : 'public'
}
}, async (span) => {
const response = await makeAPICall(resource, controllerName, action, apiOptions);
// prepare data properties for use with handlebars
if (response[resource] && response[resource].length) {
response[resource].forEach(prepareContextResource);
}
// used for logging details of slow requests
returnedRowsCount = response[resource] && response[resource].length;
// block params allows the theme developer to name the data using something like
// `{{#get "posts" as |result pageInfo|}}`
const blockParams = [response[resource]];
if (response.meta && response.meta.pagination) {
response.pagination = response.meta.pagination;
blockParams.push(response.meta.pagination);
}
// Call the main template function
return options.fn(response, {
data: data,
blockParams: blockParams
// prepare data properties for use with handlebars
if (response[resource] && response[resource].length) {
response[resource].forEach(prepareContextResource);
}
// used for logging details of slow requests
returnedRowsCount = response[resource] && response[resource].length;
span?.setTag('returnedRows', returnedRowsCount);
// block params allows the theme developer to name the data using something like
// `{{#get "posts" as |result pageInfo|}}`
const blockParams = [response[resource]];
if (response.meta && response.meta.pagination) {
response.pagination = response.meta.pagination;
blockParams.push(response.meta.pagination);
}
// Call the main template function
return options.fn(response, {
data: data,
blockParams: blockParams
});
});
return result;
} catch (error) {
logging.error(error);
data.error = error.message;

View file

@ -57,6 +57,31 @@ const beforeSend = function (event, hint) {
}
};
const ALLOWED_HTTP_TRANSACTIONS = [
'/ghost/api', // any Ghost API call
'/members/api', // any Members API call
'/:slug', // any frontend post/page
'/author', // any frontend author page
'/tag' // any frontend tag page
].map((path) => {
// Sentry names HTTP transactions like: "<HTTP_METHOD> <PATH>" i.e. "GET /ghost/api/content/settings"
// Match any of the paths above with any HTTP method, and also the homepage "GET /"
return new RegExp(`^(GET|POST|PUT|DELETE)\\s(?<path>${path}\\/.+|\\/$)`);
});
const beforeSendTransaction = function (event) {
// Drop transactions that are not in the allowed list
for (const transaction of ALLOWED_HTTP_TRANSACTIONS) {
const match = event.transaction.match(transaction);
if (match?.groups?.path) {
return event;
}
}
return null;
};
if (sentryConfig && !sentryConfig.disabled) {
const Sentry = require('@sentry/node');
const version = require('@tryghost/version').full;

View file

@ -155,4 +155,30 @@ describe('UNIT: sentry', function () {
assert.deepEqual(result, expected);
});
});
describe('beforeSendTransaction', function () {
it('filters transactions based on an allow list', function () {
sentry = require('../../../core/shared/sentry');
const beforeSendTransaction = sentry. beforeSendTransaction;
const allowedTransactions = [
{transaction: 'GET /ghost/api/settings'},
{transaction: 'PUT /members/api/member'},
{transaction: 'POST /ghost/api/tiers'},
{transaction: 'DELETE /members/api/member'},
{transaction: 'GET /'},
{transaction: 'GET /:slug/options(edit)?/'},
{transaction: 'GET /author/:slug'},
{transaction: 'GET /tag/:slug'}
];
allowedTransactions.forEach((transaction) => {
assert.equal(beforeSendTransaction(transaction), transaction);
});
assert.equal(beforeSendTransaction({transaction: 'GET /foo/bar'}), null);
assert.equal(beforeSendTransaction({transaction: 'Some other transaction'}), null);
});
});
});