feat(ARSN-572): Propagate W3C trace context to MongoDB metadata writes#2611
feat(ARSN-572): Propagate W3C trace context to MongoDB metadata writes#2611delthas wants to merge 3 commits into
Conversation
Hello delthas,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
d42f30b to
01693cd
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## development/8.3 #2611 +/- ##
===================================================
+ Coverage 73.48% 73.53% +0.04%
===================================================
Files 222 223 +1
Lines 18183 18207 +24
Branches 3762 3790 +28
===================================================
+ Hits 13362 13388 +26
+ Misses 4816 4814 -2
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
01693cd to
ca456c9
Compare
ca456c9 to
a30bce8
Compare
a30bce8 to
39233c8
Compare
39233c8 to
a1fe7ca
Compare
a1fe7ca to
eb5a535
Compare
eb5a535 to
7c5ecce
Compare
7c5ecce to
28bca79
Compare
28bca79 to
8619151
Compare
04348d1 to
972bdac
Compare
972bdac to
d9198ea
Compare
francoisferrand
left a comment
There was a problem hiding this comment.
LGTM, with just a small fear (probably not an issue, but I'd like to be sure) about behavior if/when Otel is not configured/imported/... at all in the component (c.f. comment).
Which brings a related question: shoud this be merged in 8.3, or in a new branch?
d9198ea to
593039a
Compare
593039a to
72ad687
Compare
Added a test for this; no issues. But moving to 8.4 since the other repos for tracing will target their latest branch, too. |
Add a traceContext field to ObjectMDData carrying the currently-active OTEL trace context (W3C traceparent/tracestate). A new helper, stampActiveTraceContext, reads the active OTEL context and stamps it onto the metadata object in place. It is called from MongoUtils.serialize (covering the putObject / putObjectVerCase4 / repair write paths) and explicitly at the two paths that bypass serialize because they rewrite an already-escaped doc read from the DB (internalPutObject, internalDeleteObject). The helper is overloaded so callers pass either an ObjectMD instance or a raw metadata object. The value ends up in the MongoDB oplog; downstream consumers (backbeat, sorbet) can extract it to continue the trace across the async boundary, closing the loop on end-to-end tracing for flows that cross the oplog. When OTEL is not active on the caller (no SDK, or a request outside a traced context), there is no active span: stampActiveTraceContext clears the field and returns — zero cost, and no stale context loaded from storage is carried forward into a new oplog entry. Only adds @opentelemetry/api as a runtime dependency (the API-only surface package, a no-op when no SDK is registered). Issue: ARSN-572
3499403 to
7a8cc7b
Compare
|
LGTM |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
Context
Cloudserver recently gained OpenTelemetry tracing (CLDSRV-884, cloudserver PR scality/cloudserver#6140). That gives us a single end-to-end trace per S3 request spanning cloudserver → vault → hdproxy → hyperiod.
But many state mutations in the S3 stack are written to MongoDB and consumed asynchronously by worker services via the oplog:
Today the oplog entry carries no trace context, so async work performed hours or days after the S3 request cannot be tied back to the request that caused it. A user's
POST …?restoreproduces a trace that ends at the metadata write; the actual cold-tier retrieval by Sorbet is invisible from the original trace.Solution
Stamp a W3C trace context (
traceparent/tracestate) on every mutating metadata write, auto-derived from the currently-active OTEL context.The field lives on
ObjectMDDatanext tooriginOp:A new helper
stampActiveTraceContextinlib/storage/metadata/captureTraceContext.ts:@opentelemetry/apipropagation.inject()data.traceContext, or clears the field if no span is active (so a stale value loaded from storage cannot leak forward into a new oplog entry)It is exposed with TS overloads so callers pass whichever shape they have — an
ObjectMDinstance or a raw metadata object:It is called from inside
MongoUtils.serialize(), so every metadata write that goes through serialize automatically gets the stamp (theputObject/putObjectVerCase4/repairpaths inMongoClientInterface). For the 2 paths that bypassserializebecause they rewrite a doc previously read from the DB with already-escaped tags (internalPutObject,internalDeleteObject),stampActiveTraceContext()is called explicitly on theObjectMDinstance (the overload resolves.getValue()internally).Consumers (backbeat, sorbet — out of scope for this PR) will later:
value.traceContext.traceparentfrom the oplog entrypropagation.extract()it to reconstruct a parent contextcontext.with(extractedCtx, ...)This closes the loop on end-to-end tracing across the async boundary.
Design decisions
1. Auto-inject at the arsenal write boundary (vs. param plumbing).
The alternative was to thread a new
traceContextparameter through the ~15 cloudserver call sites that already setoriginOp. Rejected because:@opentelemetry/context-async-hooks(loaded by the OTEL SDK at cloudserver startup) already propagates the request context through arsenal's async call chain for free. Re-reading it at the mongo write is idiomatic OTEL.originOpout of params and attaches it to theObjectMD.originOpeasy to forget on new API endpoints today.2. Only
@opentelemetry/apias a runtime dep (~10 KB, no SDK).The api package is specifically designed to be safe for libraries to depend on. When no SDK is registered (today's state for anyone not opting into cloudserver OTEL),
context.active()returns a no-op context,trace.getSpan()returns undefined, andstampActiveTraceContext()clears the field and returns. Zero runtime cost when OTEL is off — covered by a dedicated unit test (see Verification).3. Optional field, no schema migration.
traceContextis absent by default — consistent with other optional ObjectMD fields likelegalHold,retentionDate. No default-value change in the ObjectMDData construction. Existing consumers that don't understand the field simply ignore it; fully backward compatible.4. Always reflects the current write — never carries a stale context forward.
If no span is active when
stampActiveTraceContext()runs, it clearstraceContext(rather than leaving whatever was loaded from storage). Same whenpropagation.inject()produces notraceparent. This prevents both edge-case OTEL output and the more practical hazard where a doc read for a delete/repair carries atraceContextfrom its original write that does not belong on the new oplog entry.5. Single helper, single contract.
The trace-stamping API on arsenal is one function —
stampActiveTraceContext, with overloads for theObjectMDand raw-data call shapes but a single implementation. There is intentionally no public setter onObjectMD: the contract is "trace context is stamped by the metadata backend at write time," and a model-level setter would invite callers in cloudserver/backbeat to bypass that contract (issue raised in review). Internally this also keeps the stamp/clear semantics in exactly one place. The overload pattern matches existing arsenal usage (convertToEpochTime,uuidv4).Interaction with async flows (cold restore as example)
Cold storage restore exercises both a sync and an async metadata write:
ObjectRestore:Post(sync, user-initiated viaPOST /bucket/key?restore) — cloudserver setsoriginOp; arsenal now also stampstraceContextpointing to the user's request trace. ✅ObjectRestore:Completed(async, Sorbet-triggered via cloudserver's/_/backbeat/*route) — behavior depends on Sorbet:traceparenton its HTTP call (ideally extracted from step 1's storedtraceContext): arsenal picks it up viacontext.with(remoteContext, ...)in cloudserver'sserver.js, and the Completed write ties back to the original user restore. ✅traceparent(today's state): safely no-ops — Completed write has notraceContext, no regression. ✅Same pattern applies to lifecycle expiration/transition, CRR replication, and GC.
Verification
Unit tests
tests/unit/storage/metadata/captureTraceContext.spec.js—stampActiveTraceContextagainst a mocked@opentelemetry/api: no-active-span clears (and is a no-op when there was nothing to clear), traceparent-only write, traceparent + tracestate write, defensive handling of a propagator that injects notraceparent, overwrite of a stale value, and theObjectMD-instance overload stamping ontogetValue().tests/unit/storage/metadata/captureTraceContext.noOtel.spec.js— drives the real@opentelemetry/apiwith no SDK registered (the "arsenal bumped into a consumer that never initialized OTEL" case): confirms a safe no-op that clears stale context and never throws.tests/unit/storage/metadata/mongoclient/{putObject,delObject,repair}.spec.js— sinon-stubbed tests verify the field lands on the document handed tofindOneAndReplace/bulkWritefor each write path the cloudserver hot path exercises, and that it is cleared rather than carried forward when no span is active.All unit tests in
tests/unit/models/andtests/unit/storage/metadata/pass;yarn buildis clean.End-to-end (follow-up on a test cluster)
ENABLE_OTEL=trueo.value.traceContext.traceparentshould be present, with characters 3-35 matching the Jaeger trace IDENABLE_OTEL=false→ field should be absentIssue: ARSN-572