-
Notifications
You must be signed in to change notification settings - Fork 86
chore: refactor otel tracing of Storage to be a decorator rather than in method modification #2856
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
Changes from all commits
65cc5f7
2b1c574
2ef4ab6
e55c3a4
32d5850
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,10 +81,11 @@ | |
| import io.grpc.MethodDescriptor; | ||
| import io.grpc.Status; | ||
| import io.grpc.protobuf.ProtoUtils; | ||
| import io.opentelemetry.sdk.OpenTelemetrySdk; | ||
| import io.opentelemetry.api.OpenTelemetry; | ||
| import java.io.Closeable; | ||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.io.ObjectInputStream; | ||
| import java.io.Serializable; | ||
| import java.net.URI; | ||
| import java.nio.ByteBuffer; | ||
|
|
@@ -120,7 +121,7 @@ public final class GrpcStorageOptions extends StorageOptions | |
| private final boolean grpcClientMetricsManuallyEnabled; | ||
| private final GrpcInterceptorProvider grpcInterceptorProvider; | ||
| private final BlobWriteSessionConfig blobWriteSessionConfig; | ||
| private final OpenTelemetrySdk openTelemetrySdk; | ||
| private transient OpenTelemetry openTelemetry; | ||
|
|
||
| private GrpcStorageOptions(Builder builder, GrpcStorageDefaults serviceDefaults) { | ||
| super(builder, serviceDefaults); | ||
|
|
@@ -137,7 +138,7 @@ private GrpcStorageOptions(Builder builder, GrpcStorageDefaults serviceDefaults) | |
| this.grpcClientMetricsManuallyEnabled = builder.grpcMetricsManuallyEnabled; | ||
| this.grpcInterceptorProvider = builder.grpcInterceptorProvider; | ||
| this.blobWriteSessionConfig = builder.blobWriteSessionConfig; | ||
| this.openTelemetrySdk = builder.openTelemetrySdk; | ||
| this.openTelemetry = builder.openTelemetry; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -160,6 +161,11 @@ StorageSettings getStorageSettings() throws IOException { | |
| return resolveSettingsAndOpts().x(); | ||
| } | ||
|
|
||
| private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { | ||
| in.defaultReadObject(); | ||
| this.openTelemetry = HttpStorageOptions.getDefaultInstance().getOpenTelemetry(); | ||
| } | ||
|
|
||
| /** | ||
| * We have to perform several introspections and detections to cross-wire/support several features | ||
| * that are either gapic primitives, ServiceOption primitives or GCS semantic requirements. | ||
|
|
@@ -352,9 +358,11 @@ private Tuple<StorageSettings, Opts<UserProject>> resolveSettingsAndOpts() throw | |
| return Tuple.of(builder.build(), defaultOpts); | ||
| } | ||
|
|
||
| /** @since 2.47.0 This new api is in preview and is subject to breaking changes. */ | ||
| @BetaApi | ||
| @Override | ||
| public OpenTelemetrySdk getOpenTelemetrySdk() { | ||
| return openTelemetrySdk; | ||
| public OpenTelemetry getOpenTelemetry() { | ||
| return openTelemetry; | ||
| } | ||
|
|
||
| /** @since 2.14.0 */ | ||
|
|
@@ -372,6 +380,7 @@ public int hashCode() { | |
| enableGrpcClientMetrics, | ||
| grpcInterceptorProvider, | ||
| blobWriteSessionConfig, | ||
| openTelemetry, | ||
| baseHashCode()); | ||
| } | ||
|
|
||
|
|
@@ -390,6 +399,7 @@ public boolean equals(Object o) { | |
| && Objects.equals(terminationAwaitDuration, that.terminationAwaitDuration) | ||
| && Objects.equals(grpcInterceptorProvider, that.grpcInterceptorProvider) | ||
| && Objects.equals(blobWriteSessionConfig, that.blobWriteSessionConfig) | ||
| && Objects.equals(openTelemetry, that.openTelemetry) | ||
| && this.baseEquals(that); | ||
| } | ||
|
|
||
|
|
@@ -431,11 +441,10 @@ public static final class Builder extends StorageOptions.Builder { | |
| GrpcStorageDefaults.INSTANCE.grpcInterceptorProvider(); | ||
| private BlobWriteSessionConfig blobWriteSessionConfig = | ||
| GrpcStorageDefaults.INSTANCE.getDefaultStorageWriterConfig(); | ||
| private OpenTelemetry openTelemetry = GrpcStorageDefaults.INSTANCE.getDefaultOpenTelemetry(); | ||
|
|
||
| private boolean grpcMetricsManuallyEnabled = false; | ||
|
|
||
| private OpenTelemetrySdk openTelemetrySdk; | ||
|
|
||
| Builder() {} | ||
|
|
||
| Builder(StorageOptions options) { | ||
|
|
@@ -447,7 +456,7 @@ public static final class Builder extends StorageOptions.Builder { | |
| this.enableGrpcClientMetrics = gso.enableGrpcClientMetrics; | ||
| this.grpcInterceptorProvider = gso.grpcInterceptorProvider; | ||
| this.blobWriteSessionConfig = gso.blobWriteSessionConfig; | ||
| this.openTelemetrySdk = gso.openTelemetrySdk; | ||
| this.openTelemetry = gso.openTelemetry; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -633,13 +642,13 @@ public GrpcStorageOptions.Builder setBlobWriteSessionConfig( | |
| /** | ||
| * Enable OpenTelemetry Tracing and provide an instance for the client to use. | ||
| * | ||
| * @param openTelemetrySdk User defined instance of OpenTelemetry SDK to be used by the library | ||
| * @since 2.46.1 This new api is in preview and is subject to breaking changes. | ||
| * @param openTelemetry User defined instance of OpenTelemetry to be used by the library | ||
| * @since 2.47.0 This new api is in preview and is subject to breaking changes. | ||
| */ | ||
| @BetaApi | ||
| public GrpcStorageOptions.Builder setOpenTelemetrySdk(OpenTelemetrySdk openTelemetrySdk) { | ||
| requireNonNull(openTelemetrySdk, "openTelemetry must be non null"); | ||
| this.openTelemetrySdk = openTelemetrySdk; | ||
| public GrpcStorageOptions.Builder setOpenTelemetry(OpenTelemetry openTelemetry) { | ||
| requireNonNull(openTelemetry, "openTelemetry must be non null"); | ||
| this.openTelemetry = openTelemetry; | ||
| return this; | ||
| } | ||
|
|
||
|
|
@@ -719,6 +728,12 @@ public GrpcInterceptorProvider grpcInterceptorProvider() { | |
| public BlobWriteSessionConfig getDefaultStorageWriterConfig() { | ||
| return BlobWriteSessionConfigs.getDefault(); | ||
| } | ||
|
|
||
| /** @since 2.47.0 This new api is in preview and is subject to breaking changes. */ | ||
| @BetaApi | ||
| public OpenTelemetry getDefaultOpenTelemetry() { | ||
| return OpenTelemetry.noop(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -775,22 +790,27 @@ public Storage create(StorageOptions options) { | |
| new InternalZeroCopyGrpcStorageStub( | ||
| stubSettings, clientContext, grpcStorageCallableFactory); | ||
| StorageClient client = new InternalStorageClient(stub); | ||
| return new GrpcStorageImpl( | ||
| grpcStorageOptions, | ||
| client, | ||
| stub.getObjectMediaResponseMarshaller, | ||
| grpcStorageOptions.blobWriteSessionConfig.createFactory(Clock.systemUTC()), | ||
| defaultOpts); | ||
| GrpcStorageImpl grpcStorage = | ||
| new GrpcStorageImpl( | ||
| grpcStorageOptions, | ||
| client, | ||
| stub.getObjectMediaResponseMarshaller, | ||
| grpcStorageOptions.blobWriteSessionConfig.createFactory(Clock.systemUTC()), | ||
| defaultOpts); | ||
| return OtelStorageDecorator.decorate( | ||
| grpcStorage, options.getOpenTelemetry(), Transport.GRPC); | ||
| } else { | ||
| StorageClient client = StorageClient.create(storageSettings); | ||
| return new GrpcStorageImpl( | ||
| grpcStorageOptions, | ||
| client, | ||
| ResponseContentLifecycleManager.noop(), | ||
| grpcStorageOptions.blobWriteSessionConfig.createFactory(Clock.systemUTC()), | ||
| defaultOpts); | ||
| GrpcStorageImpl grpcStorage = | ||
| new GrpcStorageImpl( | ||
| grpcStorageOptions, | ||
| client, | ||
| ResponseContentLifecycleManager.noop(), | ||
| grpcStorageOptions.blobWriteSessionConfig.createFactory(Clock.systemUTC()), | ||
| defaultOpts); | ||
| return OtelStorageDecorator.decorate( | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Weave in the decorator if necessary |
||
| grpcStorage, options.getOpenTelemetry(), Transport.GRPC); | ||
| } | ||
|
|
||
| } catch (IOException e) { | ||
| throw new IllegalStateException( | ||
| "Unable to instantiate gRPC com.google.cloud.storage.Storage client.", e); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,7 +38,7 @@ | |
| import com.google.cloud.storage.spi.v1.StorageRpc; | ||
| import com.google.common.base.MoreObjects; | ||
| import com.google.common.collect.ImmutableSet; | ||
| import io.opentelemetry.sdk.OpenTelemetrySdk; | ||
| import io.opentelemetry.api.OpenTelemetry; | ||
| import java.io.IOException; | ||
| import java.io.ObjectInputStream; | ||
| import java.io.Serializable; | ||
|
|
@@ -60,7 +60,7 @@ public class HttpStorageOptions extends StorageOptions { | |
| private transient RetryDependenciesAdapter retryDepsAdapter; | ||
| private final BlobWriteSessionConfig blobWriteSessionConfig; | ||
|
|
||
| private final OpenTelemetrySdk openTelemetrySdk; | ||
| private transient OpenTelemetry openTelemetry; | ||
|
|
||
| private HttpStorageOptions(Builder builder, StorageDefaults serviceDefaults) { | ||
| super(builder, serviceDefaults); | ||
|
|
@@ -70,7 +70,7 @@ private HttpStorageOptions(Builder builder, StorageDefaults serviceDefaults) { | |
| builder.storageRetryStrategy, defaults().getStorageRetryStrategy())); | ||
| retryDepsAdapter = new RetryDependenciesAdapter(); | ||
| blobWriteSessionConfig = builder.blobWriteSessionConfig; | ||
| openTelemetrySdk = builder.openTelemetrySdk; | ||
| openTelemetry = builder.openTelemetry; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -88,9 +88,11 @@ StorageRpc getStorageRpcV1() { | |
| return (StorageRpc) getRpc(); | ||
| } | ||
|
|
||
| /** @since 2.47.0 This new api is in preview and is subject to breaking changes. */ | ||
| @BetaApi | ||
| @Override | ||
| public OpenTelemetrySdk getOpenTelemetrySdk() { | ||
| return openTelemetrySdk; | ||
| public OpenTelemetry getOpenTelemetry() { | ||
| return openTelemetry; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -100,7 +102,8 @@ public HttpStorageOptions.Builder toBuilder() { | |
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(retryAlgorithmManager, blobWriteSessionConfig, baseHashCode()); | ||
| return Objects.hash( | ||
| retryAlgorithmManager, blobWriteSessionConfig, openTelemetry, baseHashCode()); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -114,12 +117,14 @@ public boolean equals(Object o) { | |
| HttpStorageOptions that = (HttpStorageOptions) o; | ||
| return Objects.equals(retryAlgorithmManager, that.retryAlgorithmManager) | ||
| && Objects.equals(blobWriteSessionConfig, that.blobWriteSessionConfig) | ||
| && Objects.equals(openTelemetry, that.openTelemetry) | ||
| && this.baseEquals(that); | ||
| } | ||
|
|
||
| private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { | ||
| in.defaultReadObject(); | ||
| this.retryDepsAdapter = new RetryDependenciesAdapter(); | ||
| this.openTelemetry = HttpStorageOptions.getDefaultInstance().getOpenTelemetry(); | ||
| } | ||
|
|
||
| public static HttpStorageOptions.Builder newBuilder() { | ||
|
|
@@ -144,7 +149,7 @@ public static class Builder extends StorageOptions.Builder { | |
| private StorageRetryStrategy storageRetryStrategy; | ||
| private BlobWriteSessionConfig blobWriteSessionConfig = | ||
| HttpStorageDefaults.INSTANCE.getDefaultStorageWriterConfig(); | ||
| private OpenTelemetrySdk openTelemetrySdk; | ||
| private OpenTelemetry openTelemetry = HttpStorageDefaults.INSTANCE.getDefaultOpenTelemetry(); | ||
|
|
||
| Builder() {} | ||
|
|
||
|
|
@@ -153,7 +158,7 @@ public static class Builder extends StorageOptions.Builder { | |
| HttpStorageOptions hso = (HttpStorageOptions) options; | ||
| this.storageRetryStrategy = hso.retryAlgorithmManager.retryStrategy; | ||
| this.blobWriteSessionConfig = hso.blobWriteSessionConfig; | ||
| this.openTelemetrySdk = hso.getOpenTelemetrySdk(); | ||
| this.openTelemetry = hso.getOpenTelemetry(); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -283,13 +288,13 @@ public HttpStorageOptions build() { | |
| /** | ||
| * Enable OpenTelemetry Tracing and provide an instance for the client to use. | ||
| * | ||
| * @param openTelemetrySdk | ||
| * @since 2.46.1 This new api is in preview and is subject to breaking changes. | ||
| * @param openTelemetry User defined instance of OpenTelemetry to be used by the library | ||
| * @since 2.47.0 This new api is in preview and is subject to breaking changes. | ||
| */ | ||
| @BetaApi | ||
| public HttpStorageOptions.Builder setOpenTelemetrySdk(OpenTelemetrySdk openTelemetrySdk) { | ||
| requireNonNull(openTelemetrySdk, "openTelemetry must be non null"); | ||
| this.openTelemetrySdk = openTelemetrySdk; | ||
| public HttpStorageOptions.Builder setOpenTelemetry(OpenTelemetry openTelemetry) { | ||
| requireNonNull(openTelemetry, "openTelemetry must be non null"); | ||
| this.openTelemetry = openTelemetry; | ||
| return this; | ||
| } | ||
| } | ||
|
|
@@ -325,6 +330,12 @@ public StorageRetryStrategy getStorageRetryStrategy() { | |
| public BlobWriteSessionConfig getDefaultStorageWriterConfig() { | ||
| return BlobWriteSessionConfigs.getDefault(); | ||
| } | ||
|
|
||
| /** @since 2.47.0 This new api is in preview and is subject to breaking changes. */ | ||
| @BetaApi | ||
| public OpenTelemetry getDefaultOpenTelemetry() { | ||
| return OpenTelemetry.noop(); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sure we default to a non-null value |
||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -365,8 +376,12 @@ public Storage create(StorageOptions options) { | |
| HttpStorageOptions httpStorageOptions = (HttpStorageOptions) options; | ||
| Clock clock = Clock.systemUTC(); | ||
| try { | ||
| return new StorageImpl( | ||
| httpStorageOptions, httpStorageOptions.blobWriteSessionConfig.createFactory(clock)); | ||
| StorageImpl storage = | ||
| new StorageImpl( | ||
| httpStorageOptions, | ||
| httpStorageOptions.blobWriteSessionConfig.createFactory(clock)); | ||
| return OtelStorageDecorator.decorate( | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Weave in the decorator if necessary |
||
| storage, httpStorageOptions.getOpenTelemetry(), Transport.HTTP); | ||
| } catch (IOException e) { | ||
| throw new IllegalStateException( | ||
| "Unable to instantiate HTTP com.google.cloud.storage.Storage client.", e); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure we default to a non-null value