blob: allow a custom correlation-id key across object metadata, logs and traces by LihaoLiuXs · Pull Request #576 · salesforce/multicloudj

LihaoLiuXs · 2026-07-28T16:57:48Z

What

Lets a caller choose the key name under which the SDK records the correlation id, via a new optional OperationContext.correlationIdKey.

OperationContext ctx = OperationContext.builder()
    .correlationId("request-abc-987")
    .correlationIdKey("x-request-id")   // optional
    .build();

The key applies to all three observability surfaces at once, so a caller can align the SDK with an existing correlation convention:

Surface	Default (unchanged)	With `correlationIdKey("x-request-id")`
Stored object metadata	`sdk-logging-correlation-id`	`x-request-id`
SLF4J MDC entry	`correlation_id`	`x-request-id`
OpenTelemetry span attribute	`correlation_id`	`x-request-id`

Replace semantics, not dual-stamping: when a custom key is supplied the default key is not also written.

Why the scope grew since the first review

The original revision renamed only the object metadata key. That turned out to be the wrong boundary in practice: a caller who set a custom key still saw correlation_id in their log aggregator and had nothing to correlate on. Extending it to MDC and the span attribute makes the feature actually usable for its stated purpose.

Backward compatibility

Nothing changes for existing callers. The defaults are untouched, so dashboards, log queries, and metric filters keyed on correlation_id continue to work. correlationIdKey is opt-in and unset by default. sdk-logging-service-id and sdk-logging-tenant-id remain fixed and are not customizable.

Validation

A supplied key must match ^[a-z0-9][a-z0-9_-]{0,127}$ and must not collide with a reserved SDK identifier (trace_id, span_id, sdk_service, sdk_provider, tenant_id, service_id, sdk-logging-tenant-id, sdk-logging-service-id). Violations throw InvalidArgumentException.

Two deliberate choices here:

Lowercase only. S3 and GCS lowercase user-metadata keys on read, so an uppercase key would not round-trip; it is rejected up front with an error message suggesting the lowercased form.
Validated in the constructor, not lazily at upload. An invalid key fails at build(), including through toBuilder(), rather than surfacing much later during an upload.

Review comments addressed

Reserved-key collision (T1) — previously a custom key equal to sdk-logging-tenant-id would be stamped first and silently drop the real tenant id. Now rejected at construction, with the reserved set widened to cover the MDC and span names too.
Multipart (T2) — the custom key flows through the shared stampContextMetadata helper, which the multipart paths also call. Covered by unit tests on AWS and in-memory.
Key-shape validation (T3) — implemented as described above.

MDC leak safety

MultiCloudJLogger snapshots a fixed set of SDK-managed MDC keys so it never clobbers the caller's outer MDC. A dynamic key breaks that assumption, so the resolved key is now threaded through snapshotMdc(...) at every entry point, including the async completion paths that snapshot on a different thread. Tests cover restore-on-success, restore-on-failure, and no-residue on a foreign completion thread.

Testing

All green locally (mvn test checkstyle:check, checkstyle clean on every module):

Module	Tests	Notes
multicloudj-common	156	`OperationContextTest` 41, `MultiCloudJLoggerTest` 44
blob-client	310	includes the new conformance test
blob-aws	`AwsTransformerTest` 134	incl. multipart custom key
blob-gcp	`GcpTransformerTest` 129
blob-ali	`AliTransformerTest` 86
blob-inmemory	22	`InMemoryBlobStoreCorrelationTest` 9

Conformance: testUpload_withCustomCorrelationIdKey_stampsCustomKeyOnly in AbstractBlobStoreIT, verified to execute (not skip) under InMemoryBlobStoreIT. It is gated to the in-memory provider because the AWS/GCP/Ali suites replay recorded WireMock fixtures that were never recorded with a custom key; exercising it there would require re-recording with live credentials.

Docs

Adds a "Observability: Correlation, Tenant, and Service IDs" section to documentation/guides/blobstore-guide.md, covering the default keys, how to customize, the validation rules, and a caution that choosing a custom key opts you out of shared dashboards keyed on correlation_id.

Follow-ups

Live validation of the log/span surface against real AWS and GCP, with the results added here.
Cloud conformance coverage for a custom key, which needs a credentialed -Drecord run to regenerate fixtures.

Callers can now customize the object-metadata key under which the SDK stamps the correlation-id value on upload, defaulting to the existing "sdk-logging-correlation-id" when unspecified. - multicloudj-common: add optional OperationContext.correlationIdMetadataKey plus resolveCorrelationIdMetadataKey(), so the default lives in one place and cannot drift across providers. Backward-compatible (Lombok builder). - Stamp the correlation id under the resolved key in the AWS, GCP, Alibaba and in-memory upload transformers; service-id and tenant-id keys unchanged. - The "app already supplied this key" guard now checks the resolved key, so a custom key the application set explicitly is never overwritten. - Unit tests across common + all four providers: custom key used, default fallback, and app-supplied value not overwritten.

codecov-commenter · 2026-07-28T17:08:41Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.65%. Comparing base (37fffcc) to head (cfc1fdd).

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #576      +/-   ##
============================================
+ Coverage     83.61%   83.65%   +0.04%     
- Complexity      674      675       +1     
============================================
  Files           215      216       +1     
  Lines         15010    15053      +43     
  Branches       2076     2082       +6     
============================================
+ Hits          12550    12593      +43     
- Misses         1636     1637       +1     
+ Partials        824      823       -1

Flag	Coverage Δ
unittests	`83.65% <100.00%> (+0.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

iamabhilaksh · 2026-07-30T08:02:52Z

+   * @return the custom correlation-id metadata key when supplied and non-blank, else {@link
+   *     SdkLoggingMetadataKeys#CORRELATION_ID}
+   */
+  public String resolveCorrelationIdMetadataKey() {


resolveCorrelationIdMetadataKey() only gets exercised in this PR from the single-shot upload paths (toRequest/toBlobInfo/toPutObjectRequest/doUpload). main is 2 commits ahead of this PR's base and already has #572 merged, which pulled the correlation/service/tenant stamping into a shared stampContextMetadata helper and extended it to doInitiateMultipartUpload/toCreateMultipartUploadRequest/toMultipartUpload for AWS, GCP, and in-memory — I checked, none of those multipart call sites reference resolveCorrelationIdMetadataKey(), they still stamp under the fixed CORRELATION_ID_METADATA_KEY. GitHub is also showing this PR as CONFLICTING/dirty against main right now. Once it's rebased, are we planning a follow-up to route the multipart paths through the same resolver, or is "custom key" scoped to single-shot uploads by design?

iamabhilaksh · 2026-07-30T08:02:54Z

    Map<String, String> metadata = new HashMap<>(request.getMetadata());
    if (request.getOperationContext() != null) {
      OperationContext ctx = request.getOperationContext();
+      String correlationIdKey = ctx.resolveCorrelationIdMetadataKey();


We resolve correlationIdKey and stamp it before checking containsKey(SERVICE_ID_METADATA_KEY)/TENANT_ID_METADATA_KEY. I traced through it: if a caller sets correlationIdMetadataKey to "sdk-logging-service-id" (or the tenant-id key) while also setting serviceId, the correlation stamp lands first, metadata.containsKey(SERVICE_ID_METADATA_KEY) then evaluates true, and the service id silently never gets written — no exception, no log, just a wrong/missing value on the stored object. Same ordering in GcpTransformer.java:106 and InMemoryBlobStore.java:169. Should OperationContext (or the resolver) reject or normalize a custom key that collides with the fixed keys, instead of letting stamping order decide the outcome silently?

iamabhilaksh · 2026-07-30T08:02:55Z

+   * stored metadata <em>key</em>; it does not change the correlation id value, the span attribute,
+   * the MDC entry, or the value echoed back on responses.
+   */
+  String correlationIdMetadataKey;


There's no validation on the custom key value itself — an app could pass a key with characters that aren't valid as S3/GCS/OSS metadata header names (whitespace, colons, non-ASCII), and today that'd surface as whatever exception the underlying substrate SDK throws deep inside the upload call, not a clear InvalidArgumentException from multicloudj at the point the context is built. The rest of the SDK favors failing fast with InvalidArgumentException (storage-class handling in AwsTransformer is one example). Worth validating the key shape at resolveCorrelationIdMetadataKey() or builder time, or is deferring to the substrate SDK's own validation an accepted tradeoff here?

iamabhilaksh

Right abstraction and the tests are meaningful. Flagging one correctness edge inline — a custom key colliding with the fixed service-id/tenant-id key silently drops that value (stamping order decides it). Plus a multipart-path gap vs #572 and a key-validation question. Note the PR is currently CONFLICTING against main, so it needs a rebase before merge regardless.

# Conflicts: # blob/blob-aws/src/main/java/com/salesforce/multicloudj/blob/aws/AwsTransformer.java # blob/blob-gcp/src/main/java/com/salesforce/multicloudj/blob/gcp/GcpTransformer.java # blob/blob-inmemory/src/main/java/com/salesforce/multicloudj/blob/inmemory/InMemoryBlobStore.java

Address PR review feedback on the custom correlation-id metadata key: - Reject a custom key that collides with the reserved service-id or tenant-id key (previously the correlation stamp ran first and the colliding service/tenant id was silently dropped). - Reject a custom key whose shape is not portable across providers (only ASCII letters, digits, hyphens and underscores are allowed), failing fast with InvalidArgumentException instead of surfacing a substrate error later. - Validation is centralized in resolveCorrelationIdMetadataKey() so it applies uniformly to all providers and to both single-shot and multipart upload paths. - Add multipart custom-key coverage (AWS toCreateMultipartUploadRequest, InMemory initiateMultipartUpload) and collision/shape unit tests.

roseyang62

LGTM ✅ Clean, well-scoped change — key resolution, fallback, and validation all live in one place (resolveCorrelationIdMetadataKey()) so the providers can't drift, reserved-key collisions and invalid formats fail fast with a clear InvalidArgumentException, and routing through the shared stampContextMetadata() helper means both single-shot and multipart paths honor the custom key. Backward compatible (defaults to sdk-logging-correlation-id, service-id/tenant-id keys stay fixed as intended) with good coverage across common and all providers.

The custom correlation-id key previously renamed only the stored object's metadata key, so a caller who set one still saw the fixed correlation_id in their logs and traces and could not correlate on their own key. Rename correlationIdMetadataKey -> correlationIdKey and apply it to all three surfaces: object metadata, the SLF4J MDC entry, and the OpenTelemetry span attribute. Defaults are unchanged (sdk-logging-correlation-id for metadata, correlation_id for MDC and span), so existing consumers and dashboards keyed on correlation_id are unaffected. Validation moves into a hand-written all-args constructor so an invalid key fails at build() instead of at upload, and is enforced through toBuilder() as well. Keys are restricted to ^[a-z0-9][a-z0-9_-]{0,127}$ because S3 and GCS lowercase user-metadata keys on read, so an uppercase key would not round-trip. The reserved set now also covers the MDC and span names a custom key could collide with. MultiCloudJLogger threads the resolved key through snapshotMdc so a custom key cannot leak into the caller's MDC, including on async completion threads. Adds an in-memory conformance test, provider unit tests, and a correlation section to the blobstore guide. Consolidates the two overlapping in-memory correlation test classes into one.

iamabhilaksh

LGTM 👍

salesforce-cla Bot added the cla:signed label Jul 28, 2026

LihaoLiuXs added 2 commits July 28, 2026 14:45

Merge branch 'main' into blob/custom-correlation-id-metadata-key

5c3c07a

Merge branch 'main' into blob/custom-correlation-id-metadata-key

af31120

iamabhilaksh reviewed Jul 30, 2026

View reviewed changes

LihaoLiuXs added 2 commits August 5, 2026 12:45

roseyang62 approved these changes Aug 6, 2026

View reviewed changes

LihaoLiuXs changed the title ~~blob: allow custom correlation-id metadata key name~~ blob: allow a custom correlation-id key across object metadata, logs and traces Aug 6, 2026

iamabhilaksh approved these changes Aug 7, 2026

View reviewed changes

Navigation Menu

Uh oh!

blob: allow a custom correlation-id key across object metadata, logs and traces - #576

blob: allow a custom correlation-id key across object metadata, logs and traces#576
LihaoLiuXs wants to merge 6 commits into
mainfrom
blob/custom-correlation-id-metadata-key

LihaoLiuXs commented Jul 28, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Jul 28, 2026 •

edited

Loading

Uh oh!

iamabhilaksh Jul 30, 2026

Uh oh!

iamabhilaksh Jul 30, 2026

Uh oh!

iamabhilaksh Jul 30, 2026

Uh oh!

iamabhilaksh left a comment

Uh oh!

roseyang62 left a comment

Uh oh!

iamabhilaksh left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

LihaoLiuXs commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why the scope grew since the first review

Backward compatibility

Validation

Review comments addressed

MDC leak safety

Testing

Docs

Follow-ups

Uh oh!

codecov-commenter commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

iamabhilaksh Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

iamabhilaksh Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

iamabhilaksh Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

iamabhilaksh left a comment

Choose a reason for hiding this comment

Uh oh!

roseyang62 left a comment

Choose a reason for hiding this comment

Uh oh!

iamabhilaksh left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

LihaoLiuXs commented Jul 28, 2026 •

edited

Loading

codecov-commenter commented Jul 28, 2026 •

edited

Loading