diff --git a/api/internal/mail/mail.go b/api/internal/mail/mail.go index ac770443..ffbbd828 100644 --- a/api/internal/mail/mail.go +++ b/api/internal/mail/mail.go @@ -92,7 +92,11 @@ func NewService(cfg Config) (*Service, error) { // middleware degrades to a cheap pass-through. We wrap the leaf transport so // every delivery attempt (including retries) gets its own span, and keep the // leaf's closer so shutdown still QUITs the pooled connection. - tr := middleware.Wrap(leaf, otelmw.New(nil, nil)) + // + // smtpSpanAttrs sits inside otelmw so it can attach SMTP code / retryable + // attributes to the still-open gomailer.send span (otelmw already sets + // messaging.gomailer.outcome). Request flow: otelmw → smtp attrs → leaf. + tr := middleware.Wrap(leaf, otelmw.New(nil, nil), smtpSpanAttrs()) return newService(tr, leaf, cfg.From, cfg.ReplyTo, cfg.FrontendURL) } diff --git a/api/internal/mail/smtp_attrs.go b/api/internal/mail/smtp_attrs.go new file mode 100644 index 00000000..1541e595 --- /dev/null +++ b/api/internal/mail/smtp_attrs.go @@ -0,0 +1,66 @@ +package mail + +import ( + "context" + "errors" + + gomailer "github.com/shyim/go-mailer" + "github.com/shyim/go-mailer/middleware" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" +) + +// Low-cardinality span attributes for outbound mail failures. These extend the +// attributes already set by go-mailer's otelmw (messaging.gomailer.outcome, …). +// Never attach recipient, subject, or body — those explode cardinality. +const ( + attrSMTPCode = "messaging.gomailer.smtp_code" + attrRetryable = "mail.retryable" +) + +// IsRetryableSMTPCode reports whether an SMTP reply code is a soft/transient +// failure that is typically worth retrying. +// +// SMTP reply classes (RFC 5321): +// - 4xx — transient negative (e.g. 421, 450, 451, 452) → retryable +// - 5xx — permanent negative → not retryable +// +// Codes outside 400–599 (including 0 / unknown) are treated as not retryable +// so callers only flip mail.retryable=true for known soft failures. +func IsRetryableSMTPCode(code int) bool { + return code >= 400 && code < 500 +} + +// smtpFailureAttrs extracts low-cardinality attributes from a send error when +// it wraps a *gomailer.TransportError with an SMTP response code. Returns nil +// when no SMTP code is available. +func smtpFailureAttrs(err error) []attribute.KeyValue { + if err == nil { + return nil + } + var te *gomailer.TransportError + if !errors.As(err, &te) || te.Code == 0 { + return nil + } + return []attribute.KeyValue{ + attribute.Int(attrSMTPCode, te.Code), + attribute.Bool(attrRetryable, IsRetryableSMTPCode(te.Code)), + } +} + +// smtpSpanAttrs returns AfterSend middleware that attaches SMTP failure +// attributes to the active OpenTelemetry span. It must sit *inside* otelmw so +// the gomailer.send span is still open when the hook runs. +func smtpSpanAttrs() middleware.Middleware { + return middleware.AfterSend(func(ctx context.Context, _ *gomailer.SentMessage, err error) { + attrs := smtpFailureAttrs(err) + if len(attrs) == 0 { + return + } + span := trace.SpanFromContext(ctx) + if !span.IsRecording() { + return + } + span.SetAttributes(attrs...) + }) +} diff --git a/api/internal/mail/smtp_attrs_test.go b/api/internal/mail/smtp_attrs_test.go new file mode 100644 index 00000000..1169c114 --- /dev/null +++ b/api/internal/mail/smtp_attrs_test.go @@ -0,0 +1,162 @@ +package mail + +import ( + "context" + "errors" + "fmt" + "testing" + + gomailer "github.com/shyim/go-mailer" + "github.com/shyim/go-mailer/middleware" + "github.com/shyim/go-mailer/middleware/otelmw" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/attribute" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" +) + +func TestIsRetryableSMTPCode(t *testing.T) { + t.Parallel() + + tests := []struct { + code int + want bool + }{ + // Transient soft failures (SES 451 is the production case). + {421, true}, + {450, true}, + {451, true}, + {452, true}, + {499, true}, + + // Permanent hard failures. + {500, false}, + {550, false}, + {554, false}, + {599, false}, + + // Non-SMTP / success / unknown — not classified as retryable. + {0, false}, + {250, false}, + {354, false}, + {399, false}, + {600, false}, + {-1, false}, + } + + for _, tt := range tests { + t.Run(fmt.Sprintf("code_%d", tt.code), func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, IsRetryableSMTPCode(tt.code)) + }) + } +} + +func TestSMTPFailureAttrs(t *testing.T) { + t.Parallel() + + t.Run("nil error", func(t *testing.T) { + t.Parallel() + assert.Nil(t, smtpFailureAttrs(nil)) + }) + + t.Run("non-transport error", func(t *testing.T) { + t.Parallel() + assert.Nil(t, smtpFailureAttrs(errors.New("boom"))) + }) + + t.Run("transport error without code", func(t *testing.T) { + t.Parallel() + assert.Nil(t, smtpFailureAttrs(gomailer.NewTransportError("dial failed"))) + }) + + t.Run("wrapped 451 soft failure", func(t *testing.T) { + t.Parallel() + te := gomailer.NewTransportError(`Expected response code "250" but got code "451"`) + te.Code = 451 + attrs := smtpFailureAttrs(fmt.Errorf("send mail: %w", te)) + require.Len(t, attrs, 2) + assert.Equal(t, attribute.Int(attrSMTPCode, 451), attrs[0]) + assert.Equal(t, attribute.Bool(attrRetryable, true), attrs[1]) + }) + + t.Run("550 hard failure", func(t *testing.T) { + t.Parallel() + te := gomailer.NewTransportError("mailbox unavailable") + te.Code = 550 + attrs := smtpFailureAttrs(te) + require.Len(t, attrs, 2) + assert.Equal(t, attribute.Int(attrSMTPCode, 550), attrs[0]) + assert.Equal(t, attribute.Bool(attrRetryable, false), attrs[1]) + }) +} + +// failingTransport returns a fixed TransportError from Send. +type failingTransport struct { + err error +} + +func (f *failingTransport) String() string { return "smtp://test" } + +func (f *failingTransport) Send(context.Context, gomailer.RawMessage, *gomailer.Envelope) (*gomailer.SentMessage, error) { + return nil, f.err +} + +func TestSMTPSpanAttrsOnGomailerSend(t *testing.T) { + sr := tracetest.NewSpanRecorder() + tp := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(sr)) + + te := gomailer.NewTransportError(`Expected response code "250" but got code "451"`) + te.Code = 451 + leaf := &failingTransport{err: te} + + // Same order as NewService: otelmw outer, smtp attrs inner. + tr := middleware.Wrap(leaf, otelmw.New(tp, nil), smtpSpanAttrs()) + + from := gomailer.MustAddress("from@example.com", "") + to := gomailer.MustAddress("to@example.com", "") + env, err := gomailer.NewEnvelope(from, []gomailer.Address{to}) + require.NoError(t, err) + + _, sendErr := tr.Send(context.Background(), nil, env) + require.Error(t, sendErr) + + spans := sr.Ended() + require.Len(t, spans, 1) + span := spans[0] + assert.Equal(t, "gomailer.send", span.Name()) + + attrs := span.Attributes() + code, ok := findAttr(attrs, attrSMTPCode) + require.True(t, ok, "missing %s", attrSMTPCode) + assert.Equal(t, int64(451), code.AsInt64()) + + retryable, ok := findAttr(attrs, attrRetryable) + require.True(t, ok, "missing %s", attrRetryable) + assert.True(t, retryable.AsBool()) + + // Existing otelmw outcome attribute is preserved (we extend, not replace). + outcome, ok := findAttr(attrs, "messaging.gomailer.outcome") + require.True(t, ok) + assert.Equal(t, "error", outcome.AsString()) + + // No high-cardinality recipient/subject/body attributes. + for _, kv := range attrs { + key := string(kv.Key) + assert.NotEqual(t, "mail.recipient", key) + assert.NotEqual(t, "mail.to", key) + assert.NotEqual(t, "mail.subject", key) + assert.NotEqual(t, "mail.body", key) + assert.NotContains(t, key, "email.address") + } +} + +func findAttr(attrs []attribute.KeyValue, key string) (attribute.Value, bool) { + for _, kv := range attrs { + if string(kv.Key) == key { + return kv.Value, true + } + } + return attribute.Value{}, false +}