diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageGrpcMetricsInterceptor.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageGrpcMetricsInterceptor.java index a09d6c37b06e..5b8f9cca783e 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageGrpcMetricsInterceptor.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageGrpcMetricsInterceptor.java @@ -53,11 +53,13 @@ final class StorageGrpcMetricsInterceptor implements ClientInterceptor { @Override public ClientCall interceptCall( MethodDescriptor method, CallOptions callOptions, Channel next) { - if (metrics == null || metrics.getRpcClientCallDuration() == null) { + if (metrics == null + || (metrics.getRpcClientCallDuration() == null && metrics.getTtfb() == null)) { return next.newCall(method, callOptions); } String shortMethod = extractShortMethodName(method.getFullMethodName()); + boolean isReadMethod = "ReadObject".equals(shortMethod) || "BidiReadObject".equals(shortMethod); String initialServerAddress = StorageOptions.extractServerAddress( defaultEndpoint != null ? defaultEndpoint : next.authority()); @@ -74,9 +76,32 @@ public ClientCall interceptCall( public void start(Listener responseListener, Metadata headers) { long startTimeNanos = clock.nanoTime(); AtomicBoolean callRecorded = new AtomicBoolean(false); + AtomicBoolean ttfbRecorded = new AtomicBoolean(false); super.start( new SimpleForwardingClientCallListener(responseListener) { + @Override + public void onMessage(RespT message) { + if (isReadMethod + && metrics.getTtfb() != null + && ttfbRecorded.compareAndSet(false, true)) { + double ttfbSeconds = + Math.max(0.0, (clock.nanoTime() - startTimeNanos) / 1_000_000_000.0); + String networkPath = detectNetworkPath(getAttributes(), defaultEndpoint); + AttributesBuilder ttfbAttrs = + metrics + .clientAttributesBuilder() + .put(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc") + .put(StorageClientMetrics.KEY_RPC_METHOD, shortMethod) + .put(StorageClientMetrics.KEY_NETWORK_PATH, networkPath) + .put( + StorageClientMetrics.KEY_SERVER_ADDRESS, + serverAddress != null ? serverAddress : ""); + metrics.getTtfb().record(ttfbSeconds, ttfbAttrs.build(), otelContext); + } + super.onMessage(message); + } + @Override public void onClose(Status status, Metadata trailers) { try { diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageHttpMetricsInterceptor.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageHttpMetricsInterceptor.java index 9c50c5d7448c..e838ed47edb8 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageHttpMetricsInterceptor.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageHttpMetricsInterceptor.java @@ -55,7 +55,8 @@ public void initialize(HttpRequest request) throws java.io.IOException { if (delegate != null) { delegate.initialize(request); } - if (metrics == null || metrics.getHttpClientRequestDuration() == null) { + if (metrics == null + || (metrics.getHttpClientRequestDuration() == null && metrics.getTtfb() == null)) { return; } @@ -80,37 +81,54 @@ public void initialize(HttpRequest request) throws java.io.IOException { if (existingResp != null) { existingResp.interceptResponse(response); } - if (recorded.compareAndSet(false, true) - && metrics.getHttpClientRequestDuration() != null) { + if (recorded.compareAndSet(false, true)) { double durationSeconds = Math.max(0.0, (clock.nanoTime() - startTimeNanos.get()) / 1_000_000_000.0); int statusCode = response.getStatusCode(); - String method = - request.getRequestMethod() != null ? request.getRequestMethod() : "UNKNOWN"; - String urlTemplate = extractUrlTemplate(request.getUrl()); String serverAddress = resolveServerAddress(request.getUrl(), defaultServerAddress); - AttributesBuilder attrBuilder = - metrics - .clientAttributesBuilder() - .put(StorageClientMetrics.KEY_HTTP_REQUEST_METHOD, method) - .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, (long) statusCode) - .put( - StorageClientMetrics.KEY_NETWORK_PATH, - StorageClientMetrics.NETWORK_PATH_CLOUDPATH) - .put(StorageClientMetrics.KEY_URL_TEMPLATE, urlTemplate) - .put( - StorageClientMetrics.KEY_SERVER_ADDRESS, - serverAddress != null ? serverAddress : "") - .put( - StorageClientMetrics.KEY_ERROR_TYPE, - statusCode >= 400 - ? StorageErrorTypeMapper.mapHttpResponseCode(statusCode) - : StorageErrorTypeMapper.STATUS_OK); + if (metrics.getHttpClientRequestDuration() != null) { + String method = + request.getRequestMethod() != null ? request.getRequestMethod() : "UNKNOWN"; + String urlTemplate = extractUrlTemplate(request.getUrl()); - metrics - .getHttpClientRequestDuration() - .record(durationSeconds, attrBuilder.build(), Context.current()); + AttributesBuilder attrBuilder = + metrics + .clientAttributesBuilder() + .put(StorageClientMetrics.KEY_HTTP_REQUEST_METHOD, method) + .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, (long) statusCode) + .put( + StorageClientMetrics.KEY_NETWORK_PATH, + StorageClientMetrics.NETWORK_PATH_CLOUDPATH) + .put(StorageClientMetrics.KEY_URL_TEMPLATE, urlTemplate) + .put( + StorageClientMetrics.KEY_SERVER_ADDRESS, + serverAddress != null ? serverAddress : "") + .put( + StorageClientMetrics.KEY_ERROR_TYPE, + statusCode >= 400 + ? StorageErrorTypeMapper.mapHttpResponseCode(statusCode) + : StorageErrorTypeMapper.STATUS_OK); + + metrics + .getHttpClientRequestDuration() + .record(durationSeconds, attrBuilder.build(), Context.current()); + } + + if (metrics.getTtfb() != null && statusCode < 400 && isMediaDownloadRequest(request)) { + AttributesBuilder ttfbAttrs = + metrics + .clientAttributesBuilder() + .put(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http") + .put(StorageClientMetrics.KEY_RPC_METHOD, "ReadObject") + .put( + StorageClientMetrics.KEY_NETWORK_PATH, + StorageClientMetrics.NETWORK_PATH_CLOUDPATH) + .put( + StorageClientMetrics.KEY_SERVER_ADDRESS, + serverAddress != null ? serverAddress : ""); + metrics.getTtfb().record(durationSeconds, ttfbAttrs.build(), Context.current()); + } } }); @@ -223,4 +241,17 @@ static String extractUrlTemplate(@Nullable GenericUrl url) { String[] parts = trimmed.split("/", 2); return parts.length == 1 ? "/{bucket}" : "/{bucket}/{object}"; } + + static boolean isMediaDownloadRequest(@Nullable HttpRequest request) { + if (request == null) { + return false; + } + if (!"GET".equalsIgnoreCase(request.getRequestMethod()) || request.getUrl() == null) { + return false; + } + GenericUrl url = request.getUrl(); + String path = url.getRawPath(); + return (path != null && path.startsWith("/download/storage/v1/")) + || "media".equals(url.getFirst("alt")); + } } diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageGrpcMetricsInterceptorTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageGrpcMetricsInterceptorTest.java index 6bc17c0df9da..e96ee13539d8 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageGrpcMetricsInterceptorTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageGrpcMetricsInterceptorTest.java @@ -50,6 +50,14 @@ public final class StorageGrpcMetricsInterceptorTest { .setResponseMarshaller(new NoopMarshaller()) .build(); + private static final MethodDescriptor WRITE_OBJECT_METHOD = + MethodDescriptor.newBuilder() + .setType(MethodDescriptor.MethodType.CLIENT_STREAMING) + .setFullMethodName("google.storage.v2.Storage/WriteObject") + .setRequestMarshaller(new NoopMarshaller()) + .setResponseMarshaller(new NoopMarshaller()) + .build(); + private InMemoryMetricReader reader; private SdkMeterProvider meterProvider; private FakeClock clock; @@ -215,6 +223,79 @@ public void interceptCall_callFails_recordsErrorType() throws Exception { .isEqualTo("storage.googleapis.com"); } + @Test + public void interceptCall_readObject_recordsTtfbOnFirstMessage() throws Exception { + StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, true, false); + StorageGrpcMetricsInterceptor interceptor = + new StorageGrpcMetricsInterceptor(metrics, clock, "storage.googleapis.com"); + + TestClientCall testCall = new TestClientCall<>(io.grpc.Attributes.EMPTY); + TestChannel testChannel = new TestChannel(testCall, "storage.googleapis.com"); + + ClientCall call = + interceptor.interceptCall(READ_OBJECT_METHOD, CallOptions.DEFAULT, testChannel); + call.start(new TestListener<>(), new Metadata()); + + clock.advanceNanos(200_000_000L); // 0.2s + testCall.deliverMessage(null); // First response message: triggers TTFB + + clock.advanceNanos(300_000_000L); // 0.3s more (total 0.5s) + testCall.deliverMessage(null); // Second message: should NOT record TTFB again + testCall.close(Status.OK, new Metadata()); + + Collection metricData = reader.collectAllMetrics(); + MetricData ttfbData = + metricData.stream() + .filter(m -> m.getName().equals(StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_TTFB)) + .findFirst() + .orElse(null); + + assertThat(ttfbData).isNotNull(); + assertThat(ttfbData.getHistogramData().getPoints()).hasSize(1); + HistogramPointData ttfbPoint = ttfbData.getHistogramData().getPoints().iterator().next(); + assertThat(ttfbPoint.getSum()).isWithin(1e-6).of(0.2); + assertThat(ttfbPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_SYSTEM_NAME)) + .isEqualTo("grpc"); + assertThat(ttfbPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_METHOD)) + .isEqualTo("ReadObject"); + + MetricData durationData = + metricData.stream() + .filter(m -> m.getName().equals(StorageClientMetrics.METRIC_RPC_CLIENT_CALL_DURATION)) + .findFirst() + .orElse(null); + assertThat(durationData).isNotNull(); + HistogramPointData durationPoint = + durationData.getHistogramData().getPoints().iterator().next(); + assertThat(durationPoint.getSum()).isWithin(1e-6).of(0.5); + } + + @Test + public void interceptCall_nonReadMethod_doesNotRecordTtfb() throws Exception { + StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, true, false); + StorageGrpcMetricsInterceptor interceptor = + new StorageGrpcMetricsInterceptor(metrics, clock, "storage.googleapis.com"); + + TestClientCall testCall = new TestClientCall<>(io.grpc.Attributes.EMPTY); + TestChannel testChannel = new TestChannel(testCall, "storage.googleapis.com"); + + ClientCall call = + interceptor.interceptCall(WRITE_OBJECT_METHOD, CallOptions.DEFAULT, testChannel); + call.start(new TestListener<>(), new Metadata()); + + clock.advanceNanos(200_000_000L); + testCall.deliverMessage(null); + testCall.close(Status.OK, new Metadata()); + + Collection metricData = reader.collectAllMetrics(); + MetricData ttfbData = + metricData.stream() + .filter(m -> m.getName().equals(StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_TTFB)) + .findFirst() + .orElse(null); + assertThat(ttfbData).isNull(); + } + @Test public void interceptCall_disabledMetrics_doesNotRecord() { StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, false, false); @@ -262,6 +343,12 @@ public io.grpc.Attributes getAttributes() { return attributes; } + void deliverMessage(RespT message) { + if (listener != null) { + listener.onMessage(message); + } + } + void close(Status status, Metadata trailers) { if (listener != null) { listener.onClose(status, trailers); diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageHttpMetricsInterceptorTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageHttpMetricsInterceptorTest.java index 0ce3d3376225..9adf49f1cba2 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageHttpMetricsInterceptorTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageHttpMetricsInterceptorTest.java @@ -334,6 +334,121 @@ public LowLevelHttpResponse execute() throws IOException { .isEqualTo("storage.googleapis.com"); } + @Test + public void isMediaDownloadRequest_cases() throws IOException { + HttpTransport transport = new MockHttpTransport(); + HttpRequest getDownload = + transport + .createRequestFactory() + .buildGetRequest( + new GenericUrl( + "https://storage.googleapis.com/download/storage/v1/b/my-b/o/my-obj")); + assertThat(StorageHttpMetricsInterceptor.isMediaDownloadRequest(getDownload)).isTrue(); + + HttpRequest getAltMedia = + transport + .createRequestFactory() + .buildGetRequest( + new GenericUrl( + "https://storage.googleapis.com/storage/v1/b/my-b/o/my-obj?alt=media")); + assertThat(StorageHttpMetricsInterceptor.isMediaDownloadRequest(getAltMedia)).isTrue(); + + HttpRequest getMetadata = + transport + .createRequestFactory() + .buildGetRequest( + new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b/o/my-obj")); + assertThat(StorageHttpMetricsInterceptor.isMediaDownloadRequest(getMetadata)).isFalse(); + + HttpRequest postReq = + transport + .createRequestFactory() + .buildPostRequest( + new GenericUrl( + "https://storage.googleapis.com/download/storage/v1/b/my-b/o/my-obj"), + null); + assertThat(StorageHttpMetricsInterceptor.isMediaDownloadRequest(postReq)).isFalse(); + + assertThat(StorageHttpMetricsInterceptor.isMediaDownloadRequest(null)).isFalse(); + } + + @Test + public void initialize_mediaDownload_recordsTtfb() throws IOException { + StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, true, false); + StorageHttpMetricsInterceptor interceptor = + new StorageHttpMetricsInterceptor(null, metrics, clock, "https://storage.googleapis.com"); + + MockLowLevelHttpResponse lowLevelResponse = new MockLowLevelHttpResponse(); + lowLevelResponse.setStatusCode(200); + HttpTransport transport = + new MockHttpTransport() { + @Override + public LowLevelHttpRequest buildRequest(String method, String url) { + return new MockLowLevelHttpRequest() { + @Override + public LowLevelHttpResponse execute() throws IOException { + clock.advanceNanos(250_000_000L); // 0.25s network duration + return lowLevelResponse; + } + }; + } + }; + + HttpRequest request = + transport + .createRequestFactory(interceptor) + .buildGetRequest( + new GenericUrl( + "https://storage.googleapis.com/download/storage/v1/b/my-b/o/my-obj")); + request.execute(); + + Collection metricData = reader.collectAllMetrics(); + MetricData ttfbData = + metricData.stream() + .filter(m -> m.getName().equals(StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_TTFB)) + .findFirst() + .orElse(null); + + assertThat(ttfbData).isNotNull(); + assertThat(ttfbData.getHistogramData().getPoints()).hasSize(1); + HistogramPointData ttfbPoint = ttfbData.getHistogramData().getPoints().iterator().next(); + assertThat(ttfbPoint.getSum()).isWithin(1e-6).of(0.25); + Attributes attrs = ttfbPoint.getAttributes(); + assertThat(attrs.get(StorageClientMetrics.KEY_RPC_SYSTEM_NAME)).isEqualTo("http"); + assertThat(attrs.get(StorageClientMetrics.KEY_RPC_METHOD)).isEqualTo("ReadObject"); + assertThat(attrs.get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_CLOUDPATH); + assertThat(attrs.get(StorageClientMetrics.KEY_SERVER_ADDRESS)) + .isEqualTo("storage.googleapis.com"); + } + + @Test + public void initialize_nonDownloadGet_doesNotRecordTtfb() throws IOException { + StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, true, false); + StorageHttpMetricsInterceptor interceptor = + new StorageHttpMetricsInterceptor(null, metrics, clock, "https://storage.googleapis.com"); + + MockLowLevelHttpResponse lowLevelResponse = new MockLowLevelHttpResponse(); + lowLevelResponse.setStatusCode(200); + HttpTransport transport = + new MockHttpTransport.Builder().setLowLevelHttpResponse(lowLevelResponse).build(); + + HttpRequest request = + transport + .createRequestFactory(interceptor) + .buildGetRequest( + new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b/o/my-obj")); + request.execute(); + + Collection metricData = reader.collectAllMetrics(); + MetricData ttfbData = + metricData.stream() + .filter(m -> m.getName().equals(StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_TTFB)) + .findFirst() + .orElse(null); + assertThat(ttfbData).isNull(); + } + @Test public void initialize_metricsDisabled_doesNotRecord() throws IOException { StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, false, false);