Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions packages/google-api-core/google/api_core/client_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ class ClientOptions(object):
`googleapis.com`. If both `api_endpoint` and `universe_domain` are set,
then `api_endpoint` is used as the service endpoint. If `api_endpoint` is
not specified, the format will be `{service}.{universe_domain}`.
tracer_provider (Optional[object]): The OpenTelemetry TracerProvider to use
for tracing. If not set, the global tracer provider is used, if
available.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we be more specific about the types here? (You can use string annotations if you can't import the types yet)


Raises:
ValueError: If both ``client_cert_source`` and ``client_encrypted_cert_source``
Expand All @@ -117,6 +120,7 @@ def __init__(
api_key: Optional[str] = None,
api_audience: Optional[str] = None,
universe_domain: Optional[str] = None,
tracer_provider: Optional[object] = None,
):
if credentials_file is not None:
warnings.warn(general_helpers._CREDENTIALS_FILE_WARNING, DeprecationWarning)
Expand All @@ -136,6 +140,7 @@ def __init__(
self.api_key = api_key
self.api_audience = api_audience
self.universe_domain = universe_domain
self.tracer_provider = tracer_provider

def __repr__(self) -> str:
return "ClientOptions: " + repr(self.__dict__)
Expand Down
32 changes: 29 additions & 3 deletions packages/google-api-core/google/api_core/grpc_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@
import google.auth.transport.requests
import google.protobuf
import grpc

from google.api_core import exceptions, general_helpers
from google.api_core import _feature_gating_helpers, exceptions, general_helpers

# The list of gRPC Callable interfaces that return iterators.
_STREAM_WRAP_CLASSES = (grpc.UnaryStreamMultiCallable, grpc.StreamStreamMultiCallable)
Expand Down Expand Up @@ -384,10 +383,37 @@ def create_channel(
if attempt_direct_path:
target = _modify_target_for_direct_path(target)

return grpc.secure_channel(
configuration = kwargs.pop("configuration", None)

channel = grpc.secure_channel(
target, composite_credentials, compression=compression, **kwargs
)

is_tracing_enabled = _feature_gating_helpers.resolve_feature_flags(
env_var="GOOGLE_CLOUD_PYTHON_TRACING_ENABLED",
feature_key="tracer_provider",
configuration=configuration,
)

if is_tracing_enabled:
try:
import opentelemetry.instrumentation.grpc as otel_grpc # type: ignore[import-not-found]

tracer_provider = None
if configuration is not None:
if isinstance(configuration, dict):
tracer_provider = configuration.get("tracer_provider")
else:
tracer_provider = getattr(configuration, "tracer_provider", None)
Comment on lines +402 to +407

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can all of this and is_tracing_enabled be resolved in a function on ClientOptions?

It isn't clear how ClientOptions are connecting to the kwargs["configuration"] here, or why we wouldn't pass ClientOptions directly to this function.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@westarle

Thanks for the feedback. Let's look at the design decisions captured in the Feature Gating Low-Level Design (LLD) to clarify why we ended up here, as regards this question:

can all of this and is_tracing_enabled be resolved in a function on ClientOptions?

The core reason for this structure is Separation of Concerns and Generalization:

  1. The Resolver's Role: _feature_gating_helpers isn't just checking a boolean; it is resolving precedence (Environment Variable vs. Object Attribute vs. Dictionary Key). If we move is_tracing_enabled() to ClientOptions, then ClientOptions suddenly has to become aware of environment variables and resolution logic, which bloats its responsibilities.
  2. Generalization (beyond ClientOptions): As discussed during the design phase (Item 11 in the Alternatives section and as noted in the Out of Scope section), we generalized this so it can gate any feature (not just o11y features), whether the configuration comes from ClientOptions, a plain dict, or another *Options object (e.g., custom retry settings). Naming the parameter configuration instead of "provider_key" in create_channel was a nod to this duck-typing/flexibility.

If we want to pivot and tightly couple all feature gating exclusively to ClientOptions, we can certainly return to the design doc and re-evaluate the testing and architectural footprint, but the current approach was chosen specifically to keep ClientOptions lean and the gating logic reusable across different types of configuration.

Regarding this question:

It isn't clear how ClientOptions are connecting to the kwargs["configuration"] here, or why we wouldn't pass ClientOptions directly to this function.

Why did we extract configuration from kwargs instead of making it an explicit argument?

We needed a way to get the tracer_provider or feature flags down into this helper.

Several factors played into our decision:

  • Avoiding Signature Churn: create_channel is a highly public, widely used function in google-api-core. Adding a brand new explicit argument changes the signature. While adding it at the very end (before **kwargs) is usually safe, popping it from kwargs feels even "safer" because it doesn't change the explicit parameter list at all.

  • Convenience during Prototyping: If we decide to revisit this at a later time (i.e. consider expanding the args list), that would be a doable thing but it did not seem worth it to go through all the effort to revise all the tests, the call signatures, the docstrings, type hints, etc unless we feel changing the create_channel signature is the right thing to do.

  • Pass-Through Concerns: configuration is not just another option to pass down ... create_channel was largely a thin wrapper around grpc.secure_channel. At this time, grpc.secure_channel does NOT accept a configuration argument hence the step to remove it from kwargs before passing kwargs on.

If your question were more general: can't we encapsulate some of this bloat... we could do that (and i now believe we should)

Keep create_channel as lean as possible

def create_channel(
    target,
    credentials=None,
    scopes=None,
    ssl_credentials=None,
    credentials_file=None,
    quota_project_id=None,
    default_scopes=None,
    default_host=None,
    compression=None,
    attempt_direct_path: Optional[bool] = False,
    **kwargs,
):
    # ... setup credentials and target ...
    
    configuration = kwargs.pop("configuration", None)

    channel = grpc.secure_channel(
        target, composite_credentials, compression=compression, **kwargs
    )

    # All the OpenTelemetry mechanics are encapsulated in this helper
    channel = _intercept_channel_if_tracing_enabled(channel, configuration)

    return channel

Create an extracted helper

def _intercept_channel_if_tracing_enabled(channel, configuration):
    """If enabled, wraps the channel with OpenTelemetry tracing."""
    is_tracing_enabled = _feature_gating_helpers.resolve_feature_flags(
        env_var="GOOGLE_CLOUD_PYTHON_TRACING_ENABLED",
        feature_key="tracer_provider",
        configuration=configuration,
    )
    
    if not is_tracing_enabled:
        return channel

    try:
        import opentelemetry.instrumentation.grpc as otel_grpc  # type: ignore[import-not-found]

        tracer_provider = None
        if configuration is not None:
            if isinstance(configuration, dict):
                tracer_provider = configuration.get("tracer_provider")
            else:
                tracer_provider = getattr(configuration, "tracer_provider", None)

        interceptor = otel_grpc.client_interceptor(tracer_provider=tracer_provider)
        return otel_grpc.intercept_channel(channel, interceptor)
    except ImportError:
        # Fail open if instrumentation is missing
        return channel

@daniel-sanche daniel-sanche Aug 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should keep api_core.grpc_helpers.create_channel as a light-weight wrapper over grpc channel creation, and leave this logic out of it. Attaching an interceptor like this feels like a decision we'd want to make at in the client layer, not baked into the api_core layer. It breaks some of the abstractions we have in place

We currently already set up a logging interceptor within the gapic transport class, and that feels like a more natural place for this kind of thing.

Although I'm not really a fan of how the LoggingInterceptor is set up currently, so don't follow that pattern exactly. LoggingInterceptor is hard-coded into the grpc transport class, so any veneers/users that use custom transport implementations lose the interceptor. Now that we're starting to scale up the number of interceptors in place, I think it would be better if we could either:

  • A. pass in a list of interceptors to the transport at init time, like the rest client supports.
  • B. add an attach_interceptor() method to the transport, and then call that to set-up our interceptors inside Client.__init__

TL;DR: I think the interceptor should live on the Transport, and then we can read the ClientOptions and optionally attach the transport when setting up a client. Would that work?


interceptor = otel_grpc.client_interceptor(tracer_provider=tracer_provider)
channel = otel_grpc.intercept_channel(channel, interceptor)
except ImportError:
# If OpenTelemetry gRPC instrumentation is missing, this should simply NOOP and fail open rather than failing import.
pass

return channel


def _modify_target_for_direct_path(target: str) -> str:
"""
Expand Down
12 changes: 10 additions & 2 deletions packages/google-api-core/google/api_core/grpc_helpers_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@
from typing import AsyncGenerator, Generic, Iterator, Optional, TypeVar

import grpc
from grpc import aio

from google.api_core import exceptions, general_helpers, grpc_helpers
from grpc import aio

# denotes the proto response type for grpc calls
P = TypeVar("P")
Expand Down Expand Up @@ -303,6 +302,15 @@ def create_channel(
if attempt_direct_path:
target = grpc_helpers._modify_target_for_direct_path(target)

# NOTE: 'configuration' is popped to prevent a TypeError.
# Generated async transports (like those in google-cloud-* libs) pass 'configuration'
# down to this helper via **kwargs to support tracing in sync transports.
# However, 'aio.secure_channel' does not recognize this parameter yet and will
# crash if it is passed through.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So this means we'd be forced to bump up the minimum api_core version across all libraries going forward, right?

If at all possible, we should aim to fail gracefully, even if observability features are locked behind a certain api_core version

# Async gRPC tracing is deferred to a future phase/PR, so we simply discard
# this parameter for now to ensure generated async code doesn't fail at runtime.
kwargs.pop("configuration", None)

return aio.secure_channel(
target, composite_credentials, compression=compression, **kwargs
)
Expand Down
7 changes: 7 additions & 0 deletions packages/google-api-core/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ dependencies = [
"proto-plus >= 1.26.1, < 2.0.0",
"google-auth >= 2.14.1, < 3.0.0",
"requests >= 2.33.0, < 3.0.0",
"opentelemetry-api >= 1.27.0, < 2.0.0",
]
dynamic = ["version"]

Expand All @@ -64,6 +65,10 @@ grpc = [
"grpcio-status >= 1.59.0, < 2.0.0",
"grpcio-status >= 1.75.1, < 2.0.0; python_version >= '3.14'",
]
tracing = [
"opentelemetry-instrumentation-grpc >= 0.46b0, < 1.0.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this tracing extra required for customers to use tracing? Could we move the otel-api dependency into this extra?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@westarle

I reviewed the decisions we made in the draft HLD, the HLD, and the LLD where we decided to make opentelemetry-api required (since it's NOOP) while keeping opentelemetry-instrumentation-grpc optional.

If we move otel-api to the extra too, it seems we might introduce friction for customers who only want our Logical spans (T3) but have custom transport instrumentation (so they don't want otel-inst-grpc). If they are both in the same extra, they can't choose.

Also, it means we'll need to inject more try...except ImportError fallback logic into all GAPICs for T3 spans, which we were hoping to avoid.

Thoughts?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The solution should work by default for most customers who don't know about gRPC and aren't installing instrumentation. So I'd like to enable comprehensive spans T3/T4 with a single opt-in without needing to support and document the tracing extra.

If there's a strong technical reason to put otel-inst-grpc in a tracing extra (like diamond dependency issues or if it's unstable) then I'd prefer to make the tracing extra serve as opt-in for all tracing instrumentation.

We should test the interaction of our implementation in case customers have otel-inst-grpc monkey-patched in so we can provide guidance (either configure the monkeypatching to avoid our instances or avoid it).

Generally we would recommend customers with their own custom transport tracing not enable our tracing at all, in the future we might recommend some attributes and stuff if this is common.

@chalmerlowe chalmerlowe Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@westarle

We can create a [tracing] extra that carries both:

  • opentelemetry-api
  • opentelemetry-instrumentation-grpc

As context:
I am doing my best to keep this PR small and tight. I am happy to add more sophisticated tests and such to some follow-on PRs, but I would very much prefer that we do what we can to incrementally add bits and bobs and multiple testing approaches.

I have a PR in the works already that will add a more integration level of E2E testing using a fake endpoint as you mention in one of your other comments.

I would feel more comfortable following that with addition test PRs to focus on more complicated issues related to interactions we might have if global monkey-patching is turned on by default in the customer ecosystem.

There is a Testing Section in the LLD where I am capturing this feedback and defining what we expect more complex testing to look like.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's put everything in the tracing extra for now and consider using this as an enablement signal. Maybe call it tracing-experimental for now to prevent dependencies in case we want to refactor it.

No need to add test automation for the "what happens if the customer has already enabled grpc instrumentation" -- just try it out and account for it in the design (i.e. with docs for the customer to exclude our instrumentation).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My understanding was that we were planning to use the otel conventions, by making the api package required by default, and leaving the heavier sdk/instrumentation implementations as optional. I'm open to adding new required dependencies for Observability, if you think it's necessary for the intended experience @westarle. But seeing the beta label does give me pause

Let's put everything in the tracing extra for now and consider using this as an enablement signal. Maybe call it tracing-experimental for now to prevent dependencies in case we want to refactor it.

I think we should avoid adding temporary experimental extras like this though. The only purpose of advertising extra dependencies is as a contract to our users. If we want to keep this kind of thing internal while we work out the details, we can just make a private requirements.txt configuration we can include in our tests for now

]



[tool.setuptools.dynamic]
Expand Down Expand Up @@ -93,4 +98,6 @@ filterwarnings = [
"ignore:.*custom tp_new.*in Python 3.14:DeprecationWarning",
# Remove once https://github.com/grpc/grpc/issues/35086 is fixed (and version newer than 1.60.0 is published)
"ignore:There is no current event loop:DeprecationWarning",
# Ignore external OpenTelemetry/importlib.metadata SelectableGroups warning
"ignore:.*SelectableGroups dict interface is deprecated:DeprecationWarning",
]
1 change: 1 addition & 0 deletions packages/google-api-core/testing/constraints-3.10.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ requests==2.33.0
grpcio==1.59.0
grpcio-status==1.59.0
proto-plus==1.26.1
opentelemetry-api==1.27.0
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ grpcio==1.59.0
grpcio-status==1.59.0
proto-plus==1.26.1
aiohttp==3.13.4
opentelemetry-api==1.27.0
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@


import google.auth.credentials

from google.api_core import exceptions, grpc_helpers_async


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
# limitations under the License.

from re import match
from unittest import mock

import pytest

from google.api_core import client_options

from ..helpers import warn_deprecated_credentials_file
Expand All @@ -42,6 +42,7 @@ def test_constructor():
],
api_audience="foo2.googleapis.com",
universe_domain="googleapis.com",
tracer_provider=mock.Mock(),
)

assert options.api_endpoint == "foo.googleapis.com"
Expand All @@ -54,6 +55,7 @@ def test_constructor():
]
assert options.api_audience == "foo2.googleapis.com"
assert options.universe_domain == "googleapis.com"
assert options.tracer_provider is not None


def test_constructor_with_encrypted_cert_source():
Expand Down Expand Up @@ -123,6 +125,7 @@ def test_from_dict():
"https://www.googleapis.com/auth/cloud-platform.read-only",
],
"api_audience": "foo2.googleapis.com",
"tracer_provider": mock.Mock(),
}
)

Expand All @@ -136,6 +139,7 @@ def test_from_dict():
"https://www.googleapis.com/auth/cloud-platform.read-only",
]
assert options.api_key is None
assert options.tracer_provider is not None
assert options.api_audience == "foo2.googleapis.com"


Expand All @@ -162,6 +166,7 @@ def test_repr():
"scopes",
"api_key",
"api_audience",
"tracer_provider",
]
)
options = client_options.ClientOptions(api_endpoint="foo.googleapis.com")
Expand Down
3 changes: 1 addition & 2 deletions packages/google-api-core/tests/unit/test_grpc_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@
pytest.skip("No GRPC", allow_module_level=True)

import google.auth.credentials
from google.longrunning import operations_pb2

from google.api_core import exceptions, grpc_helpers
from google.longrunning import operations_pb2


def test__patch_callable_name():
Expand Down
132 changes: 132 additions & 0 deletions packages/google-api-core/tests/unit/test_grpc_helpers_otel.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
# Copyright 2026 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Tests for OpenTelemetry gRPC interceptor integration in google-api-core."""

import sys
import types
from unittest import mock

import pytest

try:
from google.api_core import grpc_helpers

HAS_GRPC_HELPERS = True
except ImportError:
HAS_GRPC_HELPERS = False


@pytest.fixture
def mock_otel_grpc(monkeypatch):
"""Fixture to mock OpenTelemetry gRPC hierarchy."""
mock_otel = mock.Mock()
mock_otel_grpc = mock_otel.instrumentation.grpc
mock_interceptor = mock.Mock()
mock_otel_grpc.client_interceptor.return_value = mock_interceptor

modules = {
"opentelemetry": mock_otel,
"opentelemetry.instrumentation": mock_otel.instrumentation,
"opentelemetry.instrumentation.grpc": mock_otel_grpc,
}

for name, mod in modules.items():
monkeypatch.setitem(sys.modules, name, mod)

return mock_otel_grpc


@pytest.mark.parametrize(
"is_otel_installed, tracing_env_var_value, expect_otel_interceptor",
[
pytest.param(True, "true", True, id="installed_and_enabled"),
pytest.param(True, "false", False, id="installed_but_disabled"),
pytest.param(False, "true", False, id="not_installed_fails_open"),
],
)
@pytest.mark.skipif(not HAS_GRPC_HELPERS, reason="Requires google-api-core[grpc]")
def test_create_channel_otel_combos(
monkeypatch,
mock_otel_grpc,
is_otel_installed,
tracing_env_var_value,
expect_otel_interceptor,
):
"""Verify create_channel behavior with various OTel installation and enablement states."""

monkeypatch.setenv("GOOGLE_CLOUD_PYTHON_TRACING_ENABLED", tracing_env_var_value)

if not is_otel_installed:
monkeypatch.setitem(sys.modules, "opentelemetry.instrumentation.grpc", None)

mock_channel = "raw_channel"
mock_otel_grpc.intercept_channel.side_effect = lambda ch, inc: f"wrapped_{ch}"

with (
mock.patch(
"grpc.secure_channel", return_value=mock_channel
) as mock_secure_channel,
):
with mock.patch(
"google.api_core.grpc_helpers._create_composite_credentials",
return_value=mock.Mock(),
):
channel = grpc_helpers.create_channel("localhost:1234")

# Always expect raw channel creation
mock_secure_channel.assert_called_once()

if expect_otel_interceptor:
mock_otel_grpc.client_interceptor.assert_called_once()
mock_otel_grpc.intercept_channel.assert_called_once_with(
mock_channel, mock_otel_grpc.client_interceptor.return_value
)
assert channel == f"wrapped_{mock_channel}"
else:
# OTel should NOT have been called
mock_otel_grpc.intercept_channel.assert_not_called()
assert channel == mock_channel


@pytest.mark.parametrize(
"config_factory",
[
lambda tp: {"tracer_provider": tp},
lambda tp: types.SimpleNamespace(tracer_provider=tp),
],
ids=["dict", "object"],
)
@pytest.mark.skipif(not HAS_GRPC_HELPERS, reason="Requires google-api-core[grpc]")
def test_create_channel_with_custom_tracer_provider(
monkeypatch, mock_otel_grpc, config_factory
):
"""Verify that create_channel passes custom tracer_provider to OTel interceptor."""

mock_tracer_provider = mock.Mock()
config = config_factory(mock_tracer_provider)

mock_channel = "raw_channel"
with (
mock.patch("grpc.secure_channel", return_value=mock_channel),
):
with mock.patch(
"google.api_core.grpc_helpers._create_composite_credentials",
return_value=mock.Mock(),
):
grpc_helpers.create_channel("localhost:1234", configuration=config)

mock_otel_grpc.client_interceptor.assert_called_once_with(
tracer_provider=mock_tracer_provider
)
Loading