Include inherited fields in service stub options toString - #3070
Open
sdjcse wants to merge 2 commits into
Open
Conversation
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>
|
|
1 similar comment
|
|
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
2 participants
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