feat: Add ctx to the origin prefetch client#552
feat: Add ctx to the origin prefetch client#552hweawer wants to merge 1 commit intofeat/tracing-proxy-prefetchfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds context support to the PrefetchBlob method across the origin blob client infrastructure, enabling proper distributed tracing and request cancellation for blob prefetching operations.
Changes:
- Updated
PrefetchBlobinterface signatures to acceptcontext.Contextas the first parameter - Added comprehensive OpenTelemetry tracing instrumentation to prefetch operations
- Updated all call sites and tests to pass context appropriately
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| origin/blobclient/client.go | Updated Client interface and HTTPClient.PrefetchBlob implementation to accept context, added tracing spans and context propagation |
| origin/blobclient/cluster_client.go | Updated ClusterClient interface and clusterClient.PrefetchBlob implementation to accept context, added comprehensive tracing and error handling |
| proxy/proxyserver/prefetch.go | Removed obsolete comment about context not being supported, updated call to pass context |
| proxy/proxyserver/server_test.go | Updated mock expectations to include gomock.Any() for context parameter |
| origin/blobserver/server_test.go | Updated test to pass context.Background() when calling PrefetchBlob |
| mocks/origin/blobclient/client.go | Regenerated mock to reflect updated Client interface signature |
| mocks/origin/blobclient/clusterclient.go | Regenerated mock to reflect updated ClusterClient interface signature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ctx, span := otel.Tracer("kraken-origin-cluster").Start(ctx, "cluster.prefetch_blob", | ||
| trace.WithSpanKind(trace.SpanKindClient), | ||
| trace.WithAttributes( | ||
| attribute.String("component", "origin-cluster-client"), | ||
| attribute.String("namespace", namespace), | ||
| attribute.String("blob.digest", d.Hex()), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
The span attributes are missing the "operation" attribute that is present in other similar methods (UploadBlob and DownloadBlob) in this file. For consistency with the established tracing pattern in this codebase, consider adding attribute.String("operation", "prefetch_blob") to the attributes list.
1 participant
This PR adds context support to the PrefetchBlob method across the origin blob client infrastructure, enabling proper distributed tracing and request cancellation for blob prefetching operations.
Changes:
Updated PrefetchBlob interface signatures to accept context.Context as the first parameter
Added comprehensive OpenTelemetry tracing instrumentation to prefetch operations
Updated all call sites and tests to pass context appropriately