Skip to content
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

fix: use debug logging level for OpenTelemetry message #442

Merged
merged 5 commits into from
Jan 11, 2021
Merged

fix: use debug logging level for OpenTelemetry message #442

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dc39d0f
fix: use debug logging level for OpenTelemetry message
tswast Dec 21, 2020
5cc4e72
only warn at span creation time
tswast Dec 21, 2020
43636f1
add unit test for skipping warning
tswast Dec 21, 2020
2551dec
Merge remote-tracking branch 'upstream/master' into issue441-telemetr…
tswast Jan 11, 2021
fe5b4e6
refactor: rename _warned_telemetry to indicate private and mutable
tswast Jan 11, 2021
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
Prev Previous commit
Next Next commit
only warn at span creation time
  • Loading branch information
@tswast
tswast committed Dec 21, 2020
commit 5cc4e721b97daea17f462f00fd285e3511748aba
19 changes: 12 additions & 7 deletions google/cloud/bigquery/opentelemetry_tracing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,11 @@
from opentelemetry.trace.status import Status

HAS_OPENTELEMETRY = True
WARNED_OPENTELEMETRY = True

except ImportError:
logger.debug(
"This service is instrumented using OpenTelemetry. "
"OpenTelemetry could not be imported; please "
"add opentelemetry-api and opentelemetry-instrumentation "
"packages in order to get BigQuery Tracing data."
)

HAS_OPENTELEMETRY = False
WARNED_OPENTELEMETRY = False

_default_attributes = {
"db.system": "BigQuery"
Expand Down Expand Up @@ -64,8 +59,18 @@ def create_span(name, attributes=None, client=None, job_ref=None):
Raised if a span could not be yielded or issue with call to
OpenTelemetry.
"""
global WARNED_OPENTELEMETRY
final_attributes = _get_final_span_attributes(attributes, client, job_ref)
if not HAS_OPENTELEMETRY:
if not WARNED_OPENTELEMETRY:
logger.debug(
"This service is instrumented using OpenTelemetry. "
"OpenTelemetry could not be imported; please "
"add opentelemetry-api and opentelemetry-instrumentation "
"packages in order to get BigQuery Tracing data."
)
WARNED_OPENTELEMETRY = True
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit) It's a bit unexpected to change a "constant", perhaps rename it? Also, since it's an internal state-keeping flag that users should neither know about nor modify it, we should probably mark it as such (e.g. _warned_telemetry)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. I've renamed in the latest commit.


yield None
return
tracer = trace.get_tracer(__name__)
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/test_opentelemetry_tracing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,10 @@ def setup():
def test_opentelemetry_not_installed(setup, monkeypatch):
monkeypatch.setitem(sys.modules, "opentelemetry", None)
reload_module(opentelemetry_tracing)
assert not opentelemetry_tracing.WARNED_OPENTELEMETRY
with opentelemetry_tracing.create_span("No-op for opentelemetry") as span:
assert span is None
assert opentelemetry_tracing.WARNED_OPENTELEMETRY


@pytest.mark.skipif(opentelemetry is None, reason="Require `opentelemetry`")
Expand Down