Skip to content

feat(spanner): add setMetricsProjectId to fix metrics export#13241

Open
RRap0so wants to merge 1 commit into
googleapis:mainfrom
RRap0so:fix/metrics-project-default
Open

feat(spanner): add setMetricsProjectId to fix metrics export#13241
RRap0so wants to merge 1 commit into
googleapis:mainfrom
RRap0so:fix/metrics-project-default

Conversation

@RRap0so
Copy link
Copy Markdown

@RRap0so RRap0so commented May 20, 2026

Fixes #13240

Problem

On GKE with shared VPC, SpannerOptions.getProjectId() defaults to the host project via the metadata server. While DatabaseId correctly routes database operations to the application project, the client-side metrics exporter uses getProjectId() — causing createServiceTimeSeries to target the wrong project and fail with permission errors.

Changes

  • Add setMetricsProjectId(String) to SpannerOptions.Builder
  • Add resolveMetricsProjectId() that prefers explicit value, falls back to getProjectId()
  • Update 3 metrics export call sites to use resolveMetricsProjectId()
  • Log WARNING in getDatabaseClient() when DatabaseId project differs from metrics project

Usage

SpannerOptions options = SpannerOptions.newBuilder()
    .setMetricsProjectId("my-app-project")
    .build();

When not set, behavior is unchanged (uses getProjectId()).

@RRap0so RRap0so requested review from a team as code owners May 20, 2026 16:22
@google-cla
Copy link
Copy Markdown

google-cla Bot commented May 20, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@RRap0so RRap0so changed the title feat(spanner): add setMetricsProjectId to fix metrics export on GKE shared VPC feat(spanner): add setMetricsProjectId to fix metrics export May 20, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the ability to explicitly configure a project ID for client-side metrics export via SpannerOptions, addressing potential permission issues in environments like GKE with shared VPCs. The changes include adding a metricsProjectId field to the SpannerOptions builder and updating metrics initialization to use this resolved project ID. Feedback suggests moving the project mismatch warning in SpannerImpl to the client initialization block to avoid excessive logging on every getDatabaseClient call and ensuring that equals() and hashCode() in SpannerOptions are updated to include the new field.

Copy link
Copy Markdown
Contributor

@sakthivelmanii sakthivelmanii left a comment

Choose a reason for hiding this comment

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

Can you address these comments and rebase it with the latest main?

@RRap0so RRap0so force-pushed the fix/metrics-project-default branch 4 times, most recently from f9503a2 to 106610d Compare May 25, 2026 07:51
@RRap0so RRap0so requested a review from sakthivelmanii May 25, 2026 07:58
@RRap0so
Copy link
Copy Markdown
Author

RRap0so commented May 25, 2026

Hey @sakthivelmanii, thank you for the review. Simplified it a bit and removed the log line probably not necessary.

SpannerOptions.getProjectId() can resolve to the wrong project (e.g.
host project on GKE), causing createServiceTimeSeries to fail with
permission errors. Add resolveMetricsProjectId() which defaults to the
database project and setMetricsProjectId(String) to allow explicit
override when needed.
@RRap0so RRap0so force-pushed the fix/metrics-project-default branch from 106610d to 209cabf Compare May 25, 2026 07:59
@rahul2393
Copy link
Copy Markdown
Contributor

rahul2393 commented May 25, 2026

@RRap0so thanks for raising the PR, can you check if this fix #13262 is sufficient to solve your issue? by exposing a public method it is possible to export built-in metric for a database to separate project(though we can log warning) which is not expected, and the fix I shared handled those cases without exposing an option.

@RRap0so
Copy link
Copy Markdown
Author

RRap0so commented May 25, 2026

@rahul2393 you folks will know best. If it's not intended for users to define a different Metrics Project ID then your PR should do it.

@rahul2393
Copy link
Copy Markdown
Contributor

Ok, From spanner side we ideally would not like to expose that as public setter, let me try getting reviews on the PR, if it's not reviewed or I see concern from team, we will go ahead with the public setter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spanner: client-side metrics export uses wrong project

4 participants