Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: misc otel internalization/cleanup #2835

Merged
merged 1 commit into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static com.google.cloud.storage.StorageV2ProtoUtils.objectAclEntityOrAltEq;
import static com.google.cloud.storage.Utils.bucketNameCodec;
import static com.google.cloud.storage.Utils.ifNonNull;
import static com.google.cloud.storage.otel.OpenTelemetryTraceUtil.MODULE_STORAGE;
import static com.google.common.base.MoreObjects.firstNonNull;
import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -204,7 +205,7 @@ public void close() throws Exception {
@Override
public Bucket create(BucketInfo bucketInfo, BucketTargetOption... options) {
OpenTelemetryTraceUtil.Span otelSpan =
openTelemetryTraceUtil.startSpan("create", this.getClass().getName());
openTelemetryTraceUtil.startSpan("create", MODULE_STORAGE);
Opts<BucketTargetOpt> opts = Opts.unwrap(options).resolveFrom(bucketInfo).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
Expand All @@ -219,7 +220,7 @@ public Bucket create(BucketInfo bucketInfo, BucketTargetOption... options) {
.setParent("projects/_");
CreateBucketRequest req = opts.createBucketsRequest().apply(builder).build();
GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext());
try (OpenTelemetryTraceUtil.Scope unused = otelSpan.makeCurrent()) {
try (OpenTelemetryTraceUtil.Scope ignored = otelSpan.makeCurrent()) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

renamed variables to remove warnings from intellij.

return Retrying.run(
getOptions(),
retryAlgorithmManager.getFor(req),
Expand Down Expand Up @@ -252,8 +253,8 @@ public Blob create(
Opts<ObjectTargetOpt> 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", this.getClass().getName());
try (OpenTelemetryTraceUtil.Scope unused = otelSpan.makeCurrent()) {
openTelemetryTraceUtil.startSpan("create", MODULE_STORAGE);
try (OpenTelemetryTraceUtil.Scope ignored = otelSpan.makeCurrent()) {
return internalDirectUpload(
blobInfo,
opts,
Expand All @@ -272,8 +273,8 @@ public Blob create(
@Override
public Blob create(BlobInfo blobInfo, InputStream content, BlobWriteOption... options) {
OpenTelemetryTraceUtil.Span otelSpan =
openTelemetryTraceUtil.startSpan("create", this.getClass().getName());
try (OpenTelemetryTraceUtil.Scope ununsed = otelSpan.makeCurrent()) {
openTelemetryTraceUtil.startSpan("create", MODULE_STORAGE);
try (OpenTelemetryTraceUtil.Scope ignored = otelSpan.makeCurrent()) {
requireNonNull(blobInfo, "blobInfo must be non null");
InputStream inputStreamParam = firstNonNull(content, new ByteArrayInputStream(ZERO_BYTES));

Expand Down Expand Up @@ -318,8 +319,8 @@ public Blob createFrom(BlobInfo blobInfo, Path path, BlobWriteOption... options)
public Blob createFrom(BlobInfo blobInfo, Path path, int bufferSize, BlobWriteOption... options)
throws IOException {
OpenTelemetryTraceUtil.Span otelSpan =
openTelemetryTraceUtil.startSpan("createFrom", this.getClass().getName());
try (OpenTelemetryTraceUtil.Scope unused = otelSpan.makeCurrent()) {
openTelemetryTraceUtil.startSpan("createFrom", MODULE_STORAGE);
try (OpenTelemetryTraceUtil.Scope ignored = otelSpan.makeCurrent()) {
Opts<ObjectTargetOpt> opts = Opts.unwrap(options).resolveFrom(blobInfo).prepend(defaultOpts);
return internalCreateFrom(path, blobInfo, opts, openTelemetryTraceUtil.currentContext());
} catch (Exception e) {
Expand All @@ -336,12 +337,12 @@ public Blob internalCreateFrom(
Path path, BlobInfo info, Opts<ObjectTargetOpt> opts, OpenTelemetryTraceUtil.Context ctx)
throws IOException {
OpenTelemetryTraceUtil.Span otelSpan =
openTelemetryTraceUtil.startSpan("internalCreateFrom", this.getClass().getName(), ctx);
openTelemetryTraceUtil.startSpan("internalCreateFrom", MODULE_STORAGE, ctx);
requireNonNull(path, "path must be non null");
if (Files.isDirectory(path)) {
throw new StorageException(0, path + " is a directory");
}
try (OpenTelemetryTraceUtil.Scope unused = otelSpan.makeCurrent()) {
try (OpenTelemetryTraceUtil.Scope ignored = otelSpan.makeCurrent()) {
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
WriteObjectRequest req = getWriteObjectRequest(info, opts);
Expand Down Expand Up @@ -391,8 +392,8 @@ public Blob createFrom(
BlobInfo blobInfo, InputStream in, int bufferSize, BlobWriteOption... options)
throws IOException {
OpenTelemetryTraceUtil.Span otelSpan =
openTelemetryTraceUtil.startSpan("createFrom", this.getClass().getName());
try (OpenTelemetryTraceUtil.Scope unused = otelSpan.makeCurrent()) {
openTelemetryTraceUtil.startSpan("createFrom", MODULE_STORAGE);
try (OpenTelemetryTraceUtil.Scope ignored = otelSpan.makeCurrent()) {
requireNonNull(blobInfo, "blobInfo must be non null");

Opts<ObjectTargetOpt> opts = Opts.unwrap(options).resolveFrom(blobInfo).prepend(defaultOpts);
Expand Down Expand Up @@ -700,8 +701,8 @@ public Blob compose(ComposeRequest composeRequest) {

@Override
public CopyWriter copy(CopyRequest copyRequest) {
Span otelSpan = openTelemetryTraceUtil.startSpan("copy", this.getClass().getName());
try (Scope unused = otelSpan.makeCurrent()) {
Span otelSpan = openTelemetryTraceUtil.startSpan("copy", MODULE_STORAGE);
try (Scope ignored = otelSpan.makeCurrent()) {
BlobId src = copyRequest.getSource();
BlobInfo dst = copyRequest.getTarget();
Opts<ObjectSourceOpt> srcOpts =
Expand Down Expand Up @@ -770,11 +771,11 @@ public byte[] readAllBytes(String bucket, String blob, BlobSourceOption... optio
@Override
public byte[] readAllBytes(BlobId blob, BlobSourceOption... options) {
OpenTelemetryTraceUtil.Span otelSpan =
openTelemetryTraceUtil.startSpan("readAllBytes", this.getClass().getName());
openTelemetryTraceUtil.startSpan("readAllBytes", MODULE_STORAGE);
UnbufferedReadableByteChannelSession<Object> session = unbufferedReadSession(blob, options);

ByteArrayOutputStream baos = new ByteArrayOutputStream();
try (OpenTelemetryTraceUtil.Scope unused = otelSpan.makeCurrent();
try (OpenTelemetryTraceUtil.Scope ignored = otelSpan.makeCurrent();
UnbufferedReadableByteChannel r = session.open();
WritableByteChannel w = Channels.newChannel(baos)) {
ByteStreams.copy(r, w);
Expand Down Expand Up @@ -815,11 +816,11 @@ public GrpcBlobReadChannel reader(BlobId blob, BlobSourceOption... options) {

@Override
public void downloadTo(BlobId blob, Path path, BlobSourceOption... options) {
Span otelSpan = openTelemetryTraceUtil.startSpan("downloadTo", this.getClass().getName());
Span otelSpan = openTelemetryTraceUtil.startSpan("downloadTo", MODULE_STORAGE);

UnbufferedReadableByteChannelSession<Object> session = unbufferedReadSession(blob, options);

try (Scope unused = otelSpan.makeCurrent();
try (Scope ignored = otelSpan.makeCurrent();
UnbufferedReadableByteChannel r = session.open();
WritableByteChannel w = Files.newByteChannel(path, WRITE_OPS)) {
ByteStreams.copy(r, w);
Expand All @@ -834,11 +835,11 @@ public void downloadTo(BlobId blob, Path path, BlobSourceOption... options) {

@Override
public void downloadTo(BlobId blob, OutputStream outputStream, BlobSourceOption... options) {
Span otelSpan = openTelemetryTraceUtil.startSpan("downloadTo", this.getClass().getName());
Span otelSpan = openTelemetryTraceUtil.startSpan("downloadTo", MODULE_STORAGE);

UnbufferedReadableByteChannelSession<Object> session = unbufferedReadSession(blob, options);

try (Scope unused = otelSpan.makeCurrent();
try (Scope ignored = otelSpan.makeCurrent();
UnbufferedReadableByteChannel r = session.open();
WritableByteChannel w = Channels.newChannel(outputStream)) {
ByteStreams.copy(r, w);
Expand Down Expand Up @@ -889,15 +890,15 @@ public BlobInfo internalDirectUpload(
requireNonNull(blobInfo, "blobInfo must be non null");
requireNonNull(buf, "content must be non null");
OpenTelemetryTraceUtil.Span otelSpan =
openTelemetryTraceUtil.startSpan("internalDirectUpload", this.getClass().getName(), ctx);
openTelemetryTraceUtil.startSpan("internalDirectUpload", MODULE_STORAGE, ctx);
Opts<ObjectTargetOpt> optsWithDefaults = opts.prepend(defaultOpts);
GrpcCallContext grpcCallContext =
optsWithDefaults.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
WriteObjectRequest req = getWriteObjectRequest(blobInfo, optsWithDefaults);
Hasher hasher = Hasher.enabled();
GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext());
RewindableContent content = RewindableContent.of(buf);
try (OpenTelemetryTraceUtil.Scope unused = otelSpan.makeCurrent()) {
try (OpenTelemetryTraceUtil.Scope ignored = otelSpan.makeCurrent()) {
return Retrying.run(
getOptions(),
retryAlgorithmManager.getFor(req),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.cloud.storage;

import static com.google.cloud.storage.SignedUrlEncodingHelper.Rfc3986UriEncode;
import static com.google.cloud.storage.otel.OpenTelemetryTraceUtil.MODULE_STORAGE;
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
Expand Down Expand Up @@ -247,8 +248,8 @@ public Blob createFrom(BlobInfo blobInfo, Path path, BlobWriteOption... options)
public Blob createFrom(BlobInfo blobInfo, Path path, int bufferSize, BlobWriteOption... options)
throws IOException {
OpenTelemetryTraceUtil.Span otelSpan =
openTelemetryTraceUtil.startSpan("createFrom", this.getClass().getName());
try (OpenTelemetryTraceUtil.Scope scope = otelSpan.makeCurrent()) {
openTelemetryTraceUtil.startSpan("createFrom", MODULE_STORAGE);
try (OpenTelemetryTraceUtil.Scope ignored = otelSpan.makeCurrent()) {
if (Files.isDirectory(path)) {
throw new StorageException(0, path + " is a directory");
}
Expand Down Expand Up @@ -308,8 +309,8 @@ public Blob createFrom(
BlobInfo blobInfo, InputStream content, int bufferSize, BlobWriteOption... options)
throws IOException {
OpenTelemetryTraceUtil.Span otelSpan =
openTelemetryTraceUtil.startSpan("createFrom", this.getClass().getName());
try (OpenTelemetryTraceUtil.Scope scope = otelSpan.makeCurrent()) {
openTelemetryTraceUtil.startSpan("createFrom", MODULE_STORAGE);
try (OpenTelemetryTraceUtil.Scope ignored = otelSpan.makeCurrent()) {

ApiFuture<BlobInfo> objectFuture;
try (StorageWriteChannel writer = writer(blobInfo, options)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@

package com.google.cloud.storage.otel;

import static com.google.common.base.Preconditions.checkArgument;

import com.google.api.core.ApiFuture;
import com.google.api.gax.core.GaxProperties;
import com.google.cloud.storage.GrpcStorageOptions;
import com.google.cloud.storage.StorageOptions;
import com.google.cloud.storage.otel.OpenTelemetryTraceUtil.Context;
import com.google.cloud.storage.otel.OpenTelemetryTraceUtil.Span;
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.Attributes;
Expand All @@ -42,19 +41,18 @@ class OpenTelemetryInstance implements OpenTelemetryTraceUtil {

private final String transport;

public OpenTelemetryInstance(StorageOptions storageOptions) {
OpenTelemetryInstance(StorageOptions storageOptions) {
this.storageOptions = storageOptions;
this.openTelemetry = storageOptions.getOpenTelemetrySdk();
this.tracer =
openTelemetry.getTracer(LIBRARY_NAME, GaxProperties.getLibraryVersion(this.getClass()));
this.tracer = openTelemetry.getTracer(LIBRARY_NAME, storageOptions.getLibraryVersion());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Switch to using the logic from storageOptions rather than gax. Our version is more stable to repackaging, and is what is used for headers and such. We will want to keep them in sync.

this.transport = storageOptions instanceof GrpcStorageOptions ? "grpc" : "http";
}

static class Span implements OpenTelemetryTraceUtil.Span {
private final io.opentelemetry.api.trace.Span span;
private final String spanName;

Span(io.opentelemetry.api.trace.Span span, String spanName) {
private Span(io.opentelemetry.api.trace.Span span, String spanName) {
this.span = span;
this.spanName = spanName;
}
Expand Down Expand Up @@ -129,7 +127,7 @@ public <T> void endAtFuture(ApiFuture<T> futureValue) {}
static class Scope implements OpenTelemetryTraceUtil.Scope {
private final io.opentelemetry.context.Scope scope;

Scope(io.opentelemetry.context.Scope scope) {
private Scope(io.opentelemetry.context.Scope scope) {
this.scope = scope;
}

Expand All @@ -142,7 +140,7 @@ public void close() {
static class Context implements OpenTelemetryTraceUtil.Context {
private final io.opentelemetry.context.Context context;

Context(io.opentelemetry.context.Context context) {
private Context(io.opentelemetry.context.Context context) {
this.context = context;
}

Expand All @@ -164,13 +162,13 @@ public OpenTelemetryTraceUtil.Span startSpan(String methodName, String module) {
@Override
public OpenTelemetryTraceUtil.Span startSpan(
String methodName, String module, OpenTelemetryTraceUtil.Context parent) {
assert (parent instanceof OpenTelemetryInstance.Context);
checkArgument(
parent instanceof OpenTelemetryInstance.Context,
"parent must be an instance of " + OpenTelemetryInstance.Context.class.getName());
String formatSpanName = String.format("%s/%s", module, methodName);
Context p2 = (Context) parent;
SpanBuilder spanBuilder =
tracer
.spanBuilder(formatSpanName)
.setSpanKind(SpanKind.CLIENT)
.setParent(((OpenTelemetryInstance.Context) parent).context);
tracer.spanBuilder(formatSpanName).setSpanKind(SpanKind.CLIENT).setParent(p2.context);
io.opentelemetry.api.trace.Span span =
addSettingsAttributesToCurrentSpan(spanBuilder).startSpan();
return new Span(span, formatSpanName);
Expand All @@ -193,9 +191,9 @@ private SpanBuilder addSettingsAttributesToCurrentSpan(SpanBuilder spanBuilder)
spanBuilder =
spanBuilder.setAllAttributes(
Attributes.builder()
.put("gcp.client.version", GaxProperties.getLibraryVersion(this.getClass()))
.put("gcp.client.version", storageOptions.getLibraryVersion())
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ditto

.put("gcp.client.repo", "googleapis/java-storage")
.put("gcp.client.artifact", "com.google.cloud.google-cloud-storage")
.put("gcp.client.artifact", "com.google.cloud:google-cloud-storage")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

use the maven package format

.put("rpc.system", transport)
.build());
return spanBuilder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,20 @@
package com.google.cloud.storage.otel;

import com.google.api.core.ApiFuture;
import com.google.api.core.InternalApi;
import com.google.cloud.storage.Storage;
import com.google.cloud.storage.StorageOptions;
import com.google.cloud.storage.spi.v1.StorageRpc;
import io.opentelemetry.api.trace.StatusCode;
import java.util.Map;
import javax.annotation.Nonnull;

@InternalApi
public interface OpenTelemetryTraceUtil {
String MODULE_STORAGE = Storage.class.getName();
String MODULE_STORAGE_RPC = StorageRpc.class.getName();

@InternalApi
static OpenTelemetryTraceUtil getInstance(@Nonnull StorageOptions storageOptions) {
boolean createNoOp = storageOptions.getOpenTelemetrySdk() == null;

Expand All @@ -35,23 +42,31 @@ static OpenTelemetryTraceUtil getInstance(@Nonnull StorageOptions storageOptions
}

/** Represents a trace span. */
@InternalApi
interface Span {
@InternalApi
Span recordException(Throwable error);

@InternalApi
Span setStatus(StatusCode status, String name);
/** Adds the given event to this span. */
@InternalApi
Span addEvent(String name);

/** Adds the given event with the given attributes to this span. */
@InternalApi
Span addEvent(String name, Map<String, Object> attributes);

/** Marks this span as the current span. */
@InternalApi
Scope makeCurrent();

/** Ends this span. */
@InternalApi
void end();

/** Ends this span in an error. */
@InternalApi
void end(Throwable error);

/**
Expand All @@ -60,35 +75,44 @@ interface Span {
* future. In order for telemetry info to be recorded, the future returned by this method should
* be completed.
*/
@InternalApi
<T> void endAtFuture(ApiFuture<T> futureValue);
}

/** Represents a trace context. */
@InternalApi
interface Context {
/** Makes this context the current context. */
@InternalApi
Scope makeCurrent();
}

/** Represents a trace scope. */
@InternalApi
interface Scope extends AutoCloseable {
/** Closes the current scope. */
@InternalApi
void close();
}

/** Starts a new span with the given name, sets it as the current span, and returns it. */
@InternalApi
Span startSpan(String spanName, String module);

/**
* Starts a new span with the given name and the given context as its parent, sets it as the
* current span, and returns it.
*/
@InternalApi
Span startSpan(String spanName, String module, Context parent);

/** Returns the current span. */
@Nonnull
@InternalApi
Span currentSpan();

/** Returns the current Context. */
@Nonnull
@InternalApi
Context currentContext();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright 2024 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* Set of internal utilities to make our OTel use a bit more terse.
*
* <p>All classes, interfaces, etc are considered to be for internal library use only and can break
* at any time.
*/
@InternalApi
package com.google.cloud.storage.otel;

import com.google.api.core.InternalApi;
Loading
Loading