Repository navigation
fix(gapic-generator): default client_options.api_endpoint to transport host for OpenTelemetry spans - #18869
fix(gapic-generator): default client_options.api_endpoint to transport host for OpenTelemetry spans#18869chalmerlowe wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the base transport templates and generated golden files to automatically populate the api_endpoint in client_options with the transport's host if it is not already set, supporting both ClientOptions instances and dictionaries. It also updates the corresponding unit tests and telemetry requirements matrix. A review comment correctly points out that mutating client_options in-place can cause unexpected side effects if the caller reuses the options object, and suggests copying the options before modification.
| if isinstance(client_options, client_options_lib.ClientOptions) and not client_options.api_endpoint: | ||
| client_options.api_endpoint = self._host | ||
| elif isinstance(client_options, dict) and not client_options.get("api_endpoint"): | ||
| client_options["api_endpoint"] = self._host |
There was a problem hiding this comment.
Mutating the client_options argument in-place (whether it is a ClientOptions instance or a dict) can lead to unexpected side effects if the caller reuses the same options object across multiple client or transport instances. To prevent this, we should create a copy of the options before modifying them.
if isinstance(client_options, client_options_lib.ClientOptions):
if not client_options.api_endpoint:
import copy
client_options = copy.copy(client_options)
client_options.api_endpoint = self._host
elif isinstance(client_options, dict):
if not client_options.get("api_endpoint"):
client_options = dict(client_options)
client_options["api_endpoint"] = self._host
5591f09 to
3c63f33
Compare
f636b0e to
a8fbedc
Compare
3c63f33 to
63c5e32
Compare
a8fbedc to
fd9c5ba
Compare
63c5e32 to
aded2a1
Compare
6855304 to
64d36b7
Compare
7568958 to
781a900
Compare
64d36b7 to
3cac2f0
Compare
781a900 to
aeacfb7
Compare
3cac2f0 to
7ecc47d
Compare
aeacfb7 to
813aa30
Compare
7ecc47d to
e5c1686
Compare
813aa30 to
91cde6b
Compare
e5c1686 to
a0113ae
Compare
91cde6b to
d6f0e97
Compare
a0113ae to
7edc460
Compare
d6f0e97 to
9bca1d0
Compare
7edc460 to
dde7880
Compare
…t host for OpenTelemetry spans
9bca1d0 to
07e6fa4
Compare
Summary
Stacked on #18868.
Populates
client_options.api_endpointwith the transport's resolvedself._hostinTransport.__init__(base.py.j2) whenclient_optionsis provided without an explicitapi_endpointoverride.Why
_extract_endpoint_attributesingoogle-api-coreonly populatedserver.addressand non-defaultserver.porton Tier 3 (GAPIC client) and gRPC Tier 4 (wire attempt) spans when the caller explicitly passedClientOptions(api_endpoint=...).DEFAULT_HOST(or ahostpassed directly to the transport constructor) without settingclient_options.api_endpoint,server.addressandserver.portwere omitted from Tier 3 and gRPC Tier 4 spans.client_options.api_endpointtoself._hostin the base transport constructor ensures Tier 3 and gRPC Tier 4 spans consistently recordserver.addressand non-defaultserver.port.Files Modified
packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/base.py.j2packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2packages/gapic-generator/tests/system/telemetry_requirements_matrix.csvpackages/gapic-generator/tests/integration/goldens/**