-
Notifications
You must be signed in to change notification settings - Fork 333
feat: Add support to disable telemetry instrumentation #611
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,16 @@ | |
| - Automatic recording of exceptions and setting of span status. | ||
| - Selective method tracing in classes using include/exclude lists. | ||
|
|
||
| Configuration: | ||
| - Environment Variable Control: OpenTelemetry instrumentation can be | ||
| disabled using the `OTEL_INSTRUMENTATION_A2A_SDK_ENABLED` environment | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer that the default value for the flag is 0/false which are usually the defaults for unset values. How about renaming it to
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't feel too strongly on the name though but it is enabled in code by default. it could be |
||
| variable. | ||
|
|
||
| - Default: `true` (tracing enabled when OpenTelemetry is installed) | ||
| - To disable: Set `OTEL_INSTRUMENTATION_A2A_SDK_ENABLED=false` | ||
| - Case insensitive: 'true', 'True', 'TRUE' all enable tracing | ||
| - Any other value disables tracing and logs a debug message | ||
|
|
||
| Usage: | ||
| For a single function: | ||
| ```python | ||
|
|
@@ -57,6 +67,7 @@ def internal_method(self): | |
| import functools | ||
| import inspect | ||
| import logging | ||
| import os | ||
|
|
||
| from collections.abc import Callable | ||
| from typing import TYPE_CHECKING, Any | ||
|
|
@@ -74,11 +85,33 @@ def internal_method(self): | |
| from opentelemetry.trace import SpanKind as _SpanKind | ||
| from opentelemetry.trace import StatusCode | ||
|
|
||
| otel_installed = True | ||
|
|
||
| except ImportError: | ||
| logger.debug( | ||
| 'OpenTelemetry not found. Tracing will be disabled. ' | ||
| 'Install with: \'pip install "a2a-sdk[telemetry]"\'' | ||
| ) | ||
| otel_installed = False | ||
|
|
||
| ENABLED_ENV_VAR = 'OTEL_INSTRUMENTATION_A2A_SDK_ENABLED' | ||
| INSTRUMENTING_MODULE_NAME = 'a2a-python-sdk' | ||
| INSTRUMENTING_MODULE_VERSION = '1.0.0' | ||
|
|
||
| # Check if tracing is enabled via environment variable | ||
| env_value = os.getenv(ENABLED_ENV_VAR, 'true') | ||
| otel_enabled = env_value.lower() == 'true' | ||
vinoo999 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| # Log when tracing is explicitly disabled via environment variable | ||
| if otel_installed and not otel_enabled: | ||
| logger.debug( | ||
| 'A2A OTEL instrumentation disabled via environment variable ' | ||
| '%s=%r. Tracing will be disabled.', | ||
| ENABLED_ENV_VAR, | ||
| env_value, | ||
| ) | ||
|
|
||
| if not otel_installed or not otel_enabled: | ||
|
|
||
| class _NoOp: | ||
| """A no-op object that absorbs all tracing calls when OpenTelemetry is not installed.""" | ||
|
|
@@ -99,12 +132,9 @@ def __getattr__(self, name: str) -> Any: | |
| _SpanKind = _NoOp() # type: ignore | ||
| StatusCode = _NoOp() # type: ignore | ||
|
|
||
| SpanKind = _SpanKind | ||
| SpanKind = _SpanKind # type: ignore | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. was receiving unbound linting errors since i moved the assignment to line 132 rather than exception block. |
||
| __all__ = ['SpanKind'] | ||
|
|
||
| INSTRUMENTING_MODULE_NAME = 'a2a-python-sdk' | ||
| INSTRUMENTING_MODULE_VERSION = '1.0.0' | ||
|
|
||
|
|
||
| def trace_function( # noqa: PLR0915 | ||
| func: Callable | None = None, | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please explain why you need this extra dependency. Can we do without it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new tests need to reload the telemetry module with different
OTEL_INSTRUMENTATION_A2A_SDK_ENABLEDenvironment variable values. The module-level code intelemetry.pychecks this env var once at import time, so changing it mid-test-run doesn't work unless you reload the module. xdist prevents race conditions when modifying the module level code vars so that these tests don't set/re-set the env var accordingly.This dependency is only added for
devis there an issue here?