Trace GCS upload/download duration and bytes - #333
Conversation
8e80779 to
797c68e
Compare
Emit one OpenTelemetry span per GCS transfer so we can see, on prod, how long uploads and downloads actually take and for how many bytes. Each span (tangle.storage tracer, name gcs.<operation>) carries gcs.operation, gcs.uri, and gcs.bytes, and is marked ERROR with the exception recorded on failure. TracingGoogleCloudStorageProvider subclasses the upstream provider and overrides only the four public transfer methods; the GKE launchers construct it in place of the plain provider. This is measurement only: it changes no transfer behavior and lets us find where latency concentrates before deciding on any timeout policy.
797c68e to
db1c13e
Compare
| yield span | ||
| except Exception as exception: | ||
| span.set_status(StatusCode.ERROR) | ||
| span.record_exception(exception) |
There was a problem hiding this comment.
AI review: start_as_current_span() already records escaping exceptions and marks the span as ERROR by default. Because this handler also calls set_status() and record_exception() before re-raising, each failed transfer emits two exception events (confirmed with an in-memory span exporter). Could we remove the manual exception handling, or explicitly disable the context manager’s automatic handling?
Mbeaulne
left a comment
Mbeaulne
left a comment
There was a problem hiding this comment.
AI review left some comments. If they are true, fix. Otherwise I've approved
camielvs
left a comment
camielvs
left a comment
There was a problem hiding this comment.
I don't have enough python & backend context to review on an implementation level, but overall the intent looks fine to me
|
Closing in favor of orchestrator-side telemetry. The inv-26772 investigation showed the orchestrator freeze happens in Superseded by a stack that instruments the actual freeze path, attributed by
|
3 participants
Why
A recent incident had Tangle pipelines stalling on GCS transfers. The first fix attempt was per-request timeouts (#330, now closed), but the stock
google-cloud-storageclient already enforces a 60s per-request timeout plus a ~120s retry deadline, so a same-default override added little. Before choosing any timeout policy we need to know how long real uploads/downloads take on prod — so we can find concretely where the time goes and which operation to focus on.Out of scope
GCS Fuse operations, which is the more impactful area to measure, is not covered by this change.
Changes
cloud_pipelines_backend/instrumentation/gcs_tracing.py:TracingGoogleCloudStorageProvider, a subclass of the upstreamGoogleCloudStorageProviderthat overrides only the four public transfer methods (upload,upload_bytes,download,download_bytes). Each wrapssuper()in an OpenTelemetry span.gcs_clientargument is unchanged).Each span uses the
tangle.storagetracer, is namedgcs.<operation>, and carries:gcs.operation—upload/upload_bytes/download/download_bytesgcs.uri— the object/prefix being transferredgcs.bytes— payload size, recorded only for the in-memory transfers (upload_bytes/download_bytes, where it is a freelen(data))On failure the span is marked
ERRORand the exception is recorded, then re-raised unchanged.Duration is the primary signal and is intrinsic to every span. We deliberately do not walk the local filesystem to size file/dir transfers: that scan is strictly dominated by the transfer it would measure (the base provider already does one GCS round-trip per file), it added a robustness hole (a
statrace could fail an otherwise-successful transfer), and authoritative sizes for those artifacts already live inartifact_data.total_sizeand can be joined offline.System / UX impact
__init__is not overridden and no timeouts are introduced.BatchSpanProcessor). No extra filesystem access.Before / after
Follow-up
Companion oasis-backend PR bumps the submodule and swaps the orchestrator-side provider so orchestrator-initiated GCS ops (
exists,get_info, smalldownload_bytes, output preservation) are covered too.