fix: stop counting expected dependency failures as APM errors

api/go.mod

@@ -26,7 +26,6 @@ require (
 	github.com/robfig/cron/v3 v3.0.1
 	github.com/russross/blackfriday/v2 v2.1.0
 	github.com/shyim/go-mailer v0.1.0
-	github.com/shyim/go-mailer/middleware/otelmw v0.1.0
 	github.com/shyim/go-mailer/transport/smtp v0.1.0
 	github.com/shyim/go-queue v0.0.0-20260606124220-e2aa789807c9
 	github.com/shyim/go-version v0.0.0-20260602054622-2f4aa95a0358

api/go.sum

@@ -258,8 +258,6 @@ github.com/shirou/gopsutil/v4 v4.26.5 h1:RPcBXkpz7kOj9PqGFQOlBPZHsyaPvPVQc098y9R
 github.com/shirou/gopsutil/v4 v4.26.5/go.mod h1:LZ6ewCSkBqUpvSOf+LsTGnRinC6iaNUNMGBtDkJBaLQ=
 github.com/shyim/go-mailer v0.1.0 h1:je50kIrHk2IAvVrz99pwZSNMIW+l9ZNGPwVPrSBt2s4=
 github.com/shyim/go-mailer v0.1.0/go.mod h1:XdMOEImXpUjCIGO+TPH3jELmIHAYLeprkkVaM1PjCXA=
-github.com/shyim/go-mailer/middleware/otelmw v0.1.0 h1:4/YegdksPWXOA6ipSb8Ay3FyBVbE8fHHxIuxHjH9EO8=
-github.com/shyim/go-mailer/middleware/otelmw v0.1.0/go.mod h1:eb0JR9ZEhOu2Kfopu0I4dtIzn0Pif5a9P8EXIC4hLLU=
 github.com/shyim/go-mailer/transport/sendmail v0.1.0 h1:OeV5Yq63ibUqByR9MXgqSoWWUbenf2fNJro+lon8QrA=
 github.com/shyim/go-mailer/transport/sendmail v0.1.0/go.mod h1:sajMdVzJifN0rejTZeCFaEOSsQQUyIygNTiWCmm0kJI=
 github.com/shyim/go-mailer/transport/smtp v0.1.0 h1:WIVDl2TZ+yGPT63kF+CgtP9R5RS0TY2yP+18XyW2Krs=

api/internal/catalog/sync/service.go

@@ -11,12 +11,12 @@ import (
 	"github.com/friendsofshopware/shopmon/api/internal/config"
 	"github.com/friendsofshopware/shopmon/api/internal/database/queries"
 	"github.com/friendsofshopware/shopmon/api/internal/metrics"
+	"github.com/friendsofshopware/shopmon/api/internal/otelx"
 	"github.com/friendsofshopware/shopmon/api/internal/shopwareaccount"
 	"github.com/friendsofshopware/shopmon/api/internal/version"
 	"github.com/jackc/pgx/v5/pgxpool"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
-	"go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/trace"
 )
 
@@ -146,8 +146,9 @@ func (h *Service) SyncNames(ctx context.Context, names []string, shopwareVersion
 	recordOutcome := true
 	defer func() {
 		if err != nil {
-			span.RecordError(err)
-			span.SetStatus(codes.Error, err.Error())
+			// 429 aborts are expected (job retries; shopmon.store_sync.outcome
+			// =rate_limited). Other failures remain hard span errors.
+			otelx.RecordDependency(span, err)
 		}
 		span.End()
 		if !recordOutcome {

api/internal/httputil/client.go

@@ -11,6 +11,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/friendsofshopware/shopmon/api/internal/otelx"
 	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
 )
 
@@ -92,16 +93,25 @@ func (t *userAgentTransport) RoundTrip(req *http.Request) (*http.Response, error
 }
 
 // wrapTransport adds the Shopmon User-Agent and OpenTelemetry instrumentation.
-// Order: application -> userAgent -> otelhttp -> base, so the User-Agent is set
-// before tracing observes the request and still applies when a custom base is used.
+//
+// Order (outbound):
+//
+//	application → userAgent → expectedStatus → otelhttp → spanCapture → base
+//
+// User-Agent is set before tracing observes the request. expectedStatus sits
+// outside otelhttp so it can downgrade expected dependency statuses (429, 401,
+// 503, …) from span status Error to Ok after otelhttp applies semconv rules;
+// spanCapture (inside otelhttp) hands it the client span via context.
 func wrapTransport(base http.RoundTripper) http.RoundTripper {
 	if base == nil {
 		base = http.DefaultTransport
 	}
 	return &userAgentTransport{
-		base: otelhttp.NewTransport(
-			base,
-			otelhttp.WithSpanNameFormatter(ClientSpanName),
+		base: otelx.WrapClientTransport(
+			otelhttp.NewTransport(
+				otelx.CaptureClientSpan(base),
+				otelhttp.WithSpanNameFormatter(ClientSpanName),
+			),
 		),
 		ua: UserAgentString(),
 	}

api/internal/mail/mail.go

@@ -11,8 +11,6 @@ import (
 
 	"github.com/friendsofshopware/shopmon/api/internal/metrics"
 	gomailer "github.com/shyim/go-mailer"
-	"github.com/shyim/go-mailer/middleware"
-	"github.com/shyim/go-mailer/middleware/otelmw"
 	"github.com/shyim/go-mailer/transport"
 	smtptransport "github.com/shyim/go-mailer/transport/smtp"
 )
@@ -86,13 +84,10 @@ func NewService(cfg Config) (*Service, error) {
 		st.SetAllowPlaintextAuth(true)
 	}
 
-	// Instrument each delivery attempt with an OpenTelemetry span and metrics.
-	// Passing nil providers makes otelmw fall back to the globals configured by
-	// the telemetry package; when telemetry is disabled those are no-ops, so the
-	// 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))
+	// Instrument each delivery attempt with a classifying OpenTelemetry span
+	// (see instrumentTransport). Keep the leaf's closer so shutdown still QUITs
+	// the pooled connection — observability wrappers do not forward Close.
+	tr := instrumentTransport(leaf)
 
 	return newService(tr, leaf, cfg.From, cfg.ReplyTo, cfg.FrontendURL)
 }

api/internal/mail/otel.go

@@ -0,0 +1,90 @@
+package mail
+
+import (
+	"context"
+	"errors"
+
+	"github.com/friendsofshopware/shopmon/api/internal/otelx"
+	gomailer "github.com/shyim/go-mailer"
+	"github.com/shyim/go-mailer/middleware"
+	"go.opentelemetry.io/otel"
+	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/codes"
+	"go.opentelemetry.io/otel/trace"
+)
+
+// Span name matches go-mailer's otelmw default so existing Datadog dashboards
+// keep working after we swap in a classifying tracer.
+const mailSpanName = "gomailer.send"
+
+// instrumentTransport wraps leaf with an OpenTelemetry client span per Send
+// attempt. Soft SMTP failures (421/450/451/452) and retryable network blips
+// set error.expected=true and status Ok; other failures keep status Error.
+//
+// Outcome volume for alerts stays on shopmon.mail.send (package metrics). We
+// intentionally do not use otelmw.New here: its Span.SetError path cannot
+// classify expected degradations.
+func instrumentTransport(leaf gomailer.Transport) gomailer.Transport {
+	return middleware.Wrap(leaf, middleware.Observability(
+		middleware.WithTracer(&classifyingMailTracer{
+			tracer: otel.Tracer("shopmon/mail"),
+		}),
+		middleware.WithSpanName(mailSpanName),
+	))
+}
+
+type classifyingMailTracer struct {
+	tracer trace.Tracer
+}
+
+func (t *classifyingMailTracer) Start(ctx context.Context, name string) (context.Context, middleware.Span) {
+	ctx, span := t.tracer.Start(ctx, name, trace.WithSpanKind(trace.SpanKindClient))
+	return ctx, &classifyingMailSpan{span: span}
+}
+
+type classifyingMailSpan struct {
+	span    trace.Span
+	lastErr error
+}
+
+func (s *classifyingMailSpan) SetAttributes(attrs ...middleware.Attr) {
+	if len(attrs) == 0 {
+		return
+	}
+	kvs := make([]attribute.KeyValue, 0, len(attrs))
+	for _, a := range attrs {
+		switch a.Kind {
+		case middleware.KindInt:
+			kvs = append(kvs, attribute.Int64(a.Key, a.Int))
+		case middleware.KindBool:
+			kvs = append(kvs, attribute.Bool(a.Key, a.Bool))
+		default:
+			kvs = append(kvs, attribute.String(a.Key, a.Str))
+		}
+	}
+	s.span.SetAttributes(kvs...)
+}
+
+func (s *classifyingMailSpan) RecordError(err error) {
+	// Stash until SetError so we can choose expected vs hard. The observability
+	// middleware always calls RecordError then SetError on failure.
+	s.lastErr = err
+}
+
+func (s *classifyingMailSpan) SetError(description string) {
+	err := s.lastErr
+	if IsExpectedSMTPError(err) {
+		otelx.RecordExpected(s.span, err)
+		var te *gomailer.TransportError
+		if errors.As(err, &te) && te.Code != 0 {
+			s.span.SetAttributes(attribute.Int("smtp.response.code", te.Code))
+		}
+		return
+	}
+	if err != nil {
+		s.span.RecordError(err)
+	}
+	s.span.SetStatus(codes.Error, description)
+}
+
+func (s *classifyingMailSpan) End() { s.span.End() }

api/internal/mail/smtp_expected.go

@@ -0,0 +1,35 @@
+package mail
+
+import (
+	"errors"
+
+	"github.com/friendsofshopware/shopmon/api/internal/otelx"
+	gomailer "github.com/shyim/go-mailer"
+)
+
+// SMTPCodeExpected reports whether an SMTP response code is a soft/transient
+// failure (SES 451 timeouts, greylisting, mailbox busy, etc.).
+//
+// 421/450/451/452 are widely treated as retryable; permanent 5xx rejects and
+// other 4xx (e.g. 550) are hard failures.
+func SMTPCodeExpected(code int) bool {
+	switch code {
+	case 421, 450, 451, 452:
+		return true
+	default:
+		return false
+	}
+}
+
+// IsExpectedSMTPError reports whether err is (or wraps) a retryable SMTP
+// transport failure or a retryable network blip talking to the relay.
+func IsExpectedSMTPError(err error) bool {
+	if err == nil {
+		return false
+	}
+	var te *gomailer.TransportError
+	if errors.As(err, &te) && SMTPCodeExpected(te.Code) {
+		return true
+	}
+	return otelx.IsRetryableNetError(err)
+}

api/internal/mail/smtp_expected_test.go

@@ -0,0 +1,39 @@
+package mail
+
+import (
+	"errors"
+	"net"
+	"syscall"
+	"testing"
+
+	gomailer "github.com/shyim/go-mailer"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSMTPCodeExpected(t *testing.T) {
+	for _, code := range []int{421, 450, 451, 452} {
+		assert.True(t, SMTPCodeExpected(code), "code %d", code)
+	}
+	for _, code := range []int{0, 250, 550, 554, 400} {
+		assert.False(t, SMTPCodeExpected(code), "code %d", code)
+	}
+}
+
+func TestIsExpectedSMTPError(t *testing.T) {
+	assert.False(t, IsExpectedSMTPError(nil))
+
+	soft := gomailer.NewTransportError("timeout")
+	soft.Code = 451
+	assert.True(t, IsExpectedSMTPError(soft))
+	assert.True(t, IsExpectedSMTPError(errors.Join(errors.New("wrap"), soft)))
+
+	hard := gomailer.NewTransportError("mailbox missing")
+	hard.Code = 550
+	assert.False(t, IsExpectedSMTPError(hard))
+
+	assert.True(t, IsExpectedSMTPError(&net.OpError{
+		Op:  "dial",
+		Net: "tcp",
+		Err: syscall.ECONNREFUSED,
+	}))
+}

api/internal/monitoring/scrape/service.go

@@ -15,6 +15,7 @@ import (
 	"github.com/friendsofshopware/shopmon/api/internal/mail"
 	"github.com/friendsofshopware/shopmon/api/internal/metrics"
 	"github.com/friendsofshopware/shopmon/api/internal/notify"
+	"github.com/friendsofshopware/shopmon/api/internal/otelx"
 	"github.com/friendsofshopware/shopmon/api/internal/ptr"
 	"github.com/friendsofshopware/shopmon/api/internal/shopware/checker"
 	"github.com/jackc/pgx/v5/pgxpool"
@@ -125,8 +126,10 @@ func (h *Service) scrapeEnvironment(ctx context.Context, env queries.GetAllEnvir
 		authCtx, authSpan := tracer.Start(ctx, "environment.scrape.authenticate")
 		err := client.Authenticate(authCtx)
 		if err != nil {
-			authSpan.RecordError(err)
-			authSpan.SetStatus(codes.Error, err.Error())
+			// Tenant-side 401/403 (and other expected dependency statuses) must
+			// not inflate APM error rate; shopmon.scrape.outcome=auth_error
+			// remains the alert signal (#793).
+			otelx.RecordDependency(authSpan, err)
 		}
 		authSpan.End()
 

api/internal/monitoring/sitespeed/service.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"log/slog"
@@ -15,8 +16,14 @@ import (
 	"github.com/friendsofshopware/shopmon/api/internal/database/queries"
 	"github.com/friendsofshopware/shopmon/api/internal/httputil"
 	"github.com/friendsofshopware/shopmon/api/internal/metrics"
+	"github.com/friendsofshopware/shopmon/api/internal/otelx"
+	"go.opentelemetry.io/otel"
+	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/trace"
 )
 
+var tracer = otel.Tracer("shopmon/monitoring/sitespeed")
+
 type Service struct {
 	queries *queries.Queries
 	cfg     *config.Config
@@ -44,6 +51,11 @@ func (s *Service) Scrape(ctx context.Context, environmentID int32) (err error) {
 }
 
 func (s *Service) scrapeEnvironment(ctx context.Context, env queries.GetEnvironmentsWithSitespeedEnabledRow) (err error) {
+	ctx, span := tracer.Start(ctx, "sitespeed.scrape",
+		trace.WithAttributes(attribute.Int("environment.id", int(env.ID))),
+	)
+	defer span.End()
+
 	log := slog.With("environmentId", env.ID)
 
 	if s.cfg.SitespeedEndpoint == "" || s.cfg.SitespeedAPIKey == "" {
@@ -54,6 +66,10 @@ func (s *Service) scrapeEnvironment(ctx context.Context, env queries.GetEnvironm
 
 	defer func() {
 		if err != nil {
+			// 503 / connection-refused are retryable (go-queue retries the job);
+			// mark expected so intermediate attempts do not dominate APM errors.
+			// shopmon.sitespeed.outcome still counts every failure.
+			recordSitespeedSpan(span, err)
 			metrics.RecordSitespeedOutcome(ctx, metrics.OutcomeError)
 			return
 		}
@@ -99,7 +115,10 @@ func (s *Service) scrapeEnvironment(ctx context.Context, env queries.GetEnvironm
 
 	resp, err := httputil.NewHTTPClient(httputil.WithTimeout(300 * time.Second)).Do(req)
 	if err != nil {
-		return fmt.Errorf("call sitespeed: %w", err)
+		return &sitespeedError{
+			err:       fmt.Errorf("call sitespeed: %w", err),
+			retryable: otelx.IsRetryableNetError(err),
+		}
 	}
 	defer func() { _ = resp.Body.Close() }()
 
@@ -108,7 +127,11 @@ func (s *Service) scrapeEnvironment(ctx context.Context, env queries.GetEnvironm
 		return fmt.Errorf("read sitespeed response for environment %d: %w", env.ID, err)
 	}
 	if resp.StatusCode >= 400 {
-		return fmt.Errorf("sitespeed error (%d): %s", resp.StatusCode, string(body))
+		return &sitespeedError{
+			err:        fmt.Errorf("sitespeed error (%d): %s", resp.StatusCode, string(body)),
+			statusCode: resp.StatusCode,
+			retryable:  otelx.HTTPClientStatusExpected(resp.StatusCode),
+		}
 	}
 
 	// Parse response and save metrics
@@ -149,3 +172,26 @@ func (s *Service) scrapeEnvironment(ctx context.Context, env queries.GetEnvironm
 	log.Info("sitespeed scrape completed")
 	return nil
 }
+
+// sitespeedError wraps an upstream Sitespeed failure and whether the job should
+// treat it as an expected/retryable dependency degradation for span status.
+type sitespeedError struct {
+	err        error
+	statusCode int
+	retryable  bool
+}
+
+func (e *sitespeedError) Error() string { return e.err.Error() }
+func (e *sitespeedError) Unwrap() error { return e.err }
+
+// HTTPStatusCode exposes a positive status for otelx classification when set.
+func (e *sitespeedError) HTTPStatusCode() int { return e.statusCode }
+
+func recordSitespeedSpan(span trace.Span, err error) {
+	var se *sitespeedError
+	if errors.As(err, &se) && se.retryable {
+		otelx.RecordExpected(span, err)
+		return
+	}
+	otelx.RecordDependency(span, err)
+}