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.

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

chore(spanner): Add changes in Spanner executor for testing end to end tracing #10450

Open
wants to merge 47 commits into
base: main
Choose a base branch
from

Conversation

nareshz
Copy link
Contributor

@nareshz nareshz commented Jun 27, 2024

This PR adds changes in spanner executor code required for testing end to end tracing feature.
Dependent on #10241

@nareshz nareshz requested review from a team as code owners June 27, 2024 09:51
@product-auto-label product-auto-label bot added the api: spanner Issues related to the Spanner API. label Jun 27, 2024
spanner/client.go Outdated Show resolved Hide resolved
spanner/client.go Outdated Show resolved Hide resolved
@nareshz nareshz requested a review from harshachinta August 20, 2024 10:14
@harshachinta
Copy link
Contributor

LGTM with small comments.
@nareshz Can you share the systest run with this change?

@nareshz
Copy link
Contributor Author

nareshz commented Sep 11, 2024

LGTM with small comments. @nareshz Can you share the systest run with this change?

https://fusion2.corp.google.com/invocations/679341f2-7708-4795-98bd-4ea9ebe46768/targets

texporter "github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/trace"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/propagation"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
Copy link
Contributor

Choose a reason for hiding this comment

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

I am worried about this dep since we are adding a direct dependency of OTEL SDK package to the main package. This will download SDK dep to customer.

Can you check if we can have a seperate dependency file for executor which now relies on main package dependency. I have done something similar for opentelemetry tests since it also had SDK dependency.

https://github.com/googleapis/google-cloud-go/tree/main/spanner/test/opentelemetry/test

@harshachinta harshachinta added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 12, 2024
@codyoss
Copy link
Member

codyoss commented Dec 10, 2024

@harshachinta can we help get this merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API. do not merge Indicates a pull request not ready for merge, due to either quality or timing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants