Set billing project id as the quota project id in BigQuery #17796
Conversation
plugin/trino-bigquery/src/main/java/io/trino/plugin/bigquery/CredentialsOptionsConfigurer.java
Show resolved
Hide resolved
regadas
force-pushed
the
17795_quota_project
branch
from
5cb62c3 to
b4056ef
Compare
ebyhr
force-pushed
the
17795_quota_project
branch
from
b4056ef to
feae76c
Compare
Please use reserved comment https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword to close the issue automatically.
The commit title is obsolete. |
ebyhr
force-pushed
the
17795_quota_project
branch
from
feae76c to
cb3ee7e
Compare
|
/test-with-secrets sha=cb3ee7ee0810d3a8f086b03a2cfa3800300859e7 |
|
The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/5238574352 |
| } | ||
|
|
||
| @Config("bigquery.quota-project-id") | ||
| @ConfigDescription("The Google Cloud Project ID to attribute quota usage") |
There was a problem hiding this comment.
according to the javadocs both billing and quota happens against what is set via setQuotaProjectId.
The setParentProjectId is not clearly documented although it's what actually gets used for billing in absence of quota project id.
From all that I read so far it appears that:
- By default the quota project id is taken from the service account credentials which means there's no need to explicitly set it unless user credentials are being used instead of service accounts
So is my understanding correct that this change is related to your other change at #17717?
This is useful context IMO and maybe it makes more sense to implement both changes in a single PR and explain this background in the commit message. This also needs to be clarified in docs why this config exists otherwise users won't know when or when not to configure this.
There was a problem hiding this comment.
So is my understanding correct that this change is related to your other change at #17717?
Actually, the need for this stems out of the need to correctly attribute quota usage when using the Streaming API for writes. Currently, everything is being attributed to the table underlying project id.
By default the quota project id is taken from the service account credentials which means there's no need to explicitly set it unless user credentials are being used instead of service accounts
Couldn't fully confirm this but given the above, it seems that could be implementation specific.
That said this is why I feel that this should be a separate PR. Though I feel, given a snd thought that I'm gonna revert my changes and keep quota project id set with the Trino's parent project id.
There was a problem hiding this comment.
If the goal is to attribute quota and cost having a single config for that will be useful. Is there a case where you'd want different values for parent project id and quota project id?
There was a problem hiding this comment.
Is there a case where you'd want different values for parent project id and quota project id?
Yeah that's what I was reflecting about, and I think ideally they should be the same. The name "parent" is just an artifact of the storage api which in essence is sort of the same as the quota project id.
There was a problem hiding this comment.
If the goal is to attribute quota and cost having a single config for that will be useful.
+1 so I'm leaning in just keeping the bigquery.parent-project-id and probably augment a bit the docs to encompass the quota.
regadas
force-pushed
the
17795_quota_project
branch
from
cb3ee7e to
8d0a94e
Compare
Description
Fixes #17795
Additional context and related issues
Release notes
( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: