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
feat(core): Add default ignoreTransactions for Healthchecks #8191
Changes from 1 commit
ed248df
5628289
47f8849
898e514
c063be3
7040d86
979f3ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,16 @@ import { getEventDescription, logger, stringMatchesSomePattern } from '@sentry/u | |
// this is the result of a script being pulled in from an external domain and CORS. | ||
const DEFAULT_IGNORE_ERRORS = [/^Script error\.?$/, /^Javascript error: Script error\.? on line 0$/]; | ||
|
||
const DEFAULT_IGNORE_TRANSACTIONS = [ | ||
/^.*healthcheck.*$/, | ||
/^.*healthy.*$/, | ||
/^.*live.*$/, | ||
/^.*ready.*$/, | ||
/^.*heartbeat.*$/, | ||
/^.*\/health$/, | ||
/^.*\/healthz$/ | ||
]; | ||
|
||
/** Options for the InboundFilters integration */ | ||
export interface InboundFiltersOptions { | ||
allowUrls: Array<string | RegExp>; | ||
|
@@ -26,7 +36,7 @@ export class InboundFilters implements Integration { | |
*/ | ||
public name: string = InboundFilters.id; | ||
|
||
public constructor(private readonly _options: Partial<InboundFiltersOptions> = {}) {} | ||
public constructor(private readonly _options: Partial<InboundFiltersOptions> = {}) { } | ||
|
||
/** | ||
* @inheritDoc | ||
|
@@ -64,7 +74,11 @@ export function _mergeOptions( | |
...(clientOptions.ignoreErrors || []), | ||
...DEFAULT_IGNORE_ERRORS, | ||
], | ||
ignoreTransactions: [...(internalOptions.ignoreTransactions || []), ...(clientOptions.ignoreTransactions || [])], | ||
ignoreTransactions: [ | ||
...(internalOptions.ignoreTransactions || []), | ||
...(clientOptions.ignoreTransactions || []), | ||
...DEFAULT_IGNORE_TRANSACTIONS, | ||
], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we add them like this, we don't give users the possibility to opt-out of ignoring health checks. I see two options:
wdyt? Although it's probably worth mentioning that even if we allow opt-in, users would need to enable this in two places - the SDK and the UI. Is this good UX? |
||
ignoreInternal: internalOptions.ignoreInternal !== undefined ? internalOptions.ignoreInternal : true, | ||
}; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw, I've also seen
ping
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we decided to keep consistency with https://docs.sentry.io/product/performance/performance-at-scale/#health-checks
but we can also change it there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
had a customer email me today that
ping
was 13 TPM while the rest were all .01 TPM and below. said it ate up their quota.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was python