Skip to content

feat(storage): add CalculateChecksum spans for upload writers - #20372

Open
sahusneha2004 wants to merge 3 commits into
googleapis:mainfrom
sahusneha2004:feat/otel-writer-checksums
Open

feat(storage): add CalculateChecksum spans for upload writers#20372
sahusneha2004 wants to merge 3 commits into
googleapis:mainfrom
sahusneha2004:feat/otel-writer-checksums

Conversation

@sahusneha2004

@sahusneha2004 sahusneha2004 commented Aug 12, 2026

Copy link
Copy Markdown
  • Emits T5 Storage.CalculateChecksum spans during single-shot HTTP uploads when verifying client-side CRC against the server-returned CRC.
  • Emits chunk Storage.CalculateChecksum spans during one-shot and resumable gRPC chunk writes.
  • Includes unit test in storage/trace_test.go.

@sahusneha2004
sahusneha2004 requested review from a team as code owners August 12, 2026 17:07
@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label Aug 12, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces OpenTelemetry tracing spans for CRC32C checksum calculations and verifications across both gRPC and HTTP storage clients. The feedback highlights two critical issues: first, when OTel tracing is disabled, returning the parent span from startChecksumSpan will cause endSpan to prematurely terminate the parent span, which should be resolved by returning a no-op span instead. Second, adding the len(r.buf) > 0 check to the outer condition in the gRPC writers prevents the checksum from being populated for empty buffers, altering the original behavior; the checksum should be computed unconditionally while only wrapping the calculation in a span if the buffer is non-empty.

Comment thread storage/trace.go
Comment thread storage/grpc_writer.go Outdated
Comment thread storage/grpc_writer.go Outdated
@sahusneha2004
sahusneha2004 force-pushed the feat/otel-writer-checksums branch from b45beff to 73cdb20 Compare August 13, 2026 08:18
@sahusneha2004
sahusneha2004 force-pushed the feat/otel-writer-checksums branch from 73cdb20 to 0f19584 Compare August 17, 2026 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant