[rajeshkumarchandolu]

What changes were proposed in this pull request?

Ozone services export OpenTelemetry spans, but their log lines don't include the trace context, so it is hard to go from a trace in Jaeger to the matching logs, or the other way round. This change keeps the current span's trace_id and span_id in the slf4j MDC, so a log pattern can print them with %X{trace_id} and %X{span_id}.

Approach: every makeCurrent() / Context.wrap(...) in OpenTelemetry goes through ContextStorage.attach(). TracingUtil.MdcContextStorage wraps the default storage and updates the MDC from the current span on attach and on scope close. It is added with ContextStorage.addWrapper by TracingUtil.installMdcContextStorage(), which is called only from Ozone startup paths:

• SCM, OM and datanode: TracingReconfigurationCallback.init
• S3 Gateway: Gateway.call
• Ozone CLIs (ozone sh, ozone fs, Freon, ozone ratis): TracingUtil.execute

Details:

• The client library (RpcClient) never installs it, so applications using the Ozone client keep their own MDC and context storage. No ContextStorageProvider is registered through META-INF/services, so it can't conflict with a provider the application ships.
• No call-site changes. All existing spans, and any added later, get the IDs in their log lines.
• The MDC is derived from the current OpenTelemetry context rather than saved and restored. This avoids re-applying stale values that log4j's InheritableThreadLocal MDC copies into child threads.
• The install must run before OpenTelemetry Context is first used in the JVM. If the context is already initialized, it logs a single WARN and the IDs are not added.
• The default log4j.properties patterns are unchanged. To print the IDs, add e.g. [trace=%X{trace_id} span=%X{span_id}] to the pattern.

Scope: this covers log lines written inside an active span, i.e. the synchronous request paths that show up in Jaeger for each command. Asynchronous tasks don't carry the trace context yet. For example, SCM EventQueue-* threads created during a datanode Register RPC inherit that span's IDs through the MDC and keep them. This will be handled in a follow-up that propagates the trace context to SCM EventQueue handlers.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-13679

How was this patch tested?

• Unit tests (TestTracingUtil):
  • Without the install (as in client applications), an app-set trace_id in the MDC is left unchanged.
  • Installing after the context is initialized has no effect.
  • IDs are set and restored for nested spans, and cleared for invalid spans.
  • A stale inherited MDC value is cleared.
  • The MDC is cleared on worker threads after the task.
  • All 16 tests pass, and so does checkstyle.sh.
• Manual test: a docker-compose cluster (ozone + monitoring.yaml) with tracing enabled and Jaeger. The log pattern was changed locally to [trace=%X{trace_id} span=%X{span_id}]; that change is not part of this PR. No OpenTelemetry context is already initialized warning appeared in the OM, SCM, S3 Gateway or datanode logs.

Verification in local docker compose

Tested with an ozone sh key put. The SCM and datanode log lines for the request carry the same trace_id, and each span_id matches the span in Jaeger (SCM AllocateScmBlock, datanode WriteChunk):

scm-1      | [IPC Server handler 0 on default port 9863] INFO [trace=ae95c75a38fd69a453885f2ec358dba8 span=fdbc2706b19b6aca] ha.SequenceIdGenerator: Allocate a batch for containerId, change lastId from 0 to 1000.
scm-1      | [IPC Server handler 0 on default port 9863] INFO [trace=ae95c75a38fd69a453885f2ec358dba8 span=fdbc2706b19b6aca] ha.SequenceIdGenerator: Allocate a batch for localId, change lastId from 117889302528000000 to 117889302528001000.
datanode-3 | [...-ChunkWriter-1-0] WARN [trace=ae95c75a38fd69a453885f2ec358dba8 span=7d2e77907911a2f7] statemachine.DatanodeConfiguration: ...

Tested S3 Gateway with aws s3api create-bucket, put-object and get-object. Each request's S3G log lines carry its own trace_id, matching the request's trace in Jaeger (BucketEndpoint.put, ObjectEndpoint.put, ObjectEndpoint.get):

s3g | [qtp...-47] INFO [trace=bc8a13ced9a7c1da14cc84b5366fbe5f span=299d22ddd269ae78] rpc.RpcClient: Creating Bucket: s3v/mdc-s3-proof, with bucket layout OBJECT_STORE, ...
s3g | [qtp...-48] INFO [trace=9b7449732c8b316b4bdc4f14bd063215 span=d9a7019f2c00dc06] grpc.GrpcConfigKeys: raft.grpc.client.worker-group.size = 0 (default)
s3g | [grpc-default-executor-0] INFO [trace=9b7449732c8b316b4bdc4f14bd063215 span=94da405dcb3be506] netty.TcpMetrics: Epoll available during static init of TcpMetrics:true
s3g | [qtp...-43] INFO [trace=87268aebec1d1d55c9174e994a8e9f9f span=313c0f8516bb7abe] scm.XceiverClientGrpc: XceiverClientGrpc is created for pipeline ...

Build:

https://github.com/rajeshkumarchandolu/ozone/actions/runs/40675421635

[rajeshkumarchandolu]

@sravani-revuri , @sumitagrawl can you review this pull-request .

[sumitagrawl]

LGTM

[chihsuan]

Thanks for working on this! @rajeshkumarchandolu I found a few cases around the global provider that may need attention. Please see inline comments for details.

[chihsuan]

Thanks @rajeshkumarchandolu! The addWrapper switch and the clearer scope in the description look great. Just two small notes inline.

[chihsuan]

Thanks for addressing the feedback! @rajeshkumarchandolu LGTM otherwise.

I just noticed each tracing-enabled test in TestMdcContextStorageInstall takes 7-8 seconds, which may slow down CI. Could we fix it before merging?

[chihsuan]

Thanks @rajeshkumarchandolu LGTM!

[chihsuan]

Hi @rajeshkumarchandolu,

It seems TestMdcContextStorageInstall fails in CI after merging master. I reproduced it locally, and it looks related to the OpenTelemetry 1.67 bump (HDDS-17103). A provider without a span processor now hands out noop tracers. Could you add SimpleSpanProcessor.create(SpanExporter.noop()) like TestTracingInitModes does, and check that CI passes in your fork? Thanks!

[rajeshkumarchandolu]

@sumitagrawal @chihsuan All review comments have been resolved. Could you take another look and merge if there are no further changes needed?

[chihsuan]

Merged. Thanks @rajeshkumarchandolu for the patch! Thanks @sumitagrawl for the review!