Add APM support to XMTPD

[api-Hypernova]

Add Datadog APM spans across RPC handlers, DB queries, message pipelines, and sync paths in XMTPD to support tracing

Introduce gated tracing with Datadog spans across RPCs, query/publish/subscribe workers, sync and replication paths, and pgx queries; add reader/writer DB role tagging and a reader DB constructor; wire async context propagation and span-safe tagging; include server-side tracing interceptor and tracing package with sampling.

📍Where to Start

Start with the tracing API in pkg/tracing/tracing.go (https://github.com/xmtp/xmtpd/pull/1589/files#diff-f1e0d24911164f850e8c1e92b5e73073d417eb5ca067ad3feba01bc6d6211082), then review the server interceptor in pkg/interceptors/server/tracing.go (https://github.com/xmtp/xmtpd/pull/1589/files#diff-63c453a7cff21cde1e2fc68b4c24dd9a22ac2a251e1af038d19780c37b6f39a0) and the DB wiring in pkg/db/pgx.go (https://github.com/xmtp/xmtpd/pull/1589/files#diff-61a5fd647b6319a61b4ad6e44829b0fd29d8f14afefa2700f222127028ce36fc).

Changes since #1589 opened

• Modified originatorStream.listen and originatorStream.validateEnvelope methods to create parent-child span relationships by switching from StartSpan to StartSpanFromContext, threading context through validateEnvelope, and using the batch context as parent for validation spans [7cd61f8]
• Added tracing tag constants to pkg/tracing/spans.go including TagBatchSize, TagEnvelopesParsed, TagParseErrors, TagValidCount, TagInvalidCount, TagReason, TagDroppedEnvelopes, TagWrongOriginator, TagExpectedSequenceID, TagMigrationMode, TagConnectionSuccess, and TagTargetAddress [7cd61f8]
• Replaced hardcoded span tag key strings with constants from the tracing package in subscribeWorker.start and subscribeWorker.dispatchToListeners methods, syncWorker.connectToNode and syncWorker.setupStream methods [7cd61f8]
• Updated pkg/tracing/README.md to document default sampling rates of 100% in dev/test environments and 10% in prod/staging environments, and added documentation for the APM_SAMPLE_RATE environment variable [7cd61f8]
• Replaced hardcoded error tag string with Datadog's ext.Error constant in DBSubscription[ValueType, CursorType].poll method [7cd61f8]
• Added panic recovery handling to tracing.Wrap function [71c3d0e]
• Changed numeric tag assertions in tracing tests to use delta-based comparison [71c3d0e]
• Replaced direct field access with getter method in message.Service.QueryEnvelopes handler [71c3d0e]
• Replaced explicit loop counters with integer range form in tracing tests [71c3d0e]
• Updated method signatures and test assertions for Go language modernization [71c3d0e]

📊 Macroscope summarized a9d77e8. 14 files reviewed, 11 issues evaluated, 9 issues filtered, 1 comment posted

🗂️ Filtered Issues

pkg/api/message/publish_worker.go — 0 comments posted, 2 evaluated, 2 filtered

• line 116: Nil pointer dereference: p.traceContextStore.Store(stagedID, span) is called without checking if p.traceContextStore is nil. The traceContextStore field was newly added to publishWorker (per the diff), and if the constructor doesn't initialize this pointer field, this call will panic when attempting to invoke a method on a nil receiver. [ Low confidence ]
• line 152: Potential nil pointer dereference when calling p.traceContextStore.Retrieve(stagedEnv.ID). The traceContextStore field is a pointer type (*tracing.TraceContextStore) that was newly added to the struct. If this field is not initialized when creating a publishWorker instance, it will default to nil, and calling Retrieve on a nil receiver will cause a panic. Unlike other tracing functions in the package (e.g., SpanTag, Link, StartSpanFromContext) which have guards for when tracing is disabled, there is no nil check before calling Retrieve. [ Low confidence ]

pkg/db/pgx.go — 0 comments posted, 2 evaluated, 2 filtered

• line 117: No nil check on c.logTracer before calling TraceQueryEnd. If compositeTracer is constructed with a nil logTracer, this will cause a nil pointer dereference panic at runtime. [ Low confidence ]
• line 118: No nil check on c.apmTracer before calling TraceQueryEnd. If compositeTracer is constructed with a nil apmTracer, this will cause a nil pointer dereference panic at runtime. [ Already posted ]

pkg/interceptors/server/tracing.go — 0 comments posted, 1 evaluated, 1 filtered

• line 91: In tagRPCResult, the error message is set using span.SetTag(ext.ErrorMsg, err.Error()) directly instead of tracing.SpanTag. This bypasses the string truncation protection implemented in tracing.SpanTag, which limits strings to MaxTagValueLength to prevent excessive payload sizes. Error messages can be arbitrarily long (especially for wrapped errors or errors containing user data), potentially causing performance issues or exceeding Datadog payload limits. [ Already posted ]

pkg/tracing/tracing.go — 1 comment posted, 6 evaluated, 4 filtered

• line 93: The package-level variable apmEnabled is written without synchronization. If Start() is called concurrently from multiple goroutines, or if other code reads apmEnabled while Start() is executing, this creates a data race. [ Low confidence ]
• line 173: The SetEnabledForTesting function modifies the package-level apmEnabled variable without synchronization. If multiple tests run in parallel (using t.Parallel()), concurrent reads and writes to apmEnabled will cause a data race, which is undefined behavior in Go and will be detected by the race detector. [ Low confidence ]
• line 350: Silent data loss without logging: When the store reaches MaxStoreSize capacity at line 350-351, new trace contexts are silently dropped with no logging or metrics. The comment says this "indicates publish_worker is falling behind and needs investigation," but there's no way to detect this condition is occurring since it returns silently without any observable side effect. [ Low confidence ]
• line 354: Assignment to nil map: At line 354, s.contexts[stagedID] = traceContextEntry{...} will panic if s.contexts was never initialized. While len(s.contexts) at line 350 safely returns 0 for a nil map, writing to a nil map causes a runtime panic. The method should either lazily initialize the map or the Store method should check for nil before assignment. [ Low confidence ]

[fbac]

Left a couple comments. Will continue reviewing locally.

[fbac]

In the interceptor grpc_metrics.go there's a func parseProcedure(procedure string) (string, string) method.

Seems like the perfect opportunity to decide on a common standard. I don't have strong opinions other than probably we want the function under pkg/utils/grpc or similar.

[fbac]

What happens here if the context already carries an span? I.E the connection is originated by a client and it carries a header. I don't recall talking about anything like that.

Maybe it's already taken in account by tracer.StartSpanFromContext?