From 0f4dad8f518c4ef3f54210da915353f68cc7e771 Mon Sep 17 00:00:00 2001 From: Dhananjayan Santhanakrishnan Date: Sat, 29 Aug 2026 08:01:19 -0700 Subject: [PATCH 1/2] Include inherited fields in service stub options toString WorkflowServiceStubsOptions.toString rendered only the five fields declared on the subclass, silently dropping every field inherited from ServiceStubsOptions. Target, TLS settings, timeouts, and headers were absent from the output, which is exactly the connection information wanted when diagnosing a client from logs. OperatorServiceStubsOptions and CloudServiceStubsOptions had no toString at all and fell back to Object.toString, logging as an unreadable identity hash. Extract the field rendering on ServiceStubsOptions into a package-private toStringFields helper and have each subclass inline it alongside its own fields. The helper is package-private because all four classes live in io.temporal.serviceclient, so this adds no public API surface. Also include apiKeyProvided, which already participates in equals and hashCode but was omitted from toString. The API key itself is held by a metadata provider and is still never rendered. Fixes #2148. Co-Authored-By: Claude Opus 5 --- .../CloudServiceStubsOptions.java | 5 ++ .../OperatorServiceStubsOptions.java | 5 ++ .../serviceclient/RpcRetryOptions.java | 2 +- .../serviceclient/ServiceStubsOptions.java | 17 +++-- .../WorkflowServiceStubsOptions.java | 3 +- .../ServiceStubsOptionsTest.java | 66 +++++++++++++++++++ 6 files changed, 92 insertions(+), 6 deletions(-) diff --git a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/CloudServiceStubsOptions.java b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/CloudServiceStubsOptions.java index 2377db05f1..31b85e72ed 100644 --- a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/CloudServiceStubsOptions.java +++ b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/CloudServiceStubsOptions.java @@ -58,6 +58,11 @@ public int hashCode() { return Objects.hash(super.hashCode(), version); } + @Override + public String toString() { + return "CloudServiceStubsOptions{" + toStringFields() + ", version='" + version + '\'' + '}'; + } + /** Builder is the builder for ClientOptions. */ public static class Builder extends ServiceStubsOptions.Builder { private String version; diff --git a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/OperatorServiceStubsOptions.java b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/OperatorServiceStubsOptions.java index 92cc42a751..5551aab0f5 100644 --- a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/OperatorServiceStubsOptions.java +++ b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/OperatorServiceStubsOptions.java @@ -20,6 +20,11 @@ private OperatorServiceStubsOptions(ServiceStubsOptions serviceStubsOptions) { super(serviceStubsOptions); } + @Override + public String toString() { + return "OperatorServiceStubsOptions{" + toStringFields() + '}'; + } + /** Builder is the builder for ClientOptions. */ public static class Builder extends ServiceStubsOptions.Builder { diff --git a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/RpcRetryOptions.java b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/RpcRetryOptions.java index bddcca5a14..801bb2b166 100644 --- a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/RpcRetryOptions.java +++ b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/RpcRetryOptions.java @@ -489,7 +489,7 @@ public String toString() { return "RetryOptions{" + "initialInterval=" + initialInterval - + "congestionInitialInterval=" + + ", congestionInitialInterval=" + congestionInitialInterval + ", backoffCoefficient=" + backoffCoefficient diff --git a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java index 4161dca7f3..a1e30a9666 100644 --- a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java +++ b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java @@ -413,8 +413,16 @@ public int hashCode() { @Override public String toString() { - return "ServiceStubsOptions{" - + "channel=" + return "ServiceStubsOptions{" + toStringFields() + '}'; + } + + /** + * Renders the fields declared on this class as a comma separated list, without the enclosing type + * name or braces. Subclasses use this to include inherited fields in their own {@link + * #toString()} instead of dropping them. + */ + String toStringFields() { + return "channel=" + channel + ", target='" + target @@ -423,6 +431,8 @@ public String toString() { + channelInitializer + ", enableHttps=" + enableHttps + + ", apiKeyProvided=" + + apiKeyProvided + ", sslContext=" + sslContext + ", healthCheckAttemptTimeout=" @@ -454,8 +464,7 @@ public String toString() { + ", metricsScope=" + metricsScope + ", grpcCompression=" - + grpcCompression - + '}'; + + grpcCompression; } public static class Builder> { diff --git a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/WorkflowServiceStubsOptions.java b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/WorkflowServiceStubsOptions.java index f22a2ac67e..2f89349dfc 100644 --- a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/WorkflowServiceStubsOptions.java +++ b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/WorkflowServiceStubsOptions.java @@ -137,7 +137,8 @@ public int hashCode() { @Override public String toString() { return "WorkflowServiceStubsOptions{" - + "disableHealthCheck=" + + toStringFields() + + ", disableHealthCheck=" + disableHealthCheck + ", rpcLongPollTimeout=" + rpcLongPollTimeout diff --git a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java index 0c0e76d745..193b0183c8 100644 --- a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java +++ b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java @@ -177,4 +177,70 @@ public void testGrpcCompressionNonePassesThroughBuilderCopy() { assertEquals(GrpcCompression.NONE, copied.getGrpcCompression()); } + + @Test + public void testWorkflowServiceStubsOptionsToStringIncludesInheritedFields() { + WorkflowServiceStubsOptions options = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .validateAndBuildWithDefaults(); + + String rendered = options.toString(); + + assertTrue(rendered.startsWith("WorkflowServiceStubsOptions{")); + // Inherited fields used to be dropped entirely. + assertTrue(rendered, rendered.contains("target='localhost:7233'")); + assertTrue(rendered, rendered.contains("enableHttps=")); + assertTrue(rendered, rendered.contains("rpcTimeout=")); + assertTrue(rendered, rendered.contains("grpcCompression=")); + // Fields declared on the subclass are still present. + assertTrue(rendered, rendered.contains("disableHealthCheck=")); + assertTrue(rendered, rendered.contains("rpcLongPollTimeout=")); + // Inherited fields are inlined, not nested inside a second wrapper. + assertFalse(rendered, rendered.contains("{ServiceStubsOptions{")); + } + + @Test + public void testOperatorServiceStubsOptionsToString() { + OperatorServiceStubsOptions options = + OperatorServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .validateAndBuildWithDefaults(); + + String rendered = options.toString(); + + assertTrue(rendered.startsWith("OperatorServiceStubsOptions{")); + assertTrue(rendered, rendered.contains("target='localhost:7233'")); + assertTrue(rendered, rendered.contains("rpcTimeout=")); + } + + @Test + public void testCloudServiceStubsOptionsToStringIncludesVersion() { + CloudServiceStubsOptions options = + CloudServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .setVersion("v1") + .validateAndBuildWithDefaults(); + + String rendered = options.toString(); + + assertTrue(rendered.startsWith("CloudServiceStubsOptions{")); + assertTrue(rendered, rendered.contains("target='localhost:7233'")); + assertTrue(rendered, rendered.contains("version='v1'")); + } + + @Test + public void testToStringDoesNotLeakApiKey() { + WorkflowServiceStubsOptions options = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .addApiKey(() -> "super-secret-api-key") + .validateAndBuildWithDefaults(); + + String rendered = options.toString(); + + assertFalse(rendered, rendered.contains("super-secret-api-key")); + // The fact that an API key was configured is still useful when debugging. + assertTrue(rendered, rendered.contains("apiKeyProvided=true")); + } } From 85140e359e0cf6ce6bc09e314875beff2463204d Mon Sep 17 00:00:00 2001 From: Dhananjayan Santhanakrishnan Date: Sat, 29 Aug 2026 15:13:44 -0700 Subject: [PATCH 2/2] Render header names instead of header values in toString Metadata.toString renders header values in the clear. The previous commit propagated the inherited headers field into WorkflowServiceStubsOptions, OperatorServiceStubsOptions, and CloudServiceStubsOptions, which would have newly exposed any credential supplied through setHeaders, such as a static Authorization header. Render the header names only. This keeps the diagnostic signal that matters when reading the output, whether a header is configured and which ones, without printing what they contain. Also add a regression test for the missing separator before congestionInitialInterval in RpcRetryOptions.toString. Co-Authored-By: Claude Opus 5 --- .../serviceclient/ServiceStubsOptions.java | 6 +++-- .../serviceclient/RpcRetryOptionsTest.java | 17 +++++++++++++ .../ServiceStubsOptionsTest.java | 25 +++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java index a1e30a9666..ebc1d58474 100644 --- a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java +++ b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java @@ -455,8 +455,10 @@ String toStringFields() { + connectionBackoffResetFrequency + ", grpcReconnectFrequency=" + grpcReconnectFrequency - + ", headers=" - + headers + // Only the header names are rendered. Values are omitted because they routinely carry + // credentials, for example an Authorization header set through setHeaders. + + ", headerNames=" + + (headers == null ? null : headers.keys()) + ", grpcMetadataProviders=" + grpcMetadataProviders + ", grpcClientInterceptors=" diff --git a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/RpcRetryOptionsTest.java b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/RpcRetryOptionsTest.java index 8b66a2ded3..0887c00a29 100644 --- a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/RpcRetryOptionsTest.java +++ b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/RpcRetryOptionsTest.java @@ -1,6 +1,7 @@ package io.temporal.serviceclient; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import java.time.Duration; import org.junit.Test; @@ -27,4 +28,20 @@ public void setRetryOptionsMergesCongestionInitialInterval() { assertEquals(Duration.ofMillis(200), merged.getInitialInterval()); assertEquals(Duration.ofSeconds(7), merged.getCongestionInitialInterval()); } + + /** + * toString omitted the separator before congestionInitialInterval, so the two durations ran + * together as a single unreadable token. + */ + @Test + public void toStringSeparatesInitialAndCongestionInterval() { + String rendered = + RpcRetryOptions.newBuilder() + .setInitialInterval(Duration.ofMillis(100)) + .setCongestionInitialInterval(Duration.ofSeconds(1)) + .validateBuildWithDefaults() + .toString(); + + assertTrue(rendered, rendered.contains(", congestionInitialInterval=")); + } } diff --git a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java index 193b0183c8..7a42cf0a28 100644 --- a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java +++ b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.*; +import io.grpc.Metadata; import org.junit.Test; public class ServiceStubsOptionsTest { @@ -243,4 +244,28 @@ public void testToStringDoesNotLeakApiKey() { // The fact that an API key was configured is still useful when debugging. assertTrue(rendered, rendered.contains("apiKeyProvided=true")); } + + @Test + public void testToStringRendersHeaderNamesWithoutValues() { + Metadata headers = new Metadata(); + headers.put( + Metadata.Key.of("authorization", Metadata.ASCII_STRING_MARSHALLER), + "Bearer super-secret-token"); + headers.put(Metadata.Key.of("x-custom", Metadata.ASCII_STRING_MARSHALLER), "plain-value"); + + WorkflowServiceStubsOptions options = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .setHeaders(headers) + .validateAndBuildWithDefaults(); + + String rendered = options.toString(); + + // Metadata.toString renders values in the clear, so it must not be embedded directly. + assertFalse(rendered, rendered.contains("super-secret-token")); + assertFalse(rendered, rendered.contains("plain-value")); + // Header names are still reported, which is what makes the output useful for debugging. + assertTrue(rendered, rendered.contains("authorization")); + assertTrue(rendered, rendered.contains("x-custom")); + } }