Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

fix: client_info default values #681

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Dec 5, 2022
Merged

fix: client_info default values #681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
50f07f5
client should attach veneer headers to all calls
daniel-sanche Dec 2, 2022
428ec68
test all services
daniel-sanche Dec 2, 2022
09628f3
added tests for gapic headers
daniel-sanche Dec 2, 2022
ef26b79
added comments, improved http test
daniel-sanche Dec 2, 2022
9394c1c
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Dec 2, 2022
d46c642
convert client info into gapic subclass
daniel-sanche Dec 2, 2022
0a33d56
fixed lint issues
daniel-sanche Dec 2, 2022
3cce324
Merge branch 'fix_headers' of github.com:googleapis/python-logging in…
daniel-sanche Dec 2, 2022
f52dd6c
improved type checking
daniel-sanche Dec 2, 2022
bfddc75
removed links
daniel-sanche Dec 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions 40 google/cloud/logging_v2/_gapic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
from google.cloud.logging_v2.sink import Sink
from google.cloud.logging_v2.metric import Metric

from google.api_core import client_info
from google.api_core import gapic_v1


class _LoggingAPI(object):
"""Helper mapping logging-related APIs."""
Expand Down Expand Up @@ -562,6 +565,22 @@ def _log_entry_mapping_to_pb(mapping):
return LogEntryPB(entry_pb)


def _client_info_to_gapic(input_info):
"""
Helper function to convert api_core.client_info to
api_core.gapic_v1.client_info subclass
"""
return gapic_v1.client_info.ClientInfo(
python_version=input_info.python_version,
grpc_version=input_info.grpc_version,
api_core_version=input_info.api_core_version,
gapic_version=input_info.gapic_version,
client_library_version=input_info.client_library_version,
user_agent=input_info.user_agent,
rest_version=input_info.rest_version,
)


def make_logging_api(client):
"""Create an instance of the Logging API adapter.

Expand All @@ -572,9 +591,14 @@ def make_logging_api(client):
Returns:
_LoggingAPI: A metrics API instance with the proper credentials.
"""
info = client._client_info
if type(info) == client_info.ClientInfo:
# convert into gapic-compatible subclass
info = _client_info_to_gapic(info)

generated = LoggingServiceV2Client(
credentials=client._credentials,
client_info=client._client_info,
client_info=info,
client_options=client._client_options,
)
return _LoggingAPI(generated, client)
Expand All @@ -590,9 +614,14 @@ def make_metrics_api(client):
Returns:
_MetricsAPI: A metrics API instance with the proper credentials.
"""
info = client._client_info
if type(info) == client_info.ClientInfo:
# convert into gapic-compatible subclass
info = _client_info_to_gapic(info)

generated = MetricsServiceV2Client(
credentials=client._credentials,
client_info=client._client_info,
client_info=info,
client_options=client._client_options,
)
return _MetricsAPI(generated, client)
Expand All @@ -608,9 +637,14 @@ def make_sinks_api(client):
Returns:
_SinksAPI: A metrics API instance with the proper credentials.
"""
info = client._client_info
if type(info) == client_info.ClientInfo:
# convert into gapic-compatible subclass
info = _client_info_to_gapic(info)

generated = ConfigServiceV2Client(
credentials=client._credentials,
client_info=client._client_info,
client_info=info,
client_options=client._client_options,
)
return _SinksAPI(generated, client)
4 changes: 4 additions & 0 deletions 4 google/cloud/logging_v2/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ def __init__(
kw_args["api_endpoint"] = api_endpoint

self._connection = Connection(self, **kw_args)
if client_info is None:
# if client info not passed in, use the discovered
# client info from _connection object
client_info = self._connection._client_info

self._client_info = client_info
losalex marked this conversation as resolved.
Show resolved Hide resolved
self._client_options = client_options
Expand Down
16 changes: 16 additions & 0 deletions 16 tests/unit/gapic/logging_v2/test_config_service_v2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,22 @@ def test__get_default_mtls_endpoint():
)


def test_config_default_client_info_headers():
import re
import pkg_resources

# test that DEFAULT_CLIENT_INFO contains the expected gapic headers
gapic_header_regex = re.compile(
r"gapic\/[0-9]+\.[\w.-]+ gax\/[0-9]+\.[\w.-]+ gl-python\/[0-9]+\.[\w.-]+ grpc\/[0-9]+\.[\w.-]+"
)
detected_info = (
google.cloud.logging_v2.services.config_service_v2.transports.base.DEFAULT_CLIENT_INFO
)
assert detected_info is not None
detected_agent = " ".join(sorted(detected_info.to_user_agent().split(" ")))
assert gapic_header_regex.match(detected_agent)


@pytest.mark.parametrize(
"client_class,transport_name",
[
Expand Down
16 changes: 16 additions & 0 deletions 16 tests/unit/gapic/logging_v2/test_logging_service_v2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,22 @@ def modify_default_endpoint(client):
)


def test_logging_default_client_info_headers():
import re
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like repeated code - can you create one function controlled/operated by parameters?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah unfortunately a lot of these test files contain mostly repeated test code pointed at different services. I considered making a different file for just these tests that could share more logic, but then decided to follow the existing conventions instead. Let me know if you think that makes sense

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you not plan to fix it in tests now, I guess lets open a bug and solve it later - lets keep a good standard to avoid code duplication

import pkg_resources

# test that DEFAULT_CLIENT_INFO contains the expected gapic headers
gapic_header_regex = re.compile(
r"gapic\/[0-9]+\.[\w.-]+ gax\/[0-9]+\.[\w.-]+ gl-python\/[0-9]+\.[\w.-]+ grpc\/[0-9]+\.[\w.-]+"
)
detected_info = (
google.cloud.logging_v2.services.logging_service_v2.transports.base.DEFAULT_CLIENT_INFO
)
assert detected_info is not None
detected_agent = " ".join(sorted(detected_info.to_user_agent().split(" ")))
assert gapic_header_regex.match(detected_agent)


def test__get_default_mtls_endpoint():
api_endpoint = "example.googleapis.com"
api_mtls_endpoint = "example.mtls.googleapis.com"
Expand Down
16 changes: 16 additions & 0 deletions 16 tests/unit/gapic/logging_v2/test_metrics_service_v2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,22 @@ def test__get_default_mtls_endpoint():
)


def test_metrics_default_client_info_headers():
import re
import pkg_resources

# test that DEFAULT_CLIENT_INFO contains the expected gapic headers
gapic_header_regex = re.compile(
r"gapic\/[0-9]+\.[\w.-]+ gax\/[0-9]+\.[\w.-]+ gl-python\/[0-9]+\.[\w.-]+ grpc\/[0-9]+\.[\w.-]+"
)
detected_info = (
google.cloud.logging_v2.services.metrics_service_v2.transports.base.DEFAULT_CLIENT_INFO
)
assert detected_info is not None
detected_agent = " ".join(sorted(detected_info.to_user_agent().split(" ")))
assert gapic_header_regex.match(detected_agent)


@pytest.mark.parametrize(
"client_class,transport_name",
[
Expand Down
58 changes: 58 additions & 0 deletions 58 tests/unit/test_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,16 @@
from datetime import datetime
from datetime import timedelta
from datetime import timezone
import re

import unittest

import mock

VENEER_HEADER_REGEX = re.compile(
r"gapic\/[0-9]+\.[\w.-]+ gax\/[0-9]+\.[\w.-]+ gccl\/[0-9]+\.[\w.-]+ gl-python\/[0-9]+\.[\w.-]+ grpc\/[0-9]+\.[\w.-]+"
)


def _make_credentials():
import google.auth.credentials
Expand Down Expand Up @@ -148,6 +153,59 @@ def make_api(client_obj):
again = client.logging_api
self.assertIs(again, api)

def test_veneer_grpc_headers(self):
# test that client APIs have client_info populated with the expected veneer headers
# required for proper instrumentation
creds = _make_credentials()
# ensure client info is set on client object
client = self._make_one(project=self.PROJECT, credentials=creds, _use_grpc=True)
self.assertIsNotNone(client._client_info)
user_agent_sorted = " ".join(
sorted(client._client_info.to_user_agent().split(" "))
)
self.assertTrue(VENEER_HEADER_REGEX.match(user_agent_sorted))
# ensure client info is propagated to gapic wrapped methods
patch = mock.patch("google.api_core.gapic_v1.method.wrap_method")
with patch as gapic_mock:
client.logging_api # initialize logging api
client.metrics_api # initialize metrics api
client.sinks_api # initialize sinks api
wrapped_call_list = gapic_mock.call_args_list
num_api_calls = 37 # expected number of distinct APIs in all gapic services (logging,metrics,sinks)
self.assertGreaterEqual(
len(wrapped_call_list),
num_api_calls,
"unexpected number of APIs wrapped",
)
for call in wrapped_call_list:
client_info = call.kwargs["client_info"]
self.assertIsNotNone(client_info)
wrapped_user_agent_sorted = " ".join(
sorted(client_info.to_user_agent().split(" "))
)
self.assertTrue(VENEER_HEADER_REGEX.match(wrapped_user_agent_sorted))

def test_veneer_http_headers(self):
# test that http APIs have client_info populated with the expected veneer headers
# required for proper instrumentation
creds = _make_credentials()
# ensure client info is set on client object
client = self._make_one(
project=self.PROJECT, credentials=creds, _use_grpc=False
)
self.assertIsNotNone(client._client_info)
user_agent_sorted = " ".join(
sorted(client._client_info.to_user_agent().split(" "))
)
self.assertTrue(VENEER_HEADER_REGEX.match(user_agent_sorted))
# ensure client info is propagated to _connection object
connection_user_agent = client._connection._client_info.to_user_agent()
self.assertIsNotNone(connection_user_agent)
connection_user_agent_sorted = " ".join(
sorted(connection_user_agent.split(" "))
)
self.assertTrue(VENEER_HEADER_REGEX.match(connection_user_agent_sorted))

def test_no_gapic_ctor(self):
from google.cloud.logging_v2._http import _LoggingAPI

Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.