-
Notifications
You must be signed in to change notification settings - Fork 93
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
tracing: decide if each mutation should trigger a new span event to deal with the case of excessive mutations and storage #1269
Labels
api: spanner
Issues related to the googleapis/python-spanner API.
Comments
product-auto-label
bot
added
the
api: spanner
Issues related to the googleapis/python-spanner API.
label
Dec 16, 2024
odeke-em
added a commit
to odeke-em/python-spanner
that referenced
this issue
Dec 16, 2024
Referencing issue googleapis#1269, this update removes adding a span event per mutation, in favour of a future TODO.
odeke-em
added a commit
to odeke-em/python-spanner
that referenced
this issue
Dec 16, 2024
Referencing issue googleapis#1269, this update removes adding a span event per mutation, in favour of a future TODO.
odeke-em
added a commit
to odeke-em/python-spanner
that referenced
this issue
Dec 16, 2024
Referencing issue googleapis#1269, this update removes adding a span event per mutation, in favour of a future TODO.
odeke-em
added a commit
to odeke-em/python-spanner
that referenced
this issue
Dec 16, 2024
Referencing issue googleapis#1269, this update removes adding a span event per mutation, in favour of a future TODO.
harshachinta
pushed a commit
that referenced
this issue
Dec 17, 2024
* observability: add updated span events + traace more methods This change carves out parts of PR #1241 in smaller pieces to ease with smaller reviews. This change adds more span events, updates important spans to make them more distinct like changing: "CloudSpanner.ReadWriteTransaction" to more direct and more pointed spans like: * CloudSpanner.Transaction.execute_streaming_sql Also added important spans: * CloudSpanner.Database.run_in_transaction * CloudSpanner.Session.run_in_transaction * all: update review comments + show type for BeginTransaction + remove prints * Remove requested span event "Using Transaction" * Move attempts into try block * Transform Session.run_in_transaction retry exceptions into events * More comprehensive test for events and attributes for pool.get * Add test guards against Python3.7 for which OpenTelemetry is unavailable + address test feedback * Remove span event per mutation in favour of future TODO Referencing issue #1269, this update removes adding a span event per mutation, in favour of a future TODO. * Sort system-test.test_transaction_abort_then_retry_spans spans by create time * Delint tests
aakashanandg
pushed a commit
to aakashanandg/python-spanner
that referenced
this issue
Jan 2, 2025
* observability: add updated span events + traace more methods This change carves out parts of PR googleapis#1241 in smaller pieces to ease with smaller reviews. This change adds more span events, updates important spans to make them more distinct like changing: "CloudSpanner.ReadWriteTransaction" to more direct and more pointed spans like: * CloudSpanner.Transaction.execute_streaming_sql Also added important spans: * CloudSpanner.Database.run_in_transaction * CloudSpanner.Session.run_in_transaction * all: update review comments + show type for BeginTransaction + remove prints * Remove requested span event "Using Transaction" * Move attempts into try block * Transform Session.run_in_transaction retry exceptions into events * More comprehensive test for events and attributes for pool.get * Add test guards against Python3.7 for which OpenTelemetry is unavailable + address test feedback * Remove span event per mutation in favour of future TODO Referencing issue googleapis#1269, this update removes adding a span event per mutation, in favour of a future TODO. * Sort system-test.test_transaction_abort_then_retry_spans spans by create time * Delint tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In our tracing update reviews, everyone initially raised the need to record a span event whenever a new mutation had been added and I spun up some pull requests. However, @harshachinta wisely noted that if a customer creates 40,000 mutations to be added before invoking .commit(), 40,000 separate events would be created hence this would constitute a horrible user experience and also lots of data that a customer could be charged for spans per https://github.com/googleapis/python-spanner/pull/1259/files#r1875359721
Facts
num_mutations
when invoking .commit() that I had added as a span attribute perpython-spanner/google/cloud/spanner_v1/batch.py
Line 189 in a6811af
This issue is left as a reference for future decisions and to gain consensus with clear thought in time.
The text was updated successfully, but these errors were encountered: