FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: use debug logging level for OpenTelemetry message by tswast · Pull Request #442 · googleapis/python-bigquery · GitHub

This repository was archived by the owner on Mar 6, 2026. It is now read-only.
/ python-bigquery Public archive

fix: use debug logging level for OpenTelemetry message - #442

Merged
tswast merged 5 commits into
googleapis:masterfrom
tswast:issue441-telemetry-warning
Jan 11, 2021
ConversationCommits (5)ChecksFiles changed
Merged

fix: use debug logging level for OpenTelemetry message#442
tswast merged 5 commits into
googleapis:masterfrom
tswast:issue441-telemetry-warning

Conversation

tswast commented Dec 21, 2020

Copy link
Copy Markdown
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #441 🦕

tswast requested review from a team and steffnay and removed request for a team December 21, 2020 15:26
google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Dec 21, 2020
product-auto-label Bot added the api: bigquery Issues related to the googleapis/python-bigquery API. label Dec 21, 2020
tswast mentioned this pull request Dec 21, 2020
Closed

tswast commented Dec 21, 2020

Copy link
Copy Markdown
Contributor Author

Updated based on apache/airflow#13131 to not warn at import time.

tswast requested a review from shollyman January 6, 2021 23:05
plamut reviewed Jan 7, 2021
View reviewed changes

plamut left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I agree that the warning should not be issued at import time, but instead when trying to actually use BigQuery (discussion).

DEBUG level is probably indeed better, as opentelemetry is not used by default (it's listed under extras), thus shouldn't bother users that don't choose to install it.

We should choose a different name for the flag, though, IMHO.

Comment thread google/cloud/bigquery/opentelemetry_tracing.py Outdated
"add opentelemetry-api and opentelemetry-instrumentation "
"packages in order to get BigQuery Tracing data."
)
WARNED_OPENTELEMETRY = True

plamut Jan 7, 2021

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

(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)

tswast Jan 11, 2021

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

tswast added the automerge Merge the pull request once unit tests and other checks pass. label Jan 11, 2021
tswast merged commit 7ea6b7c into googleapis:master Jan 11, 2021
gcf-merge-on-green Bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jan 11, 2021
release-please Bot mentioned this pull request Jan 4, 2022
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

steffnay Awaiting requested review from steffnay
3 more reviewers
plamut plamut left review comments
shollyman shollyman approved these changes
mik-laj mik-laj approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: bigquery Issues related to the googleapis/python-bigquery API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Too noise logging about telemetry

4 participants

Back | FazBrowse Home | New Git URL