Skip to content
This repository was archived by the owner on Sep 26, 2023. It is now read-only.

feat: enable setting quota_project_id #1128

Merged
merged 12 commits into from
Jul 24, 2020
Merged

feat: enable setting quota_project_id #1128

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
8e7b92f
Enable Quota Project ID in Client setting
summer-ji-eng Jun 13, 2020
c641572
Add quota project Id in clientContext
summer-ji-eng Jun 15, 2020
cea6118
change from quotaProjectID to quotaProjectId
summer-ji-eng Jun 18, 2020
a157f44
Add Executor
summer-ji-eng Jul 15, 2020
1151fdc
Add quotaProjectHidingCredentialsTest
summer-ji-eng Jul 16, 2020
c6b9d47
correct getRequestMetadata
summer-ji-eng Jul 16, 2020
0a8d179
Add unit test for QuotaProjectIdHidingCredentials
summer-ji-eng Jul 17, 2020
a63ffae
add unit test for client context
summer-ji-eng Jul 17, 2020
74541a5
Merge branch 'master' into quota_project
summer-ji-eng Jul 17, 2020
6a56e6b
Merge branch 'master' into quota_project
summer-ji-eng Jul 21, 2020
0e1e201
Add comment and beta annotation
summer-ji-eng Jul 22, 2020
7f91763
Update gax/src/test/java/com/google/api/gax/rpc/internal/QuotaProject…
summer-ji-eng Jul 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion gax/src/main/java/com/google/api/gax/rpc/ClientContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ public abstract class ClientContext {
@Nullable
public abstract String getEndpoint();

@Nullable
public abstract String getQuotaProjectID();

/** Gets the {@link ApiTracerFactory} that will be used to generate traces for operations. */
@BetaApi("The surface for tracing is not stable yet and may change in the future.")
@Nonnull
Expand All @@ -110,7 +113,8 @@ public static Builder newBuilder() {
.setClock(NanoClock.getDefaultClock())
.setStreamWatchdog(null)
.setStreamWatchdogCheckInterval(Duration.ZERO)
.setTracerFactory(NoopApiTracerFactory.getInstance());
.setTracerFactory(NoopApiTracerFactory.getInstance())
.setQuotaProjectID(null);
}

public abstract Builder toBuilder();
Expand Down Expand Up @@ -200,6 +204,7 @@ public static ClientContext create(StubSettings settings) throws IOException {
.setClock(clock)
.setDefaultCallContext(defaultCallContext)
.setEndpoint(settings.getEndpoint())
.setQuotaProjectID(settings.getQuotaProjectID())
.setStreamWatchdog(watchdog)
.setStreamWatchdogCheckInterval(settings.getStreamWatchdogCheckInterval())
.setTracerFactory(settings.getTracerFactory())
Expand Down Expand Up @@ -229,6 +234,8 @@ public abstract static class Builder {

public abstract Builder setEndpoint(String endpoint);

public abstract Builder setQuotaProjectID(String quotaProjectID);

@BetaApi("The surface for streaming is not stable yet and may change in the future.")
public abstract Builder setStreamWatchdog(Watchdog watchdog);

Expand Down
16 changes: 16 additions & 0 deletions gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ public final String getEndpoint() {
return stubSettings.getEndpoint();
}

public final String getQuotaProjectID() {
return stubSettings.getQuotaProjectID();
}

@BetaApi("The surface for streaming is not stable yet and may change in the future.")
@Nullable
public final WatchdogProvider getWatchdogProvider() {
Expand All @@ -114,6 +118,7 @@ public String toString() {
.add("internalHeaderProvider", getInternalHeaderProvider())
.add("clock", getClock())
.add("endpoint", getEndpoint())
.add("quotaProjectID", getQuotaProjectID())
.add("watchdogProvider", getWatchdogProvider())
.add("watchdogCheckInterval", getWatchdogCheckInterval())
.toString();
Expand Down Expand Up @@ -216,6 +221,11 @@ public B setEndpoint(String endpoint) {
return self();
}

public B setQuotaProjectID(String quotaProjectID) {
stubSettings.setQuotaProjectID(quotaProjectID);
return self();
}

@BetaApi("The surface for streaming is not stable yet and may change in the future.")
public B setWatchdogProvider(@Nullable WatchdogProvider watchdogProvider) {
stubSettings.setStreamWatchdogProvider(watchdogProvider);
Expand Down Expand Up @@ -264,6 +274,11 @@ public String getEndpoint() {
return stubSettings.getEndpoint();
}

/** Gets the QuotaProjectID that was previously set on this Builder. */
public String getQuotaProjectID() {
return stubSettings.getQuotaProjectID();
}

@BetaApi("The surface for streaming is not stable yet and may change in the future.")
@Nullable
public WatchdogProvider getWatchdogProvider() {
Expand Down Expand Up @@ -294,6 +309,7 @@ public String toString() {
.add("internalHeaderProvider", getInternalHeaderProvider())
.add("clock", getClock())
.add("endpoint", getEndpoint())
.add("quotaProjectID", getQuotaProjectID())
.add("watchdogProvider", getWatchdogProvider())
.add("watchdogCheckInterval", getWatchdogCheckInterval())
.toString();
Expand Down
59 changes: 59 additions & 0 deletions gax/src/main/java/com/google/api/gax/rpc/StubSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
import com.google.api.gax.core.NoCredentialsProvider;
import com.google.api.gax.tracing.ApiTracerFactory;
import com.google.api.gax.tracing.NoopApiTracerFactory;
import com.google.auth.Credentials;
import com.google.auth.oauth2.QuotaProjectIdProvider;
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
import java.io.IOException;
Expand All @@ -60,13 +62,16 @@
*/
public abstract class StubSettings<SettingsT extends StubSettings<SettingsT>> {

static final String QUOTA_PROJECT_ID_HEADER_KEY = "x-google-user-project";

private final ExecutorProvider executorProvider;
private final CredentialsProvider credentialsProvider;
private final HeaderProvider headerProvider;
private final HeaderProvider internalHeaderProvider;
private final TransportChannelProvider transportChannelProvider;
private final ApiClock clock;
private final String endpoint;
private final String quotaProjectID;
@Nullable private final WatchdogProvider streamWatchdogProvider;
@Nonnull private final Duration streamWatchdogCheckInterval;
@Nonnull private final ApiTracerFactory tracerFactory;
Expand All @@ -80,6 +85,7 @@ protected StubSettings(Builder builder) {
this.internalHeaderProvider = builder.internalHeaderProvider;
this.clock = builder.clock;
this.endpoint = builder.endpoint;
this.quotaProjectID = builder.quotaProjectID;
this.streamWatchdogProvider = builder.streamWatchdogProvider;
this.streamWatchdogCheckInterval = builder.streamWatchdogCheckInterval;
this.tracerFactory = builder.tracerFactory;
Expand Down Expand Up @@ -115,6 +121,10 @@ public final String getEndpoint() {
return endpoint;
}

public final String getQuotaProjectID() {
return quotaProjectID;
}

@BetaApi("The surface for streaming is not stable yet and may change in the future.")
@Nullable
public final WatchdogProvider getStreamWatchdogProvider() {
Expand Down Expand Up @@ -146,6 +156,7 @@ public String toString() {
.add("internalHeaderProvider", internalHeaderProvider)
.add("clock", clock)
.add("endpoint", endpoint)
.add("quotaProjectID", quotaProjectID)
.add("streamWatchdogProvider", streamWatchdogProvider)
.add("streamWatchdogCheckInterval", streamWatchdogCheckInterval)
.add("tracerFactory", tracerFactory)
Expand All @@ -164,6 +175,7 @@ public abstract static class Builder<
private TransportChannelProvider transportChannelProvider;
private ApiClock clock;
private String endpoint;
private String quotaProjectID;
@Nullable private WatchdogProvider streamWatchdogProvider;
@Nonnull private Duration streamWatchdogCheckInterval;
@Nonnull private ApiTracerFactory tracerFactory;
Expand All @@ -177,11 +189,29 @@ protected Builder(StubSettings settings) {
this.internalHeaderProvider = settings.internalHeaderProvider;
this.clock = settings.clock;
this.endpoint = settings.endpoint;
this.quotaProjectID = settings.quotaProjectID;
this.streamWatchdogProvider = settings.streamWatchdogProvider;
this.streamWatchdogCheckInterval = settings.streamWatchdogCheckInterval;
this.tracerFactory = settings.tracerFactory;
}

/** Get Quota Project ID from Client Context * */
private static String getQuotaProjectIDFromClientContext(ClientContext clientContext) {
Copy link

Choose a reason for hiding this comment

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

Seems like we use ID in methods and variables, and Id in class names. Is this idiomatic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ID used in all methods and variables, which is really just my preference. I haven't find any guideline for naming.
Let me know if any rule need to be follow here, I could make change.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should use quotaProjectId. See https://google.github.io/styleguide/javaguide.html#s5-naming

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will make change to quotaProjectId

if (clientContext.getQuotaProjectID() != null) {
return clientContext.getQuotaProjectID();
}
if (clientContext.getCredentials() instanceof QuotaProjectIdProvider) {
return ((QuotaProjectIdProvider) clientContext.getCredentials()).getQuotaProjectId();
}
if (clientContext.getHeaders().containsKey(QUOTA_PROJECT_ID_HEADER_KEY)) {
return clientContext.getHeaders().get(QUOTA_PROJECT_ID_HEADER_KEY);
}
if (clientContext.getInternalHeaders().containsKey(QUOTA_PROJECT_ID_HEADER_KEY)) {
Copy link

Choose a reason for hiding this comment

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

What is the difference between headers and internal headers? And is it possible for quota project to come in on either one?

Also, what is the use case for pulling it from the header? I think maybe it is not necessary to detect this (@broady ?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

quota project id can be gotten from header and internal header. double confirm with @chingor13

but I am not sure the use case. @broady

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the internal header provider is reserved for gapic and handwritten layers to inject headers. The general header provider is to allow library users to specify arbitrary extra headers.

It's not desired for library users to set the quota project via header provider, but previously that was the only mechanism for doing so. We should have an explicit strategy for how to handle that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for clarifying.

Copy link

Choose a reason for hiding this comment

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

It seems ok to support this then, for backwards compatibility. And also to ensure any quota project behavior stays consistent regardless of how it's set.

return clientContext.getInternalHeaders().get(QUOTA_PROJECT_ID_HEADER_KEY);
}
return null;
}

protected Builder(ClientContext clientContext) {
if (clientContext == null) {
this.executorProvider = InstantiatingExecutorProvider.newBuilder().build();
Expand All @@ -191,6 +221,7 @@ protected Builder(ClientContext clientContext) {
this.internalHeaderProvider = new NoHeaderProvider();
this.clock = NanoClock.getDefaultClock();
this.endpoint = null;
this.quotaProjectID = null;
this.streamWatchdogProvider = InstantiatingWatchdogProvider.create();
this.streamWatchdogCheckInterval = Duration.ofSeconds(10);
this.tracerFactory = NoopApiTracerFactory.getInstance();
Expand All @@ -208,6 +239,7 @@ protected Builder(ClientContext clientContext) {
FixedWatchdogProvider.create(clientContext.getStreamWatchdog());
this.streamWatchdogCheckInterval = clientContext.getStreamWatchdogCheckInterval();
this.tracerFactory = clientContext.getTracerFactory();
this.quotaProjectID = getQuotaProjectIDFromClientContext(clientContext);
}
}

Expand All @@ -234,6 +266,14 @@ public B setExecutorProvider(ExecutorProvider executorProvider) {
/** Sets the CredentialsProvider to use for getting the credentials to make calls with. */
public B setCredentialsProvider(CredentialsProvider credentialsProvider) {
this.credentialsProvider = Preconditions.checkNotNull(credentialsProvider);
try {
Credentials credentials = credentialsProvider.getCredentials();
if (this.quotaProjectID == null && credentials instanceof QuotaProjectIdProvider) {
this.quotaProjectID = ((QuotaProjectIdProvider) credentials).getQuotaProjectId();
}
} catch (IOException e) {
System.out.println("fail to fetch credentials");
}
return self();
}

Expand All @@ -247,6 +287,10 @@ public B setCredentialsProvider(CredentialsProvider credentialsProvider) {
@BetaApi("The surface for customizing headers is not stable yet and may change in the future.")
public B setHeaderProvider(HeaderProvider headerProvider) {
this.headerProvider = headerProvider;
if (this.quotaProjectID == null
&& headerProvider.getHeaders().containsKey(QUOTA_PROJECT_ID_HEADER_KEY)) {
this.quotaProjectID = headerProvider.getHeaders().get(QUOTA_PROJECT_ID_HEADER_KEY);
}
return self();
}

Expand All @@ -260,6 +304,10 @@ public B setHeaderProvider(HeaderProvider headerProvider) {
@BetaApi("The surface for customizing headers is not stable yet and may change in the future.")
protected B setInternalHeaderProvider(HeaderProvider internalHeaderProvider) {
this.internalHeaderProvider = internalHeaderProvider;
if (this.quotaProjectID == null
&& internalHeaderProvider.getHeaders().containsKey(QUOTA_PROJECT_ID_HEADER_KEY)) {
this.quotaProjectID = internalHeaderProvider.getHeaders().get(QUOTA_PROJECT_ID_HEADER_KEY);
}
return self();
}

Expand Down Expand Up @@ -298,6 +346,11 @@ public B setEndpoint(String endpoint) {
return self();
}

public B setQuotaProjectID(String quotaProjectID) {
this.quotaProjectID = quotaProjectID;
return self();
}

/**
* Sets how often the {@link Watchdog} will check ongoing streaming RPCs. Defaults to 10 secs.
* Use {@link Duration#ZERO} to disable.
Expand Down Expand Up @@ -364,6 +417,11 @@ public String getEndpoint() {
return endpoint;
}

/** Gets the QuotaProjectID that was previously set on this Builder. */
public String getQuotaProjectID() {
return quotaProjectID;
}

@BetaApi("The surface for streaming is not stable yet and may change in the future.")
@Nonnull
public Duration getStreamWatchdogCheckInterval() {
Expand Down Expand Up @@ -396,6 +454,7 @@ public String toString() {
.add("internalHeaderProvider", internalHeaderProvider)
.add("clock", clock)
.add("endpoint", endpoint)
.add("quotaProjectID", quotaProjectID)
.add("streamWatchdogProvider", streamWatchdogProvider)
.add("streamWatchdogCheckInterval", streamWatchdogCheckInterval)
.add("tracerFactory", tracerFactory)
Expand Down
Loading