Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions cmd/thv/app/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,8 @@ var serveCmd = &cobra.Command{
env = os.Getenv("SENTRY_ENVIRONMENT")
}

// Initialize Sentry for error reporting and panic capture.
// Must happen before telemetry.NewServeProvider so the Sentry span
// processor is registered in time to be picked up by NewProvider.
// Initialize Sentry for error reporting and trace export. This must happen
// before telemetry.NewServeProvider so its trace exporter is registered.
sentryCfg := sentrypkg.Config{
DSN: dsn,
Environment: env,
Expand All @@ -72,18 +71,16 @@ var serveCmd = &cobra.Command{
}

// Initialize OTEL provider from global config (thv config otel set-endpoint).
// If Sentry is also initialized, the Sentry span processor is wired in so spans
// are exported to both the configured OTLP backend and Sentry simultaneously.
// When Sentry is initialized, its trace exporter is added as a span processor,
// so spans reach both the configured OTLP backend and Sentry.
otelProvider, otelEnabled, err := telemetry.NewServeProvider(ctx)
if err != nil {
return err
}

// Shutdown ordering is intentionally LIFO via defer:
// 1. OTEL provider shuts down first — flushes the Sentry span processor
// (which calls hub.Flush internally) before the Sentry client is closed.
// 2. Sentry client closes second — safe because the span processor has
// already flushed by the time sentrypkg.Close() runs.
// 1. OTEL provider shuts down first, flushing the Sentry trace exporter.
// 2. Sentry client closes second, after trace export has completed.
// Using defer instead of a goroutine makes the ordering deterministic.
if otelProvider != nil {
defer func() {
Expand Down
6 changes: 4 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ require (
sigs.k8s.io/yaml v1.6.0
)

require github.com/getsentry/sentry-go/otel v0.44.1
require github.com/getsentry/sentry-go/otel v0.49.0

require github.com/getsentry/sentry-go/otel/otlp v0.49.0

require github.com/hashicorp/golang-lru/v2 v2.0.7

Expand Down Expand Up @@ -169,7 +171,7 @@ require (
github.com/fatih/color v1.18.0 // indirect
github.com/fsnotify/fsnotify v1.9.0 // indirect
github.com/fxamacker/cbor/v2 v2.9.0 // indirect
github.com/getsentry/sentry-go v0.47.0
github.com/getsentry/sentry-go v0.49.0
github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect
github.com/go-logr/zapr v1.3.0 // indirect
github.com/go-ole/go-ole v1.3.0 // indirect
Expand Down
10 changes: 6 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,12 @@ github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S
github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0=
github.com/fxamacker/cbor/v2 v2.9.0 h1:NpKPmjDBgUfBms6tr6JZkTHtfFGcMKsw3eGcmD/sapM=
github.com/fxamacker/cbor/v2 v2.9.0/go.mod h1:vM4b+DJCtHn+zz7h3FFp/hDAI9WNWCsZj23V5ytsSxQ=
github.com/getsentry/sentry-go v0.47.0 h1:AnSMSyrYA5qZCIN/2xpgAAwv63sVULV+vBq37ajouc8=
github.com/getsentry/sentry-go v0.47.0/go.mod h1:h+b4VHpKnK7aUXB5wc+KDnPgp9ZtfliRD4eV85FbiSA=
github.com/getsentry/sentry-go/otel v0.44.1 h1:RV2zUHEvGHJmCCpMaJ52tZZAlcbMgvtasQn/g3CcKKc=
github.com/getsentry/sentry-go/otel v0.44.1/go.mod h1:CfzTxocQJ6JX4SLFvnBrGULBAARFAd1fHmbJCTQlOP4=
github.com/getsentry/sentry-go v0.49.0 h1:Ehejknu1l023Ub7QoRBVLAI7g3Jnhqku4oWx4B4Sh5s=
github.com/getsentry/sentry-go v0.49.0/go.mod h1:nuMJAoCfe1u0Bts2ocyNI+TW8HT84vRMqwA5Qq/SKUI=
github.com/getsentry/sentry-go/otel v0.49.0 h1:BRMzf4PqYEGsgxNS8BMX54I0DYVcBWbCtQ1s+aS67n0=
github.com/getsentry/sentry-go/otel v0.49.0/go.mod h1:FVrBdl+7ofh9neiuGLYtaGwEIv8fsW8LmV62pYMIrKk=
github.com/getsentry/sentry-go/otel/otlp v0.49.0 h1:gZgeRBzBQ2utfwno7MqSGXXRLdxX+48pBjp1WDAcb00=
github.com/getsentry/sentry-go/otel/otlp v0.49.0/go.mod h1:xq+r0C0F5T6JZwNOud7168JZLXXfPJ6jT0Y0vkrhAds=
github.com/github/smimesign v0.2.0 h1:Hho4YcX5N1I9XNqhq0fNx0Sts8MhLonHd+HRXVGNjvk=
github.com/github/smimesign v0.2.0/go.mod h1:iZiiwNT4HbtGRVqCQu7uJPEZCuEE5sfSSttcnePkDl4=
github.com/gkampitakis/ciinfo v0.3.2 h1:JcuOPk8ZU7nZQjdUhctuhQofk7BGHuIy0c9Ez8BNhXs=
Expand Down
3 changes: 1 addition & 2 deletions pkg/api/errors/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ func ErrorHandler(fn HandlerWithError) http.HandlerFunc {
// runtimes that may include connection strings) to external backends.
span.RecordError(fmt.Errorf("internal server error"))
span.SetStatus(codes.Error, "internal server error")
// Sentry span processor only creates transactions; call CaptureException
// explicitly so 5xx errors also appear as Issues in the Sentry Issues tab.
// Capture the exception explicitly so 5xx errors also appear as Sentry Issues.
sentrypkg.CaptureException(r, err)

if isUpstreamStatus(code) {
Expand Down
164 changes: 153 additions & 11 deletions pkg/sentry/sentry.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@
package sentry

import (
"context"
"fmt"
"log/slog"
"net/http"
"sync"
"sync/atomic"
"time"

"github.com/getsentry/sentry-go"
sentryotel "github.com/getsentry/sentry-go/otel"
sentryotlp "github.com/getsentry/sentry-go/otel/otlp"
sdktrace "go.opentelemetry.io/otel/sdk/trace"

"github.com/stacklok/toolhive/pkg/telemetry"
"github.com/stacklok/toolhive/pkg/updates"
Expand All @@ -21,9 +25,30 @@ import (

const flushTimeout = 2 * time.Second

const (
// environmentKey and releaseKey are the attribute names Sentry itself uses
// to carry Environment and Release on OTLP payloads (see sentry-go's
// log.go/metrics.go), so exported spans must use the same names to be
// grouped alongside Issues.
environmentKey = "sentry.environment"
releaseKey = "sentry.release"
// instanceIDKey carries the anonymous instance ID on both Sentry events
// (as a scope tag) and exported spans (as a resource attribute), so Issues
// and Traces can be correlated with toolhive-studio by the same value.
instanceIDKey = "custom.instance_id"
)

// initialized tracks whether Sentry was successfully initialized.
var initialized atomic.Bool

// spanProcessor is the single Sentry OTLP span processor for this process,
// created on the first Init and reused by every subsequent one. Guarded by
// spanProcessorMu.
var (
spanProcessorMu sync.Mutex
spanProcessor sdktrace.SpanProcessor
)

// Config holds the configuration for Sentry integration.
type Config struct {
// DSN is the Sentry Data Source Name. When empty, Sentry is disabled.
Expand All @@ -46,21 +71,30 @@ func Init(cfg Config) error {
}

vi := versions.GetVersionInfo()
// Reused verbatim as a span resource attribute below so Issues and Traces
// report the same release string.
release := fmt.Sprintf("toolhive@%s", vi.Version)

err := sentry.Init(sentry.ClientOptions{
Dsn: cfg.DSN,
Environment: cfg.Environment,
Release: fmt.Sprintf("toolhive@%s", vi.Version),
Release: release,
TracesSampleRate: cfg.TracesSampleRate,
Debug: cfg.Debug,
EnableTracing: true,
AttachStacktrace: true,
SendDefaultPII: false,
DataCollection: noPIIDataCollection(),
Integrations: func(integrations []sentry.Integration) []sentry.Integration {
return append(integrations, sentryotel.NewOtelIntegration())
},
})
if err != nil {
return fmt.Errorf("sentry init: %w", err)
}

if err := registerTraceExporter(cfg); err != nil {
return err
}
initialized.Store(true)
slog.Debug("sentry initialized", "environment", cfg.Environment)

Expand All @@ -69,22 +103,121 @@ func Init(cfg Config) error {
// toolhive-studio. Note: toolhive-studio currently uses "custom.user_id"
// for the same value; these should be aligned to "custom.instance_id" in
// both repos in a follow-up to avoid misleading PII detection heuristics.
instanceID := ""
if id, err := updates.TryGetAnonymousID(); err == nil && id != "" {
instanceID = id
sentry.ConfigureScope(func(scope *sentry.Scope) {
scope.SetTag("custom.instance_id", id)
scope.SetTag(instanceIDKey, id)
})
slog.Debug("sentry anonymous instance ID tagged", "id", id)
}

// Self-register the Sentry span processor with the global OTEL registry so
// that any telemetry.NewProvider call automatically includes it. This decouples
// the OTEL provider setup from Sentry-specific code.
telemetry.RegisterSpanProcessor(sentryotel.NewSentrySpanProcessor())
slog.Debug("sentry span processor registered with OTEL registry")
// Spans are exported straight to Sentry's OTLP endpoint and never pass
// through the Sentry client, so neither ClientOptions nor the scope
// configured above reach them. Environment, release and instance ID have to
// travel as OTEL resource attributes instead, or Traces would lose the
// grouping that Issues keep and the two would disagree.
telemetry.RegisterResourceAttributes(resourceAttributes(cfg.Environment, release, instanceID))

return nil
}

// registerTraceExporter registers the Sentry OTLP span processor with the global
// OTEL registry, creating it on first use.
//
// The processor is cached because the registry deduplicates by pointer identity:
// sdktrace.NewBatchSpanProcessor allocates a fresh processor on every call, so
// without this a second Init (config reload, or a test that does not reset the
// registry) would register a second processor and double-export every span while
// leaking the first exporter's goroutine.
//
// Caching means a second Init keeps the first call's DSN. thv serve calls Init
// exactly once per process, and the registry already only feeds processors to
// providers created after registration, so re-initialising is not supported
// either way.
func registerTraceExporter(cfg Config) error {
spanProcessorMu.Lock()
defer spanProcessorMu.Unlock()

if spanProcessor == nil {
exporter, err := sentryotlp.NewTraceExporter(context.Background(), cfg.DSN)
if err != nil {
return fmt.Errorf("create Sentry trace exporter: %w", err)
}
spanProcessor = sdktrace.NewBatchSpanProcessor(exporter)
}

telemetry.RegisterSpanProcessor(spanProcessor)
// Spans no longer pass through the Sentry client, so TracesSampleRate has to
// reach the OTEL sampler or --sentry-traces-sample-rate would be ignored and
// every span would be exported.
telemetry.RegisterSamplingRate(cfg.TracesSampleRate)
slog.Debug("sentry trace exporter registered with OTEL registry",
"traces_sample_rate", cfg.TracesSampleRate)
return nil
}

// piiSensitiveTerms mirrors the deny-list that sentry-go applies internally for
// SendDefaultPII=false (its unexported extendedSensitiveTerms). These cover
// client-identifying data that an API server behind a proxy routinely sees:
// forwarding headers, remote addresses and user identifiers.
//
// The list has to be repeated here because sentry-go reaches it only through
// the deprecated SendDefaultPII path; the DataCollection API exposes no way to
// set it. In CollectionDenyList mode a behaviour's Terms are OR-ed with the
// SDK's built-in terms, so passing them per behaviour is equivalent.
//
// Re-check this against sentry-go's extendedSensitiveTerms on SDK upgrades — a
// term added upstream will not reach us automatically.
var piiSensitiveTerms = []string{
"forwarded",
"-ip",
"remote-",
"via",
"-user",
}

// noPIIDataCollection returns the DataCollection that replaces the deprecated
// SendDefaultPII=false. It is deliberately equivalent to what sentry-go's
// legacyDataCollection built for that flag: no auto-populated user info, no
// HTTP bodies, no cookies, and headers and query params scrubbed against both
// the built-in and the extended deny-lists.
func noPIIDataCollection() *sentry.DataCollection {
denyList := func() *sentry.KeyValueCollectionBehavior {
return &sentry.KeyValueCollectionBehavior{
Mode: sentry.CollectionDenyList,
Terms: piiSensitiveTerms,
}
}
return &sentry.DataCollection{
UserInfo: sentry.Set(false),
HTTPBodies: []sentry.BodyType{},
Cookies: &sentry.KeyValueCollectionBehavior{Mode: sentry.CollectionOff},
HTTPHeaders: &sentry.HeaderCollectionConfig{
Request: denyList(),
Response: denyList(),
},
QueryParams: denyList(),
}
}

// resourceAttributes returns the OTEL resource attributes Sentry needs to group
// OTLP-ingested traces the same way it groups Issues. Empty values are omitted
// so they do not show up as blank attributes on other OTLP backends.
func resourceAttributes(environment, release, instanceID string) map[string]string {
attrs := make(map[string]string, 3)
for key, value := range map[string]string{
environmentKey: environment,
releaseKey: release,
instanceIDKey: instanceID,
} {
if value != "" {
attrs[key] = value
}
}
return attrs
}

// Close flushes buffered Sentry events and shuts down the SDK.
// Safe to call even when Sentry was not initialized.
func Close() {
Expand All @@ -107,8 +240,9 @@ func Enabled() bool {
//
// The API server's error handler calls this alongside span.RecordError so that
// 5xx errors appear as both OTEL span errors (distributed tracing) and
// standalone Sentry Issues (error tracking). The Sentry span processor only
// creates transactions; explicit hub calls are required for Issues.
// standalone Sentry Issues (error tracking). The Sentry OTEL integration links
// those issues to the active OTEL trace; explicit hub calls are required to
// create Issues.
func CaptureException(r *http.Request, err error) {
if !initialized.Load() || err == nil {
return
Expand All @@ -117,7 +251,15 @@ func CaptureException(r *http.Request, err error) {
if hub == nil {
hub = sentry.CurrentHub().Clone()
}
hub.CaptureException(err)
client := hub.Client()
if client == nil {
return
}
event := client.EventFromException(err, sentry.LevelError)
hub.CaptureEventWithHint(event, &sentry.EventHint{
OriginalException: err,
Context: r.Context(),
})
}

// RecoverPanic reports a recovered panic value to Sentry.
Expand Down
Loading
Loading