From f11ba94603eb487f0c42a952b62463b740489e96 Mon Sep 17 00:00:00 2001 From: "Jens L." Date: Sun, 11 May 2025 02:40:31 +0200 Subject: [PATCH] root: improve sentry distributed tracing (#14468) * core: include all sentry headers Signed-off-by: Jens Langhammer * remove spotlight patch we dont need anymore Signed-off-by: Jens Langhammer * always trace in debug Signed-off-by: Jens Langhammer * init sentry earlier Signed-off-by: Jens Langhammer * re-add light interface https://github.com/goauthentik/authentik/pull/14331 removes 2 unneeded API calls Signed-off-by: Jens Langhammer * sentry integrated router Signed-off-by: Jens Langhammer * use new Sentry middleware to propagate headers Signed-off-by: Jens Langhammer * fix missing baggage Signed-off-by: Jens Langhammer * cleanup logs Signed-off-by: Jens Langhammer * use sanitized URLs for logging/tracing Signed-off-by: Jens Langhammer --------- Signed-off-by: Jens Langhammer --- authentik/brands/utils.py | 8 +- authentik/core/templates/base/skeleton.html | 4 +- authentik/lib/sentry.py | 16 +- web/package-lock.json | 1 - web/package.json | 1 - .../client/ESBuildObserver.js | 16 +- web/scripts/patch-spotlight.sh | 33 ---- .../admin/AdminInterface/index.entrypoint.ts | 3 +- web/src/common/api/config.ts | 11 +- web/src/common/sentry.ts | 155 +++++++++++------- web/src/elements/Interface/Interface.ts | 19 ++- web/src/elements/Interface/index.ts | 4 +- web/src/elements/router/RouteMatch.ts | 22 ++- web/src/elements/router/RouterOutlet.ts | 46 +++++- web/src/flow/FlowExecutor.ts | 2 +- .../standalone/loading/index.entrypoint.ts | 14 +- web/src/user/index.entrypoint.ts | 2 +- 17 files changed, 203 insertions(+), 154 deletions(-) delete mode 100644 web/scripts/patch-spotlight.sh diff --git a/authentik/brands/utils.py b/authentik/brands/utils.py index 6c68a04998..8f8fdf5e9b 100644 --- a/authentik/brands/utils.py +++ b/authentik/brands/utils.py @@ -5,10 +5,10 @@ from typing import Any from django.db.models import F, Q from django.db.models import Value as V from django.http.request import HttpRequest -from sentry_sdk import get_current_span from authentik import get_full_version from authentik.brands.models import Brand +from authentik.lib.sentry import get_http_meta from authentik.tenants.models import Tenant _q_default = Q(default=True) @@ -32,13 +32,9 @@ def context_processor(request: HttpRequest) -> dict[str, Any]: """Context Processor that injects brand object into every template""" brand = getattr(request, "brand", DEFAULT_BRAND) tenant = getattr(request, "tenant", Tenant()) - trace = "" - span = get_current_span() - if span: - trace = span.to_traceparent() return { "brand": brand, "footer_links": tenant.footer_links, - "sentry_trace": trace, + "html_meta": {**get_http_meta()}, "version": get_full_version(), } diff --git a/authentik/core/templates/base/skeleton.html b/authentik/core/templates/base/skeleton.html index 0c3a79122b..13a9ac96c4 100644 --- a/authentik/core/templates/base/skeleton.html +++ b/authentik/core/templates/base/skeleton.html @@ -21,7 +21,9 @@ {% block head %} {% endblock %} - + {% for key, value in html_meta.items %} + + {% endfor %} {% block body %} diff --git a/authentik/lib/sentry.py b/authentik/lib/sentry.py index 9f6793df4a..2d51a5eb33 100644 --- a/authentik/lib/sentry.py +++ b/authentik/lib/sentry.py @@ -17,7 +17,7 @@ from ldap3.core.exceptions import LDAPException from redis.exceptions import ConnectionError as RedisConnectionError from redis.exceptions import RedisError, ResponseError from rest_framework.exceptions import APIException -from sentry_sdk import HttpTransport +from sentry_sdk import HttpTransport, get_current_scope from sentry_sdk import init as sentry_sdk_init from sentry_sdk.api import set_tag from sentry_sdk.integrations.argv import ArgvIntegration @@ -27,6 +27,7 @@ from sentry_sdk.integrations.redis import RedisIntegration from sentry_sdk.integrations.socket import SocketIntegration from sentry_sdk.integrations.stdlib import StdlibIntegration from sentry_sdk.integrations.threading import ThreadingIntegration +from sentry_sdk.tracing import BAGGAGE_HEADER_NAME, SENTRY_TRACE_HEADER_NAME from structlog.stdlib import get_logger from websockets.exceptions import WebSocketException @@ -95,6 +96,8 @@ def traces_sampler(sampling_context: dict) -> float: return 0 if _type == "websocket": return 0 + if CONFIG.get_bool("debug"): + return 1 return float(CONFIG.get("error_reporting.sample_rate", 0.1)) @@ -167,3 +170,14 @@ def before_send(event: dict, hint: dict) -> dict | None: if settings.DEBUG: return None return event + + +def get_http_meta(): + """Get sentry-related meta key-values""" + scope = get_current_scope() + meta = { + SENTRY_TRACE_HEADER_NAME: scope.get_traceparent() or "", + } + if bag := scope.get_baggage(): + meta[BAGGAGE_HEADER_NAME] = bag.serialize() + return meta diff --git a/web/package-lock.json b/web/package-lock.json index 9e778c8fdc..8ff41e4c3b 100644 --- a/web/package-lock.json +++ b/web/package-lock.json @@ -7,7 +7,6 @@ "": { "name": "@goauthentik/web", "version": "0.0.0", - "hasInstallScript": true, "license": "MIT", "workspaces": [ ".", diff --git a/web/package.json b/web/package.json index 96ac4cd051..c21f2409e8 100644 --- a/web/package.json +++ b/web/package.json @@ -19,7 +19,6 @@ "lint:precommit": "wireit", "lint:types": "wireit", "lit-analyse": "wireit", - "postinstall": "bash scripts/patch-spotlight.sh", "precommit": "wireit", "prettier": "wireit", "prettier-check": "wireit", diff --git a/web/packages/esbuild-plugin-live-reload/client/ESBuildObserver.js b/web/packages/esbuild-plugin-live-reload/client/ESBuildObserver.js index b4f90b54a8..2f8ebe3ce2 100644 --- a/web/packages/esbuild-plugin-live-reload/client/ESBuildObserver.js +++ b/web/packages/esbuild-plugin-live-reload/client/ESBuildObserver.js @@ -6,7 +6,7 @@ * @import { Message as ESBuildMessage } from "esbuild"; */ -const logPrefix = "👷 [ESBuild]"; +const logPrefix = "authentik/dev/web: "; const log = console.debug.bind(console, logPrefix); /** @@ -76,7 +76,7 @@ export class ESBuildObserver extends EventSource { */ #startListener = () => { this.#trackActivity(); - log("⏰ Build started..."); + log("⏰ Build started..."); }; #internalErrorListener = () => { @@ -86,7 +86,7 @@ export class ESBuildObserver extends EventSource { clearTimeout(this.#keepAliveInterval); this.close(); - log("⛔️ Closing connection"); + log("⛔️ Closing connection"); } }; @@ -126,13 +126,13 @@ export class ESBuildObserver extends EventSource { this.#trackActivity(); if (!this.online) { - log("🚫 Build finished while offline."); + log("🚫 Build finished while offline."); this.deferredReload = true; return; } - log("🛎️ Build completed! Reloading..."); + log("🛎️ Build completed! Reloading..."); // We use an animation frame to keep the reload from happening before the // event loop has a chance to process the message. @@ -189,13 +189,13 @@ export class ESBuildObserver extends EventSource { if (!this.deferredReload) return; - log("🛎️ Reloading after offline build..."); + log("🛎️ Reloading after offline build..."); this.deferredReload = false; window.location.reload(); }); - log("🛎️ Listening for build changes..."); + log("🛎️ Listening for build changes..."); this.#keepAliveInterval = setInterval(() => { const now = Date.now(); @@ -203,7 +203,7 @@ export class ESBuildObserver extends EventSource { if (now - this.lastUpdatedAt < 10_000) return; this.alive = false; - log("👋 Waiting for build to start..."); + log("👋 Waiting for build to start..."); }, 15_000); } diff --git a/web/scripts/patch-spotlight.sh b/web/scripts/patch-spotlight.sh deleted file mode 100644 index a7886cf0ff..0000000000 --- a/web/scripts/patch-spotlight.sh +++ /dev/null @@ -1,33 +0,0 @@ -#!/usr/bin/env bash - -TARGET="./node_modules/@spotlightjs/overlay/dist/index-"[0-9a-f]*.js - -if [[ $(grep -L "QX2" "$TARGET" > /dev/null 2> /dev/null) ]]; then - patch --forward -V none --no-backup-if-mismatch -p0 $TARGET < { - const cfg = await config(); +let _sentryConfigured = false; - if (cfg.errorReporting.enabled) { - init({ - dsn: cfg.errorReporting.sentryDsn, - ignoreErrors: [ - /network/gi, - /fetch/gi, - /module/gi, - // Error on edge on ios, - // https://stackoverflow.com/questions/69261499/what-is-instantsearchsdkjsbridgeclearhighlight - /instantSearchSDKJSBridgeClearHighlight/gi, - // Seems to be an issue in Safari and Firefox - /MutationObserver.observe/gi, - /NS_ERROR_FAILURE/gi, - ], - release: `authentik@${VERSION}`, +export function configureSentry(canDoPpi = false) { + const cfg = globalAK().config; + const debug = cfg.capabilities.includes(CapabilitiesEnum.CanDebug); + if (!cfg.errorReporting.enabled && !debug) { + return cfg; + } + init({ + dsn: cfg.errorReporting.sentryDsn, + ignoreErrors: [ + /network/gi, + /fetch/gi, + /module/gi, + // Error on edge on ios, + // https://stackoverflow.com/questions/69261499/what-is-instantsearchsdkjsbridgeclearhighlight + /instantSearchSDKJSBridgeClearHighlight/gi, + // Seems to be an issue in Safari and Firefox + /MutationObserver.observe/gi, + /NS_ERROR_FAILURE/gi, + ], + release: `authentik@${VERSION}`, + integrations: [ + browserTracingIntegration({ + // https://docs.sentry.io/platforms/javascript/tracing/instrumentation/automatic-instrumentation/#custom-routing + instrumentNavigation: false, + instrumentPageLoad: false, + traceFetch: false, + }), + ], + tracePropagationTargets: [window.location.origin], + tracesSampleRate: debug ? 1.0 : cfg.errorReporting.tracesSampleRate, + environment: cfg.errorReporting.environment, + beforeSend: ( + event: ErrorEvent, + hint: EventHint, + ): ErrorEvent | PromiseLike | null => { + if (!hint) { + return event; + } + if (hint.originalException instanceof SentryIgnoredError) { + return null; + } + if ( + hint.originalException instanceof ResponseError || + hint.originalException instanceof DOMException + ) { + return null; + } + return event; + }, + }); + setTag(TAG_SENTRY_CAPABILITIES, cfg.capabilities.join(",")); + if (window.location.pathname.includes("if/")) { + setTag(TAG_SENTRY_COMPONENT, `web/${readInterfaceRouteParam()}`); + } + if (debug) { + Spotlight.init({ + injectImmediately: true, integrations: [ - browserTracingIntegration({ - shouldCreateSpanForRequest: (url: string) => { - return url.startsWith(window.location.host); - }, + Spotlight.sentry({ + injectIntoSDK: true, }), ], - tracesSampleRate: cfg.errorReporting.tracesSampleRate, - environment: cfg.errorReporting.environment, - beforeSend: ( - event: ErrorEvent, - hint: EventHint, - ): ErrorEvent | PromiseLike | null => { - if (!hint) { - return event; - } - if (hint.originalException instanceof SentryIgnoredError) { - return null; - } - if ( - hint.originalException instanceof ResponseError || - hint.originalException instanceof DOMException - ) { - return null; - } - return event; - }, }); - setTag(TAG_SENTRY_CAPABILITIES, cfg.capabilities.join(",")); - if (window.location.pathname.includes("if/")) { - setTag(TAG_SENTRY_COMPONENT, `web/${readInterfaceRouteParam()}`); - } - if (cfg.capabilities.includes(CapabilitiesEnum.CanDebug)) { - const Spotlight = await import("@spotlightjs/spotlight"); - - Spotlight.init({ injectImmediately: true }); - } - if (cfg.errorReporting.sendPii && canDoPpi) { - me().then((user) => { - setUser({ email: user.user.email }); - console.debug("authentik/config: Sentry with PII enabled."); - }); - } else { - console.debug("authentik/config: Sentry enabled."); - } + console.debug("authentik/config: Enabled Sentry Spotlight"); + } + if (cfg.errorReporting.sendPii && canDoPpi) { + me().then((user) => { + setUser({ email: user.user.email }); + console.debug("authentik/config: Sentry with PII enabled."); + }); + } else { + console.debug("authentik/config: Sentry enabled."); + } + _sentryConfigured = true; +} + +export class SentryMiddleware implements Middleware { + pre?(context: RequestContext): Promise { + if (!_sentryConfigured) { + return Promise.resolve(context); + } + const traceData = getTraceData(); + // @ts-ignore + context.init.headers["baggage"] = traceData["baggage"]; + // @ts-ignore + context.init.headers["sentry-trace"] = traceData["sentry-trace"]; + return Promise.resolve(context); } - return cfg; } diff --git a/web/src/elements/Interface/Interface.ts b/web/src/elements/Interface/Interface.ts index b25e2b5d48..ba07631bab 100644 --- a/web/src/elements/Interface/Interface.ts +++ b/web/src/elements/Interface/Interface.ts @@ -23,9 +23,20 @@ const configContext = Symbol("configContext"); const modalController = Symbol("modalController"); const versionContext = Symbol("versionContext"); -export abstract class Interface extends AKElement implements ThemedElement { +export abstract class LightInterface extends AKElement implements ThemedElement { protected static readonly PFBaseStyleSheet = createStyleSheetUnsafe(PFBase); + constructor() { + super(); + const styleParent = resolveStyleSheetParent(document); + + this.dataset.akInterfaceRoot = this.tagName.toLowerCase(); + + appendStyleSheet(styleParent, Interface.PFBaseStyleSheet); + } +} + +export abstract class Interface extends LightInterface implements ThemedElement { [configContext]: ConfigContextController; [modalController]: ModalOrchestrationController; @@ -38,12 +49,6 @@ export abstract class Interface extends AKElement implements ThemedElement { constructor() { super(); - const styleParent = resolveStyleSheetParent(document); - - this.dataset.akInterfaceRoot = this.tagName.toLowerCase(); - - appendStyleSheet(styleParent, Interface.PFBaseStyleSheet); - this.addController(new BrandContextController(this)); this[configContext] = new ConfigContextController(this); this[modalController] = new ModalOrchestrationController(this); diff --git a/web/src/elements/Interface/index.ts b/web/src/elements/Interface/index.ts index 3615d3855e..c9883ffec7 100644 --- a/web/src/elements/Interface/index.ts +++ b/web/src/elements/Interface/index.ts @@ -1,4 +1,4 @@ -import { AuthenticatedInterface, Interface } from "./Interface"; +import { AuthenticatedInterface, Interface, LightInterface } from "./Interface"; -export { Interface, AuthenticatedInterface }; +export { Interface, AuthenticatedInterface, LightInterface }; export default Interface; diff --git a/web/src/elements/router/RouteMatch.ts b/web/src/elements/router/RouteMatch.ts index 95657b0845..e81c911d9a 100644 --- a/web/src/elements/router/RouteMatch.ts +++ b/web/src/elements/router/RouteMatch.ts @@ -6,19 +6,35 @@ import { TemplateResult } from "lit"; export class RouteMatch { route: Route; arguments: { [key: string]: string }; - fullUrl?: string; + fullURL: string; - constructor(route: Route) { + constructor(route: Route, fullUrl: string) { this.route = route; this.arguments = {}; + this.fullURL = fullUrl; } render(): TemplateResult { return this.route.render(this.arguments); } + /** + * Convert the matched Route's URL regex to a sanitized, readable URL by replacing + * all regex values with placeholders according to the name of their regex group. + * + * @returns The sanitized URL for logging/tracing. + */ + sanitizedURL() { + let cleanedURL = this.fullURL; + for (const match of Object.keys(this.arguments)) { + const value = this.arguments[match]; + cleanedURL = cleanedURL?.replace(value, `:${match}`); + } + return cleanedURL; + } + toString(): string { - return ``; } diff --git a/web/src/elements/router/RouterOutlet.ts b/web/src/elements/router/RouterOutlet.ts index 1ef5abd087..9803feb7ee 100644 --- a/web/src/elements/router/RouterOutlet.ts +++ b/web/src/elements/router/RouterOutlet.ts @@ -3,8 +3,15 @@ import { AKElement } from "@goauthentik/elements/Base"; import { Route } from "@goauthentik/elements/router/Route"; import { RouteMatch } from "@goauthentik/elements/router/RouteMatch"; import "@goauthentik/elements/router/Router404"; +import { + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + getClient, + startBrowserTracingNavigationSpan, + startBrowserTracingPageLoadSpan, +} from "@sentry/browser"; +import { Client, Span } from "@sentry/types"; -import { CSSResult, TemplateResult, css, html } from "lit"; +import { CSSResult, PropertyValues, TemplateResult, css, html } from "lit"; import { customElement, property } from "lit/decorators.js"; // Poliyfill for hashchange.newURL, @@ -53,6 +60,9 @@ export class RouterOutlet extends AKElement { @property({ attribute: false }) routes: Route[] = []; + private sentryClient?: Client; + private pageLoadSpan?: Span; + static get styles(): CSSResult[] { return [ css` @@ -69,6 +79,15 @@ export class RouterOutlet extends AKElement { constructor() { super(); window.addEventListener("hashchange", (ev: HashChangeEvent) => this.navigate(ev)); + this.sentryClient = getClient(); + if (this.sentryClient) { + this.pageLoadSpan = startBrowserTracingPageLoadSpan(this.sentryClient, { + name: window.location.pathname, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: "url", + }, + }); + } } firstUpdated(): void { @@ -92,9 +111,8 @@ export class RouterOutlet extends AKElement { this.routes.some((route) => { const match = route.url.exec(activeUrl); if (match !== null) { - matchedRoute = new RouteMatch(route); + matchedRoute = new RouteMatch(route, activeUrl); matchedRoute.arguments = match.groups || {}; - matchedRoute.fullUrl = activeUrl; console.debug("authentik/router: found match ", matchedRoute); return true; } @@ -107,13 +125,31 @@ export class RouterOutlet extends AKElement { `; }); - matchedRoute = new RouteMatch(route); + matchedRoute = new RouteMatch(route, activeUrl); matchedRoute.arguments = route.url.exec(activeUrl)?.groups || {}; - matchedRoute.fullUrl = activeUrl; } this.current = matchedRoute; } + updated(changedProperties: PropertyValues): void { + if (!changedProperties.has("current") || !this.current) return; + if (!this.sentryClient) return; + // https://docs.sentry.io/platforms/javascript/tracing/instrumentation/automatic-instrumentation/#custom-routing + if (this.pageLoadSpan) { + this.pageLoadSpan.updateName(this.current.sanitizedURL()); + this.pageLoadSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, "route"); + this.pageLoadSpan = undefined; + } else { + startBrowserTracingNavigationSpan(this.sentryClient, { + op: "navigation", + name: this.current.sanitizedURL(), + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: "route", + }, + }); + } + } + render(): TemplateResult | undefined { return this.current?.render(); } diff --git a/web/src/flow/FlowExecutor.ts b/web/src/flow/FlowExecutor.ts index 7d051495e1..5c80f935a4 100644 --- a/web/src/flow/FlowExecutor.ts +++ b/web/src/flow/FlowExecutor.ts @@ -171,6 +171,7 @@ export class FlowExecutor extends Interface implements StageHost { } constructor() { + configureSentry(); super(); this.ws = new WebsocketClient(); const inspector = new URL(window.location.toString()).searchParams.get("inspector"); @@ -237,7 +238,6 @@ export class FlowExecutor extends Interface implements StageHost { } async firstUpdated(): Promise { - configureSentry(); if (this.config?.capabilities.includes(CapabilitiesEnum.CanDebug)) { this.inspectorAvailable = true; } diff --git a/web/src/standalone/loading/index.entrypoint.ts b/web/src/standalone/loading/index.entrypoint.ts index 2f2c5c8a1e..92e4196768 100644 --- a/web/src/standalone/loading/index.entrypoint.ts +++ b/web/src/standalone/loading/index.entrypoint.ts @@ -1,4 +1,4 @@ -import { Interface } from "@goauthentik/elements/Interface"; +import { LightInterface } from "@goauthentik/elements/Interface"; import { msg } from "@lit/localize"; import { CSSResult, TemplateResult, css, html } from "lit"; @@ -10,7 +10,7 @@ import PFSpinner from "@patternfly/patternfly/components/Spinner/spinner.css"; import PFBase from "@patternfly/patternfly/patternfly-base.css"; @customElement("ak-loading") -export class Loading extends Interface { +export class Loading extends LightInterface { static get styles(): CSSResult[] { return [ PFBase, @@ -25,16 +25,6 @@ export class Loading extends Interface { ]; } - registerContexts(): void { - // Stub function to avoid making API requests for things we don't need. The `Interface` base class loads - // a bunch of data that is used globally by various things, however this is an interface that is shown - // very briefly and we don't need any of that data. - } - - async _initCustomCSS(): Promise { - // Stub function to avoid fetching custom CSS. - } - render(): TemplateResult { return html`