diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java index 2735676f7..5a16b6c29 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java @@ -201,8 +201,7 @@ public void close() throws Exception { @Override public Bucket create(BucketInfo bucketInfo, BucketTargetOption... options) { - OpenTelemetryTraceUtil.Span otelSpan = - openTelemetryTraceUtil.startSpan("create(Bucket, BucketTargetOption"); + OpenTelemetryTraceUtil.Span otelSpan = openTelemetryTraceUtil.startSpan("create"); Opts opts = Opts.unwrap(options).resolveFrom(bucketInfo).prepend(defaultOpts); GrpcCallContext grpcCallContext = opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); @@ -249,8 +248,7 @@ public Blob create( BlobInfo blobInfo, byte[] content, int offset, int length, BlobTargetOption... options) { Opts opts = Opts.unwrap(options).resolveFrom(blobInfo); // Start the otel span to retain information of the origin of the request - OpenTelemetryTraceUtil.Span otelSpan = - openTelemetryTraceUtil.startSpan("create(BlobInfo, BlobTargetOption"); + OpenTelemetryTraceUtil.Span otelSpan = openTelemetryTraceUtil.startSpan("create"); try (OpenTelemetryTraceUtil.Scope unused = otelSpan.makeCurrent()) { return internalDirectUpload( blobInfo, @@ -269,7 +267,8 @@ public Blob create( @Override public Blob create(BlobInfo blobInfo, InputStream content, BlobWriteOption... options) { - try { + OpenTelemetryTraceUtil.Span otelSpan = openTelemetryTraceUtil.startSpan("create"); + try (OpenTelemetryTraceUtil.Scope ununsed = otelSpan.makeCurrent()) { requireNonNull(blobInfo, "blobInfo must be non null"); InputStream inputStreamParam = firstNonNull(content, new ByteArrayInputStream(ZERO_BYTES)); @@ -296,7 +295,11 @@ public Blob create(BlobInfo blobInfo, InputStream content, BlobWriteOption... op ApiFuture responseApiFuture = session.getResult(); return this.getBlob(responseApiFuture); } catch (IOException | ApiException e) { + otelSpan.recordException(e); + otelSpan.setStatus(io.opentelemetry.api.trace.StatusCode.ERROR, e.getClass().getSimpleName()); throw StorageException.coalesce(e); + } finally { + otelSpan.end(); } } @@ -812,6 +815,12 @@ public GrpcBlobWriteChannel writer(BlobInfo blobInfo, BlobWriteOption... options hasher); } + @Override + public BlobInfo internalDirectUpload( + BlobInfo blobInfo, Opts opts, ByteBuffer buf) { + return internalDirectUpload(blobInfo, opts, buf, null); + } + @Override public BlobInfo internalDirectUpload( BlobInfo blobInfo, diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannel.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannel.java index bb4edc2e3..0d08759d1 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannel.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannel.java @@ -219,8 +219,8 @@ public void close() throws IOException { // We never created any parts // create an empty object try { - BlobInfo blobInfo = - storage.internalDirectUpload(ultimateObject, opts, Buffers.allocate(0), null); + // TODO: Add in Otel context when available + BlobInfo blobInfo = storage.internalDirectUpload(ultimateObject, opts, Buffers.allocate(0)); finalObject.set(blobInfo); return; } catch (StorageException se) { @@ -287,7 +287,7 @@ private void internalFlush(ByteBuffer buf) { info -> { try { // TODO: Add in Otel context when available - return storage.internalDirectUpload(info, partOpts, buf, null); + return storage.internalDirectUpload(info, partOpts, buf); } catch (StorageException e) { // a precondition failure usually means the part was created, but we didn't get the // response. And when we tried to retry the object already exists. diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageInternal.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageInternal.java index 627654f56..403f03e9e 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageInternal.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageInternal.java @@ -32,6 +32,11 @@ default BlobInfo internalCreateFrom(Path path, BlobInfo info, Opts opts, ByteBuffer buf) { + throw new UnsupportedOperationException("not implemented"); + } + default BlobInfo internalDirectUpload( BlobInfo info, Opts opts, diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcOpenTelemetryTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcOpenTelemetryTest.java index d3d8e6119..98d24ba15 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcOpenTelemetryTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcOpenTelemetryTest.java @@ -88,9 +88,6 @@ public void runCreateBlob() { storage.create(BlobInfo.newBuilder(toCreate).build(), content); TestExporter testExported = (TestExporter) exporter; List spanData = testExported.getExportedSpans(); - // (1) Span when calling create - // (2) Span when passing call to internalDirectUpload - Assert.assertEquals(2, spanData.size()); for (SpanData span : spanData) { Assert.assertEquals("Storage", getAttributeValue(span, "gcp.client.service")); Assert.assertEquals("googleapis/java-storage", getAttributeValue(span, "gcp.client.repo")); @@ -98,6 +95,9 @@ public void runCreateBlob() { "com.google.cloud.google-cloud-storage", getAttributeValue(span, "gcp.client.artifact")); Assert.assertEquals("grpc", getAttributeValue(span, "rpc.system")); } + Assert.assertTrue(spanData.stream().anyMatch(x -> x.getName().contains("create"))); + Assert.assertTrue( + spanData.stream().anyMatch(x -> x.getName().contains("internalDirectUpload"))); Assert.assertEquals(spanData.get(1).getSpanContext(), spanData.get(0).getParentSpanContext()); } diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannelTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannelTest.java index 1e57b2a0b..48bb4137e 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannelTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannelTest.java @@ -39,7 +39,6 @@ import com.google.cloud.storage.UnifiedOpts.ObjectSourceOpt; import com.google.cloud.storage.UnifiedOpts.ObjectTargetOpt; import com.google.cloud.storage.UnifiedOpts.Opts; -import com.google.cloud.storage.otel.OpenTelemetryTraceUtil; import com.google.cloud.storage.spi.v1.StorageRpc; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; @@ -357,12 +356,9 @@ public void partsRetainMetadata() throws Exception { new FakeStorageInternal() { @Override public BlobInfo internalDirectUpload( - BlobInfo info, - Opts opts, - ByteBuffer buf, - OpenTelemetryTraceUtil.Context ctx) { + BlobInfo info, Opts opts, ByteBuffer buf) { metadatas.add(info.getMetadata()); - return super.internalDirectUpload(info, opts, buf, null); + return super.internalDirectUpload(info, opts, buf); } @Override @@ -450,10 +446,7 @@ public void creatingAnEmptyObjectWhichFailsIsSetAsResultFailureAndThrowFromClose new FakeStorageInternal() { @Override public BlobInfo internalDirectUpload( - BlobInfo info, - Opts opts, - ByteBuffer buf, - OpenTelemetryTraceUtil.Context ctx) { + BlobInfo info, Opts opts, ByteBuffer buf) { throw StorageException.coalesce( ApiExceptionFactory.createException( null, GrpcStatusCode.of(Code.PERMISSION_DENIED), false)); @@ -564,10 +557,7 @@ public void shortCircuitExceptionResultsInFastFailure() throws Exception { new FakeStorageInternal() { @Override public BlobInfo internalDirectUpload( - BlobInfo info, - Opts opts, - ByteBuffer buf, - OpenTelemetryTraceUtil.Context ctx) { + BlobInfo info, Opts opts, ByteBuffer buf) { if (induceFailure.getAndSet(false)) { Uninterruptibles.awaitUninterruptibly(blockForWrite1); try { @@ -581,7 +571,7 @@ public BlobInfo internalDirectUpload( blockForWrite1Complete.countDown(); } } else { - return super.internalDirectUpload(info, opts, buf, null); + return super.internalDirectUpload(info, opts, buf); } } }; @@ -724,11 +714,8 @@ public void partFailedPreconditionOnRetryIsHandledGracefully() throws Exception new FakeStorageInternal() { @Override public BlobInfo internalDirectUpload( - BlobInfo info, - Opts opts, - ByteBuffer buf, - OpenTelemetryTraceUtil.Context ctx) { - BlobInfo blobInfo = super.internalDirectUpload(info, opts, buf, null); + BlobInfo info, Opts opts, ByteBuffer buf) { + BlobInfo blobInfo = super.internalDirectUpload(info, opts, buf); if (info.getName().equals(p1.getName())) { throw StorageException.coalesce( ApiExceptionFactory.createException( @@ -791,10 +778,7 @@ public void partMetadataFieldDecoratorUsesCustomTime() throws IOException { new FakeStorageInternal() { @Override public BlobInfo internalDirectUpload( - BlobInfo info, - Opts opts, - ByteBuffer buf, - OpenTelemetryTraceUtil.Context ctx) { + BlobInfo info, Opts opts, ByteBuffer buf) { if (info.getBlobId().getName().endsWith(".part")) { // Kinda hacky but since we are creating multiple parts we will use a range // to ensure the customTimes are being calculated appropriately @@ -803,7 +787,7 @@ public BlobInfo internalDirectUpload( } else { assertThat(info.getCustomTimeOffsetDateTime()).isNull(); } - return super.internalDirectUpload(info, opts, buf, null); + return super.internalDirectUpload(info, opts, buf); } }; ParallelCompositeUploadWritableByteChannel pcu = @@ -859,10 +843,7 @@ private static class FakeStorageInternal implements StorageInternal { @Override public BlobInfo internalDirectUpload( - BlobInfo info, - Opts opts, - ByteBuffer buf, - OpenTelemetryTraceUtil.Context ctx) { + BlobInfo info, Opts opts, ByteBuffer buf) { BlobId id = info.getBlobId(); BlobInfo.Builder b = info.toBuilder();