From d05745e72c5889f5590813ade55e406b6235437f Mon Sep 17 00:00:00 2001 From: tsan88 Date: Fri, 4 Sep 2026 15:34:17 +0700 Subject: [PATCH] fix(sentry): accept ISO timestamps and non-string span data in nested items Nested envelope structures declared their timestamps as json.Number while the top-level ErrorEvent already used FlexibleTS. PHP/Laravel SDKs send ISO 8601 strings in breadcrumbs, transactions, spans and logs, so unmarshalling failed with "invalid number literal, trying to unmarshal ... into Number" and the entire event was dropped (the request still answered 200). RawSpan.Data had the same class of problem: declared map[string]string, but SDKs put numbers and booleans there (http.response.status_code: 200), which failed with "cannot unmarshal number into Go struct field RawSpan.spans.data of type string" and dropped the transaction with all of its spans. Measured on a production stream before the fix: of 1326 envelopes accepted by the proxy for one PHP project only 62 reached the database. Breadcrumb, Transaction, RawSpan and LogRecord now use FlexibleTS, and span attributes go through a SpanData type that normalizes values to strings, so existing consumers stay unchanged. --- modules/sentry/store_error.go | 2 +- modules/sentry/store_log.go | 2 +- modules/sentry/store_transaction.go | 18 +++---- modules/sentry/types.go | 80 +++++++++++++++++++++------- modules/sentry/types_test.go | 82 +++++++++++++++++++++++++++-- 5 files changed, 150 insertions(+), 34 deletions(-) diff --git a/modules/sentry/store_error.go b/modules/sentry/store_error.go index 71ed16a..34a4d66 100644 --- a/modules/sentry/store_error.go +++ b/modules/sentry/store_error.go @@ -104,7 +104,7 @@ func storeErrorEvent(db *sql.DB, ev *ErrorEvent, payload json.RawMessage, projec if ev.Breadcrumbs != nil { for _, bc := range ev.Breadcrumbs.Values { bcID := event.GenerateUUID() - bcTS := parseTimestamp(bc.Timestamp) + bcTS := parseTimestamp(bc.Timestamp.Number()) var data *string if bc.Data != nil { diff --git a/modules/sentry/store_log.go b/modules/sentry/store_log.go index 8181c90..785ea68 100644 --- a/modules/sentry/store_log.go +++ b/modules/sentry/store_log.go @@ -48,7 +48,7 @@ func storeLogs(db *sql.DB, logs []LogRecord) error { sevNum = &v } - logTS := parseLogTimestamp(log.Timestamp) + logTS := parseLogTimestamp(log.Timestamp.Number()) _, err = stmt.Exec( id, diff --git a/modules/sentry/store_transaction.go b/modules/sentry/store_transaction.go index 0bac133..d6b0436 100644 --- a/modules/sentry/store_transaction.go +++ b/modules/sentry/store_transaction.go @@ -55,9 +55,9 @@ func storeTransaction(db *sql.DB, txn *Transaction, payload json.RawMessage) (st // Insert transaction. txnID := event.GenerateUUID() - startTS := parseTimestamp(txn.StartTime) - endTS := parseTimestamp(txn.Timestamp) - durationMS := computeDurationMS(txn.StartTime, txn.Timestamp) + startTS := parseTimestamp(txn.StartTime.Number()) + endTS := parseTimestamp(txn.Timestamp.Number()) + durationMS := computeDurationMS(txn.StartTime.Number(), txn.Timestamp.Number()) op := txn.Op status := txn.Status @@ -111,9 +111,9 @@ func storeTransaction(db *sql.DB, txn *Transaction, payload json.RawMessage) (st spanUUID := event.GenerateUUID() peerType, peerAddress := classifySpan(span) serviceName := extractServiceName(span, txn.SDK) - sStartTS := parseTimestamp(span.StartTimestamp) - sEndTS := parseTimestamp(span.Timestamp) - sDuration := computeDurationMS(span.StartTimestamp, span.Timestamp) + sStartTS := parseTimestamp(span.StartTimestamp.Number()) + sEndTS := parseTimestamp(span.Timestamp.Number()) + sDuration := computeDurationMS(span.StartTimestamp.Number(), span.Timestamp.Number()) isError := 0 if span.Status == "internal_error" || span.Status == "unknown_error" { @@ -213,9 +213,9 @@ func storeSpansV2(db *sql.DB, spans []RawSpan, sdk *SDK) error { spanUUID := event.GenerateUUID() peerType, peerAddress := classifySpan(span) serviceName := extractServiceName(span, sdk) - sStartTS := parseTimestamp(span.StartTimestamp) - sEndTS := parseTimestamp(span.Timestamp) - sDuration := computeDurationMS(span.StartTimestamp, span.Timestamp) + sStartTS := parseTimestamp(span.StartTimestamp.Number()) + sEndTS := parseTimestamp(span.Timestamp.Number()) + sDuration := computeDurationMS(span.StartTimestamp.Number(), span.Timestamp.Number()) isError := 0 if span.Status == "internal_error" || span.Status == "unknown_error" { diff --git a/modules/sentry/types.go b/modules/sentry/types.go index 309a93a..4eb288a 100644 --- a/modules/sentry/types.go +++ b/modules/sentry/types.go @@ -176,7 +176,7 @@ type Breadcrumb struct { Category string `json:"category"` Level string `json:"level"` Message string `json:"message"` - Timestamp json.Number `json:"timestamp"` + Timestamp FlexibleTS `json:"timestamp"` Data json.RawMessage `json:"data"` } @@ -199,8 +199,8 @@ type Transaction struct { EventID string `json:"event_id"` Type string `json:"type"` Transaction string `json:"transaction"` - Timestamp json.Number `json:"timestamp"` - StartTime json.Number `json:"start_timestamp"` + Timestamp FlexibleTS `json:"timestamp"` + StartTime FlexibleTS `json:"start_timestamp"` Platform string `json:"platform"` Environment string `json:"environment"` Release string `json:"release"` @@ -216,16 +216,16 @@ type Transaction struct { // RawSpan represents a span within a transaction or spans envelope item. type RawSpan struct { - SpanID string `json:"span_id"` - ParentSpanID string `json:"parent_span_id"` - TraceID string `json:"trace_id"` - Op string `json:"op"` - Description string `json:"description"` - Status string `json:"status"` - StartTimestamp json.Number `json:"start_timestamp"` - Timestamp json.Number `json:"timestamp"` - IsSegment bool `json:"is_segment"` - Data map[string]string `json:"data"` + SpanID string `json:"span_id"` + ParentSpanID string `json:"parent_span_id"` + TraceID string `json:"trace_id"` + Op string `json:"op"` + Description string `json:"description"` + Status string `json:"status"` + StartTimestamp FlexibleTS `json:"start_timestamp"` + Timestamp FlexibleTS `json:"timestamp"` + IsSegment bool `json:"is_segment"` + Data SpanData `json:"data"` } // SpansEnvelope represents a Sentry spans (v2) envelope item body. @@ -240,13 +240,13 @@ type LogEnvelope struct { // LogRecord represents a single Sentry native log entry. type LogRecord struct { - TraceID string `json:"trace_id"` - SpanID string `json:"span_id"` - Level string `json:"level"` - SeverityNumber int `json:"severity_number"` - Body string `json:"body"` - Timestamp json.Number `json:"timestamp"` - Attributes map[string]any `json:"attributes"` + TraceID string `json:"trace_id"` + SpanID string `json:"span_id"` + Level string `json:"level"` + SeverityNumber int `json:"severity_number"` + Body string `json:"body"` + Timestamp FlexibleTS `json:"timestamp"` + Attributes map[string]any `json:"attributes"` } // effectiveMessage returns the event message, checking logentry fallback. @@ -275,3 +275,43 @@ func (e *ErrorEvent) spanID() string { } return "" } + +// SpanData holds span attributes. SDKs put more than strings in there: numbers +// and booleans as well (http.response.status_code: 200, http.request.redirect: +// false). With the declared map[string]string a single numeric attribute made +// the whole envelope item fail to unmarshal, so the transaction and all of its +// spans were dropped. Values are normalized to strings, which keeps existing +// consumers (classifySpan, extractServiceName, the spans table) unchanged. +type SpanData map[string]string + +func (d *SpanData) UnmarshalJSON(data []byte) error { + var raw map[string]any + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + + out := make(SpanData, len(raw)) + for k, v := range raw { + switch t := v.(type) { + case nil: + out[k] = "" + case string: + out[k] = t + case bool: + out[k] = strconv.FormatBool(t) + case json.Number: + out[k] = t.String() + case float64: + out[k] = strconv.FormatFloat(t, 'f', -1, 64) + default: + // Objects and arrays: keep the raw JSON rather than losing the value. + b, err := json.Marshal(t) + if err != nil { + return err + } + out[k] = string(b) + } + } + *d = out + return nil +} diff --git a/modules/sentry/types_test.go b/modules/sentry/types_test.go index 6be2066..2caaa92 100644 --- a/modules/sentry/types_test.go +++ b/modules/sentry/types_test.go @@ -14,9 +14,9 @@ func TestBreadcrumbList_AcceptsBothShapes(t *testing.T) { want int }{ "object form": {`{"values":[{"category":"console"},{"category":"ui.click"}]}`, 2}, - "array form": {`[{"category":"console"},{"category":"ui.click"},{"category":"navigation"}]`, 3}, - "null": {`null`, 0}, - "empty array": {`[]`, 0}, + "array form": {`[{"category":"console"},{"category":"ui.click"},{"category":"navigation"}]`, 3}, + "null": {`null`, 0}, + "empty array": {`[]`, 0}, } for name, tc := range cases { @@ -59,3 +59,79 @@ func TestErrorEvent_SvelteKitBrowserPayload(t *testing.T) { t.Fatalf("exception not parsed: %+v", ev.Exception) } } + +// Nested envelope structures get ISO timestamps too, not just the top-level +// error event: PHP/Laravel SDKs put "2026-09-03T11:04:16.035Z" into breadcrumbs +// and transactions. With json.Number there, unmarshalling failed with +// `invalid number literal, trying to unmarshal "2026-..." into Number` and the +// whole event was dropped. +func TestFlexibleTS_NestedStructures(t *testing.T) { + t.Run("breadcrumb", func(t *testing.T) { + var bc Breadcrumb + if err := json.Unmarshal([]byte(`{"category":"query","timestamp":"2026-09-03T11:04:16.035Z"}`), &bc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if got := bc.Timestamp.Number().String(); got == "" { + t.Fatal("timestamp not normalized") + } + }) + + t.Run("transaction", func(t *testing.T) { + var txn Transaction + payload := `{"type":"transaction","transaction":"GET /","start_timestamp":"2026-09-03T11:04:16.035Z","timestamp":"2026-09-03T11:04:17.135Z"}` + if err := json.Unmarshal([]byte(payload), &txn); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if computeDurationMS(txn.StartTime.Number(), txn.Timestamp.Number()) == nil { + t.Fatal("duration not computed from ISO timestamps") + } + }) + + t.Run("log record", func(t *testing.T) { + var log LogRecord + if err := json.Unmarshal([]byte(`{"body":"hi","timestamp":"2026-09-03T11:04:16Z"}`), &log); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if log.Timestamp.Number().String() == "" { + t.Fatal("timestamp not normalized") + } + }) +} + +// Span attributes are not all strings: SDKs send numbers and booleans, and with +// map[string]string the whole spans/transaction item was rejected with +// `cannot unmarshal number into Go struct field RawSpan.spans.data of type string`. +func TestSpanData_AcceptsNonStringValues(t *testing.T) { + var span RawSpan + payload := `{ + "span_id":"a1b2c3d4e5f60718","trace_id":"7f0c8f5c9b2a4d1e8f3b6c5a4d2e1f09", + "op":"http.client","start_timestamp":1774960590.1,"timestamp":1774960590.9, + "data":{ + "http.response.status_code":200, + "http.request.redirect_count":0, + "server.address":"api.example.com", + "cache.hit":true, + "sentry.sample_rate":0.25, + "http.response.header":null, + "custom.tags":["a","b"] + } + }` + if err := json.Unmarshal([]byte(payload), &span); err != nil { + t.Fatalf("span with non-string data failed to parse: %v", err) + } + + want := map[string]string{ + "http.response.status_code": "200", + "http.request.redirect_count": "0", + "server.address": "api.example.com", + "cache.hit": "true", + "sentry.sample_rate": "0.25", + "http.response.header": "", + "custom.tags": `["a","b"]`, + } + for k, v := range want { + if span.Data[k] != v { + t.Errorf("data[%q] = %q, want %q", k, span.Data[k], v) + } + } +}