-
Notifications
You must be signed in to change notification settings - Fork 0
OBS-02: Error tracking and product analytics foundation #811
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
Changes from all commits
1917b9b
b0fade6
9de43a8
9ef72f6
24893e1
afea1eb
d133017
539462d
d5743d0
45575dc
77c9123
fdaa2a7
1681ab6
7353e16
5e83325
685ce46
b7aa139
d9bfe5d
385edd5
4f6407a
0596bb6
d6c6d82
bda148f
47322dc
68815f7
7c52d71
0c03a8f
3891fb3
7be3b44
bf60d0f
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 |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| using Microsoft.AspNetCore.Authorization; | ||
| using Microsoft.AspNetCore.Mvc; | ||
| using Taskdeck.Application.Services; | ||
|
|
||
| namespace Taskdeck.Api.Controllers; | ||
|
|
||
| /// <summary> | ||
| /// Endpoints for opt-in product telemetry event recording and client configuration. | ||
| /// All telemetry is disabled by default and requires explicit opt-in. | ||
| /// </summary> | ||
| [ApiController] | ||
| [Route("api/telemetry")] | ||
| [Authorize] | ||
| public class TelemetryController : ControllerBase | ||
| { | ||
| private readonly ITelemetryEventService _telemetryEventService; | ||
| private readonly SentrySettings _sentrySettings; | ||
| private readonly AnalyticsSettings _analyticsSettings; | ||
| private readonly TelemetrySettings _telemetrySettings; | ||
|
|
||
| public TelemetryController( | ||
| ITelemetryEventService telemetryEventService, | ||
| SentrySettings sentrySettings, | ||
| AnalyticsSettings analyticsSettings, | ||
| TelemetrySettings telemetrySettings) | ||
| { | ||
| _telemetryEventService = telemetryEventService; | ||
| _sentrySettings = sentrySettings; | ||
| _analyticsSettings = analyticsSettings; | ||
| _telemetrySettings = telemetrySettings; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Returns client-side telemetry configuration. The frontend uses this to | ||
| /// determine which integrations are available and how to initialize them. | ||
| /// DSNs and script URLs are only returned when the corresponding integration | ||
| /// is enabled. No secrets or API keys are exposed. | ||
| /// </summary> | ||
| [HttpGet("config")] | ||
| [AllowAnonymous] | ||
| public IActionResult GetConfig() | ||
| { | ||
| return Ok(new ClientTelemetryConfigResponse | ||
| { | ||
| Sentry = new SentryClientConfig | ||
| { | ||
| Enabled = _sentrySettings.Enabled, | ||
| Dsn = _sentrySettings.Enabled ? _sentrySettings.Dsn : string.Empty, | ||
| Environment = _sentrySettings.Environment, | ||
| TracesSampleRate = _sentrySettings.TracesSampleRate, | ||
| }, | ||
| Analytics = new AnalyticsClientConfig | ||
| { | ||
| Enabled = _analyticsSettings.Enabled, | ||
| Provider = _analyticsSettings.Enabled ? _analyticsSettings.Provider : string.Empty, | ||
| ScriptUrl = _analyticsSettings.Enabled ? _analyticsSettings.ScriptUrl : string.Empty, | ||
| SiteId = _analyticsSettings.Enabled ? _analyticsSettings.SiteId : string.Empty, | ||
| }, | ||
| Telemetry = new TelemetryClientConfig | ||
| { | ||
| Enabled = _telemetrySettings.Enabled, | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Records a batch of product telemetry events. Requires authentication. | ||
| /// Events are validated against the taxonomy naming convention and rejected | ||
| /// if telemetry is disabled on the server. | ||
| /// </summary> | ||
| [HttpPost("events")] | ||
| public IActionResult RecordEvents([FromBody] TelemetryBatchRequest? request) | ||
| { | ||
| if (!_telemetryEventService.IsEnabled) | ||
| { | ||
| return Ok(new { recorded = 0, message = "Telemetry is disabled on this server." }); | ||
| } | ||
|
|
||
| if (request == null || request.Events == null || request.Events.Count == 0) | ||
| { | ||
| return BadRequest(new { error = "No events provided." }); | ||
| } | ||
|
|
||
| var recorded = _telemetryEventService.RecordEvents(request.Events); | ||
| return Ok(new { recorded }); | ||
| } | ||
|
Comment on lines
+74
to
+86
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. The To improve type safety and consistency, it's better to define and use a specific response DTO for this endpoint. This aligns with the if (!_telemetryEventService.IsEnabled)
{
return Ok(new TelemetryBatchResponse { Recorded = 0, Message = "Telemetry is disabled on this server." });
}
if (request.Events == null || request.Events.Count == 0)
{
return BadRequest(new { error = "No events provided." });
}
var recorded = _telemetryEventService.RecordEvents(request.Events);
return Ok(new TelemetryBatchResponse { Recorded = recorded });
}
}
public sealed class ClientTelemetryConfigResponse
{
public SentryClientConfig Sentry { get; set; } = new();
public AnalyticsClientConfig Analytics { get; set; } = new();
public TelemetryClientConfig Telemetry { get; set; } = new();
}
public sealed class SentryClientConfig
{
public bool Enabled { get; set; }
public string Dsn { get; set; } = string.Empty;
public string Environment { get; set; } = string.Empty;
public double TracesSampleRate { get; set; }
}
public sealed class AnalyticsClientConfig
{
public bool Enabled { get; set; }
public string Provider { get; set; } = string.Empty;
public string ScriptUrl { get; set; } = string.Empty;
public string SiteId { get; set; } = string.Empty;
}
public sealed class TelemetryClientConfig
{
public bool Enabled { get; set; }
}
public sealed class TelemetryBatchRequest
{
public List<TelemetryEvent> Events { get; set; } = new();
}
public sealed class TelemetryBatchResponse
{
public int Recorded { get; set; }
public string? Message { get; set; }
} |
||
| } | ||
|
|
||
| public sealed class ClientTelemetryConfigResponse | ||
| { | ||
| public SentryClientConfig Sentry { get; set; } = new(); | ||
| public AnalyticsClientConfig Analytics { get; set; } = new(); | ||
| public TelemetryClientConfig Telemetry { get; set; } = new(); | ||
| } | ||
|
|
||
| public sealed class SentryClientConfig | ||
| { | ||
| public bool Enabled { get; set; } | ||
| public string Dsn { get; set; } = string.Empty; | ||
| public string Environment { get; set; } = string.Empty; | ||
| public double TracesSampleRate { get; set; } | ||
| } | ||
|
|
||
| public sealed class AnalyticsClientConfig | ||
| { | ||
| public bool Enabled { get; set; } | ||
| public string Provider { get; set; } = string.Empty; | ||
| public string ScriptUrl { get; set; } = string.Empty; | ||
| public string SiteId { get; set; } = string.Empty; | ||
| } | ||
|
|
||
| public sealed class TelemetryClientConfig | ||
| { | ||
| public bool Enabled { get; set; } | ||
| } | ||
|
|
||
| public sealed class TelemetryBatchRequest | ||
| { | ||
| public List<TelemetryEvent> Events { get; set; } = new(); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| using System.Text.RegularExpressions; | ||
| using Taskdeck.Application.Services; | ||
|
|
||
| namespace Taskdeck.Api.Extensions; | ||
|
|
||
| public static class SentryRegistration | ||
| { | ||
| // Patterns for PII that may leak through exception messages | ||
| private static readonly Regex EmailPattern = new( | ||
| @"[a-zA-Z0-9._%+\-]+@[a-zA-Z0-9.\-]+\.[a-zA-Z]{2,}", | ||
| RegexOptions.Compiled); | ||
|
|
||
| private static readonly Regex JwtPattern = new( | ||
| @"eyJ[a-zA-Z0-9_\-]+\.eyJ[a-zA-Z0-9_\-]+\.[a-zA-Z0-9_\-]+", | ||
| RegexOptions.Compiled); | ||
|
|
||
| /// <summary> | ||
| /// Adds Sentry error tracking when enabled via configuration. | ||
| /// Disabled by default — requires Sentry:Enabled=true and a valid DSN. | ||
| /// PII is never sent (SendDefaultPii is always forced to false). | ||
| /// </summary> | ||
| public static WebApplicationBuilder AddTaskdeckSentry( | ||
| this WebApplicationBuilder builder, | ||
| SentrySettings sentrySettings) | ||
| { | ||
| if (!sentrySettings.Enabled) | ||
| { | ||
| return builder; | ||
| } | ||
|
|
||
| if (string.IsNullOrWhiteSpace(sentrySettings.Dsn)) | ||
| { | ||
| return builder; | ||
| } | ||
|
|
||
| builder.WebHost.UseSentry(options => | ||
| { | ||
| options.Dsn = sentrySettings.Dsn; | ||
| options.Environment = sentrySettings.Environment; | ||
| options.TracesSampleRate = sentrySettings.TracesSampleRate; | ||
|
|
||
| // Hard privacy guardrail: never send PII regardless of config. | ||
| // This prevents usernames, emails, IP addresses, and request | ||
| // bodies from being included in Sentry events. | ||
| options.SendDefaultPii = false; | ||
|
|
||
| // Prevent hostname leakage | ||
| options.ServerName = string.Empty; | ||
|
|
||
| // Scrub PII from exception messages and event data before sending. | ||
| // Exception messages may contain emails, JWT tokens, or usernames | ||
| // that were interpolated into error strings. | ||
| options.SetBeforeSend((sentryEvent, _) => | ||
| { | ||
| if (sentryEvent.Message?.Formatted != null) | ||
| { | ||
| sentryEvent.Message = new Sentry.SentryMessage | ||
| { | ||
| Formatted = ScrubPii(sentryEvent.Message.Formatted) | ||
| }; | ||
| } | ||
|
|
||
| // Scrub PII from captured exception values. The Sentry SDK copies | ||
| // exception messages into SentryException objects with a Value property. | ||
| if (sentryEvent.SentryExceptions != null) | ||
| { | ||
| foreach (var sentryException in sentryEvent.SentryExceptions) | ||
| { | ||
| if (!string.IsNullOrEmpty(sentryException.Value)) | ||
| { | ||
| sentryException.Value = ScrubPii(sentryException.Value); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return sentryEvent; | ||
| }); | ||
|
|
||
| // Strip sensitive data from breadcrumbs. Sentry breadcrumb Data | ||
| // is read-only, so we filter by dropping HTTP breadcrumbs that | ||
| // carry authorization or cookie information. | ||
| options.SetBeforeBreadcrumb(breadcrumb => | ||
| { | ||
| if (breadcrumb.Category == "http" && breadcrumb.Data != null) | ||
| { | ||
| var sensitiveKeys = new HashSet<string>(StringComparer.OrdinalIgnoreCase) | ||
| { | ||
| "Authorization", "Cookie", "Set-Cookie", "X-Api-Key" | ||
| }; | ||
| foreach (var key in breadcrumb.Data.Keys) | ||
| { | ||
| if (sensitiveKeys.Contains(key)) | ||
| { | ||
| // Data contains sensitive headers — drop entire breadcrumb | ||
| // to prevent PII leakage. The breadcrumb is replaced with | ||
| // a sanitized version without data. | ||
| return new Sentry.Breadcrumb( | ||
| message: breadcrumb.Message ?? string.Empty, | ||
| type: breadcrumb.Type ?? string.Empty, | ||
| data: null, | ||
| category: breadcrumb.Category, | ||
| level: breadcrumb.Level); | ||
| } | ||
| } | ||
|
Comment on lines
+84
to
+104
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. The current implementation for stripping sensitive headers from Sentry breadcrumbs is case-sensitive and could miss headers like To ensure all sensitive headers are properly stripped, I recommend using a case-insensitive check. A if (breadcrumb.Category == "http" && breadcrumb.Data != null)
{
var sensitiveKeys = new HashSet<string>(new[] { "Authorization", "Cookie" }, StringComparer.OrdinalIgnoreCase);
if (breadcrumb.Data.Keys.Any(k => sensitiveKeys.Contains(k)))
{
// Data contains sensitive headers — drop entire breadcrumb
// to prevent PII leakage. The breadcrumb is replaced with
// a sanitized version without data.
return new Sentry.Breadcrumb(
message: breadcrumb.Message ?? string.Empty,
type: breadcrumb.Type ?? string.Empty,
data: null,
category: breadcrumb.Category,
level: breadcrumb.Level);
}
} |
||
| } | ||
|
|
||
| return breadcrumb; | ||
| }); | ||
| }); | ||
|
|
||
| return builder; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Scrubs known PII patterns (emails, JWTs) from a string. | ||
| /// </summary> | ||
| internal static string ScrubPii(string input) | ||
| { | ||
| if (string.IsNullOrEmpty(input)) return input; | ||
|
|
||
| var result = EmailPattern.Replace(input, "[email-redacted]"); | ||
| result = JwtPattern.Replace(result, "[jwt-redacted]"); | ||
| return result; | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| namespace Taskdeck.Application.Services; | ||
|
|
||
| /// <summary> | ||
| /// Configuration for self-hosted web analytics (Plausible or Umami). | ||
| /// Disabled by default — the frontend reads these settings from a config endpoint | ||
| /// and injects the analytics script only when configured. | ||
| /// </summary> | ||
| public sealed class AnalyticsSettings | ||
| { | ||
| /// <summary> | ||
| /// Master switch. Default: false (opt-in only). | ||
| /// </summary> | ||
| public bool Enabled { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Analytics provider: "plausible" or "umami". Case-insensitive. | ||
| /// </summary> | ||
| public string Provider { get; set; } = string.Empty; | ||
|
|
||
| /// <summary> | ||
| /// Full URL to the analytics script (e.g. "https://plausible.example.com/js/script.js"). | ||
| /// </summary> | ||
| public string ScriptUrl { get; set; } = string.Empty; | ||
|
|
||
| /// <summary> | ||
| /// Site identifier / website ID used by the analytics provider. | ||
| /// </summary> | ||
| public string SiteId { get; set; } = string.Empty; | ||
| } |
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.
RecordEventsdoesn't defensively handle anullrequest body (e.g. client sends JSONnull), which would throw when accessingrequest.Events. With[ApiController]you often get an automatic 400, but it's safer to acceptTelemetryBatchRequest?and explicitly returnBadRequestwhenrequestis null.