Skip to content
This repository was archived by the owner on Jun 8, 2026. It is now read-only.

feat(observability): more descriptive and value adding spans - #1241

Closed
odeke-em wants to merge 1 commit into
googleapis:mainfrom
odeke-em:trace-update-more-descriptive-Database.run_in_transaction-spans
Closed

feat(observability): more descriptive and value adding spans#1241
odeke-em wants to merge 1 commit into
googleapis:mainfrom
odeke-em:trace-update-more-descriptive-Database.run_in_transaction-spans

Conversation

@odeke-em

@odeke-emodeke-em commented Nov 18, 2024

Copy link
Copy Markdown
Contributor

This change adds more descriptive and value adding spans to replace the generic CloudSpanner.ReadWriteTransaction. With this change, we add new spans:

  • CloudSpanner.Database.run_in_transaction
  • CloudSpanner.execute_pdml
  • CloudSpanner.execute_sql
  • CloudSpanner.execute_update

Exhibit

Given this code

deftx_dql_select(tx):
try:
tx.execute_update("DELETE FROM Singers WHERE 1=1")
except:
passtx.execute_update(
'INSERT INTO Singers(SingerId, FirstName, LastName) VALUES(1000, "Bryan", "Adams"), (2000, "Slash", "GunsNRoses")'
)
tx.update(
"Singers",
columns=["SingerId", "FirstName"],
values=[["1", "Bryan"], ["2", "Slash"]],
)
rows=tx.execute_sql("SELECT * FROM Singers")
forrowinrows:
print(row)

we now have this exhibit

Without gRPC instrumentation

Screenshot 2024-11-18 at 2 08 46 AM

With gRPC instrumentation

Screenshot 2024-11-18 at 2 12 26 AM

@odeke-em
odeke-em requested review from a teamNovember 18, 2024 10:09
@product-auto-labelproduct-auto-labelBot added size: m Pull request size is medium. api: spanner Issues related to the googleapis/python-spanner API. labels Nov 18, 2024
@odeke-em
odeke-emforce-pushed the trace-update-more-descriptive-Database.run_in_transaction-spans branch from bc1befe to 502d0e5CompareNovember 18, 2024 22:55
Comment threadgoogle/cloud/spanner_v1/database.py Outdated
Comment threadgoogle/cloud/spanner_v1/database.py
Comment threadgoogle/cloud/spanner_v1/database.py Outdated
Comment threadgoogle/cloud/spanner_v1/database.py
Comment threadgoogle/cloud/spanner_v1/database.py Outdated
Comment threadgoogle/cloud/spanner_v1/_opentelemetry_tracing.py Outdated
Comment threadgoogle/cloud/spanner_v1/snapshot.py Outdated
Comment threadgoogle/cloud/spanner_v1/transaction.py Outdated
Comment threadgoogle/cloud/spanner_v1/transaction.py Outdated
Comment threadgoogle/cloud/spanner_v1/session.py Outdated
@odeke-em

Copy link
Copy Markdown
ContributorAuthor

Kindly help me take a look @harshachinta and help me run the bots afresh!

Comment threadgoogle/cloud/spanner_v1/database.py Outdated
Comment on lines +699 to +707
if span:
span.add_event("Starting BeginTransaction")
txn = api.begin_transaction(
session=session.name, options=txn_options, metadata=metadata
)
if span:
span.add_event(
"Completed BeginTransaction", {"transaction.id": txn.id}
)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@harshachinta please take a look.

Comment threadgoogle/cloud/spanner_v1/session.py Outdated
@odeke-em

Copy link
Copy Markdown
ContributorAuthor

@harshachinta kindly help me run the bots one more time, I sent up a fix!

@odeke-em
odeke-emforce-pushed the trace-update-more-descriptive-Database.run_in_transaction-spans branch 2 times, most recently from 8d613ee to f767159CompareNovember 20, 2024 12:06
@sakthivelmaniisakthivelmanii added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 20, 2024
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 20, 2024
@product-auto-labelproduct-auto-labelBot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Nov 22, 2024
Comment threadgoogle/cloud/spanner_v1/_opentelemetry_tracing.py
Comment threadgoogle/cloud/spanner_v1/database.py Outdated
Comment on lines +738 to +739
if isinstance(observability_options, dict):
observability_options["db_name"] = self.name

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 can do a much simpler thing instead of adding db_name manually at every place.
We have configured a @Property for observability_options in database.py

defobservability_options(self):

Can we add a new field, db_name, to observability_options if it already exists? If the customer does not provide any observability_options, we can create a new dictionary with this field. With this we need not do it at multiple places, WDYT?

Comment threadgoogle/cloud/spanner_v1/database.py Outdated
if isinstance(observability_options, dict):
observability_options["db_name"] = self.name
with trace_call(
"CloudSpanner.execute_partitioned_pdml",

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 problem i see here is with every retry there will be a new span parent CloudSpanner.execute_partitioned_pdml which does not indicate that a retry is happening.
Instead we should capture the retries with in only one parent span CloudSpanner.execute_partitioned_pdml.

Comment threadgoogle/cloud/spanner_v1/database.py Outdated
Comment threadgoogle/cloud/spanner_v1/pool.py Outdated
@odeke-em
odeke-emforce-pushed the trace-update-more-descriptive-Database.run_in_transaction-spans branch 3 times, most recently from beb8bb9 to d297b66CompareNovember 28, 2024 07:41
@harshachintaharshachinta added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 29, 2024
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 29, 2024
@odeke-em
odeke-emforce-pushed the trace-update-more-descriptive-Database.run_in_transaction-spans branch from be40273 to 47ffdfcCompareDecember 2, 2024 05:24
@harshachintaharshachinta added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 2, 2024
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 2, 2024
@odeke-em
odeke-emforce-pushed the trace-update-more-descriptive-Database.run_in_transaction-spans branch from cd4a8b1 to 4829a9aCompareDecember 2, 2024 12:57
@product-auto-labelproduct-auto-labelBot removed the size: l Pull request size is large. label Dec 3, 2024
@product-auto-labelproduct-auto-labelBot added the size: xl Pull request size is extra large. label Dec 3, 2024
@odeke-em
odeke-emforce-pushed the trace-update-more-descriptive-Database.run_in_transaction-spans branch from 8d6dad0 to eecca2eCompareDecember 3, 2024 13:35
@harshachintaharshachinta added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 4, 2024
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 4, 2024
@odeke-em
odeke-emforce-pushed the trace-update-more-descriptive-Database.run_in_transaction-spans branch 3 times, most recently from 204067a to 847d89bCompareDecember 6, 2024 11:30
odeke-em added a commit to odeke-em/python-spanner that referenced this pull request Dec 6, 2024
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
odeke-em added a commit to odeke-em/python-spanner that referenced this pull request Dec 6, 2024
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
odeke-em added a commit to odeke-em/python-spanner that referenced this pull request Dec 6, 2024
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
odeke-em added a commit to odeke-em/python-spanner that referenced this pull request Dec 7, 2024
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
odeke-em added a commit to odeke-em/python-spanner that referenced this pull request Dec 7, 2024
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
odeke-em added a commit to odeke-em/python-spanner that referenced this pull request Dec 16, 2024
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
harshachinta pushed a commit that referenced this pull request 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
This change adds more descriptive and value adding spans to
replace the generic CloudSpanner.ReadWriteTransaction.
With this change, we add new spans:
* CloudSpanner.Database.run_in_transaction
* CloudSpanner.execute_pdml
* CloudSpanner.execute_sql
* CloudSpanner.execute_update
@odeke-em
odeke-emforce-pushed the trace-update-more-descriptive-Database.run_in_transaction-spans branch from 847d89b to f28ed47CompareDecember 17, 2024 10:38
@product-auto-labelproduct-auto-labelBot added size: l Pull request size is large. and removed size: xl Pull request size is extra large. labels Dec 17, 2024
odeke-em added a commit to odeke-em/python-spanner that referenced this pull request Dec 17, 2024
This change adds spans for Partitioned DML and making
updates for Batch.
Carved out from PR googleapis#1241.
odeke-em added a commit to odeke-em/python-spanner that referenced this pull request Dec 27, 2024
This change adds spans for Partitioned DML and making
updates for Batch.
Carved out from PR googleapis#1241.
aakashanandg pushed a commit to aakashanandg/python-spanner that referenced this pull request 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 #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
odeke-em added a commit to odeke-em/python-spanner that referenced this pull request Jan 9, 2025
This change adds spans for Partitioned DML and making
updates for Batch.
Carved out from PR googleapis#1241.
odeke-em added a commit to odeke-em/python-spanner that referenced this pull request Jan 10, 2025
This change adds spans for Partitioned DML and making
updates for Batch.
Carved out from PR googleapis#1241.
harshachinta pushed a commit that referenced this pull request Jan 10, 2025
* observability: PDML + some batch write spans
This change adds spans for Partitioned DML and making
updates for Batch.
Carved out from PR #1241.
* Add more system tests
* Account for lack of OpenTelemetry on Python-3.7
* Update tests
* Fix more test assertions
* Updates from code review
* Update tests with code review suggestions
* Remove return per code review nit
@odeke-em

Copy link
Copy Markdown
ContributorAuthor

Was split up but can be revived if needed.

@odeke-emodeke-em closed this Jun 5, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: spannerIssues related to the googleapis/python-spanner API.size: lPull request size is large.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@odeke-em@harshachinta@rahul2393@yoshi-kokoro@sakthivelmanii