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

observability: PDML + some batch write spans - #1274

Merged
harshachinta merged 8 commits into
googleapis:mainfrom
odeke-em:trace-pdml
Jan 10, 2025
Merged

observability: PDML + some batch write spans#1274
harshachinta merged 8 commits into
googleapis:mainfrom
odeke-em:trace-pdml

Conversation

@odeke-em

@odeke-emodeke-em commented Dec 17, 2024

Copy link
Copy Markdown
Contributor

This change adds spans for Partitioned DML and making updates for Batch.

Carved out from PR #1241.

@odeke-em
odeke-em requested review from a teamDecember 17, 2024 12:55
@product-auto-labelproduct-auto-labelBot added size: l Pull request size is large. api: spanner Issues related to the googleapis/python-spanner API. labels Dec 17, 2024
@odeke-em
odeke-emforce-pushed the trace-pdml branch 2 times, most recently from c11580f to 7dfb796CompareDecember 27, 2024 14:12
@odeke-em

odeke-em commented Dec 27, 2024

Copy link
Copy Markdown
ContributorAuthor

@harshachinta@olavloite kindly help me run the bots for this PR. Thank you.

@harshachintaharshachinta added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 27, 2024
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 27, 2024
@odeke-em

odeke-em commented Dec 28, 2024

Copy link
Copy Markdown
ContributorAuthor

@harshachinta@olavloite kindly help me re-run the bots, I had to account for Python3.7 in the tests and fixed that, all pass locally.

@harshachintaharshachinta added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 28, 2024
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 28, 2024
harshachinta
harshachinta reviewed Jan 8, 2025
View reviewed changes
Comment threadgoogle/cloud/spanner_v1/database.py
Comment threadgoogle/cloud/spanner_v1/database.py Outdated
Comment threadgoogle/cloud/spanner_v1/merged_result_set.py
Comment threadgoogle/cloud/spanner_v1/session.py Outdated
Comment threadgoogle/cloud/spanner_v1/snapshot.py
Comment threadtests/system/test_observability_options.py Outdated
Comment threadtests/system/test_observability_options.py
@harshachintaharshachinta added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 8, 2025
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 8, 2025
harshachinta
harshachinta reviewed Jan 8, 2025
View reviewed changes
Comment threadtests/unit/test_snapshot.py Outdated
Comment on lines +1233 to +1234
if not HAS_OPENTELEMETRY_INSTALLED:
return

@harshachintaharshachintaJan 8, 2025

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.

This is not needed right since assertSpanAttributes method already checks for this condition?

@odeke-emodeke-emJan 9, 2025

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.

Yeah, before I was manually comparing attributes and span names but with the update that I made I no longer need it, done, thanks!

Comment threadtests/unit/test_snapshot.py Outdated
Comment on lines +1231 to +1244
attributes=dict(
BASE_ATTRIBUTES, table_id=TABLE_NAME, columns=tuple(COLUMNS)
),
attributes=want_span_attributes,

@harshachintaharshachintaJan 8, 2025

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.

Nothing has changed here? why not keep it the same?

Comment threadtests/unit/test_snapshot.py
Comment threadtests/unit/test_transaction.py Outdated
Comment threadtests/unit/test_transaction.py Outdated
@odeke-em

odeke-em commented Jan 9, 2025

Copy link
Copy Markdown
ContributorAuthor

@harshachinta kindly help me run the bots. Thank you

@sakthivelmaniisakthivelmanii added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 9, 2025
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 9, 2025
harshachinta
harshachinta approved these changes Jan 10, 2025
View reviewed changes
Comment threadgoogle/cloud/spanner_v1/merged_result_set.py Outdated
@odeke-em

odeke-em commented Jan 10, 2025

Copy link
Copy Markdown
ContributorAuthor

@harshachinta all done, kindly help me kick off the bots!

@odeke-em

odeke-em commented Jan 10, 2025

Copy link
Copy Markdown
ContributorAuthor

@alkatrivedi@sakthivelmanii kindly help me run the bots

@harshachintaharshachinta added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 10, 2025
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 10, 2025
@harshachinta
harshachinta merged commit 592047f into googleapis:mainJan 10, 2025
@odeke-em
odeke-em deleted the trace-pdml branch January 10, 2025 12:01
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@harshachintaharshachintaharshachinta approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@asthamohtaasthamohta

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

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