Repository navigation
Conversation
|
|
Author
|
@Quinn-With-Two-Ns Sorry I was not able to get back the previous change on time, I signed the CLA on this one. Can you take a look when you are free? |
sdjcse
force-pushed
the
issue-2148-stub-options-tostring
branch
from
October 19, 2026 18:17
85140e3 to
37eb947
Compare
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 temporalio#2148. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
sdjcse
force-pushed
the
issue-2148-stub-options-tostring
branch
from
October 28, 2026 10:17
0aa0c0f to
8fd1390
Compare
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are blocking approval of the service stub diagnostic changes.
No open issues 🎉
What changed in this PR
Improves service stub option diagnostics in temporal-serviceclient, addressing #2148 by including inherited configuration in subclass toString() output without exposing header values.
Changes:
- Shares inherited-field rendering across workflow, operator, and cloud options, including API-key configuration status.
- Renders header names instead of values and adds tests for subclass output and credential omission.
- Fixes a missing separator in retry-option output and adds a regression test.
| File | Description |
|---|---|
temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java |
Tests inherited fields, subclass output, and credential omission. |
temporal-serviceclient/src/test/java/io/temporal/serviceclient/RpcRetryOptionsTest.java |
Tests the retry-interval separator. |
temporal-serviceclient/src/main/java/io/temporal/serviceclient/WorkflowServiceStubsOptions.java |
Includes inherited fields alongside workflow-specific options. |
temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java |
Extracts shared rendering, adds API-key status, and omits header values. |
temporal-serviceclient/src/main/java/io/temporal/serviceclient/RpcRetryOptions.java |
Adds the missing separator before the congestion interval. |
temporal-serviceclient/src/main/java/io/temporal/serviceclient/OperatorServiceStubsOptions.java |
Adds operator-labeled output using shared fields. |
temporal-serviceclient/src/main/java/io/temporal/serviceclient/CloudServiceStubsOptions.java |
Adds cloud-labeled output with inherited fields and version. |
Review effort: Balanced
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was changed
ServiceStubsOptions.toStringnow renders its fields through a package-privatetoStringFields()that each subclass inlines alongside its own fields.
WorkflowServiceStubsOptionsrendered only the five fields declared on the subclass, droppingevery inherited one — target, TLS, timeouts, headers.
OperatorServiceStubsOptionsandCloudServiceStubsOptionshad notoStringand logged as anidentity hash.
apiKeyProvided, already inequals/hashCodebut missing fromtoString.io.grpc.Metadata#toStringprints values in the clear, soinheriting the field as-is would have exposed credentials set through
setHeaders.,beforecongestionInitialIntervalinRpcRetryOptions.toString.Why?
This output is what lands in logs when diagnosing a client. Two of the three option types rendered
nothing usable; the third omitted every inherited field.
Breaking changes?
None to public API.
ServiceStubsOptions.toStringnow emitsheaderNames=[...]where it used toemit
headers=with values — deliberate, so the fix does not leak credentials.Server PR
N/A.
Checklist
Closes Better toString representations on service stub options #2148
How was this tested:
Five new tests covering each subclass's
toString, asserting no API key or header value appearsin the output, plus a regression test for the separator.
Any docs updates needed?
No.
🤖 Generated with Claude Code