-
Notifications
You must be signed in to change notification settings - Fork 52
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
feat(gax): add protobuf version tracking to headers #3199
base: main
Are you sure you want to change the base?
Changes from 18 commits
31ebd95
e6aea43
a64e14b
3a7b27d
0037a0a
ea7e7ec
4e18aa6
b1a8960
8d6065c
2dbc3a4
b69684b
02051b3
31da0e5
5a918f4
94d3f45
822ac0c
390c922
01eb41a
92e901b
a202b8b
f9fac2d
9849efd
604f00d
9c51ec7
a99c4b1
c7b3d2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,9 +32,15 @@ | |
import com.google.api.core.InternalApi; | ||
import com.google.common.annotations.VisibleForTesting; | ||
import com.google.common.base.Strings; | ||
import com.google.protobuf.Any; | ||
import java.io.File; | ||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.net.URISyntaxException; | ||
import java.util.Optional; | ||
import java.util.Properties; | ||
import java.util.jar.Attributes; | ||
import java.util.jar.JarFile; | ||
|
||
/** Provides properties of the GAX library. */ | ||
@InternalApi | ||
|
@@ -43,6 +49,9 @@ public class GaxProperties { | |
private static final String DEFAULT_VERSION = ""; | ||
private static final String GAX_VERSION = getLibraryVersion(GaxProperties.class, "version.gax"); | ||
private static final String JAVA_VERSION = getRuntimeVersion(); | ||
private static final String PROTOBUF_VERSION = | ||
getBundleVersion(Any.class).orElse(DEFAULT_VERSION); | ||
static final String BUNDLE_VERSION_KEY = "Bundle-Version"; | ||
|
||
private GaxProperties() {} | ||
|
||
|
@@ -91,6 +100,11 @@ public static String getGaxVersion() { | |
return GAX_VERSION; | ||
} | ||
|
||
/** Returns the current version of protobuf runtime library. */ | ||
public static String getProtobufVersion() { | ||
return PROTOBUF_VERSION; | ||
} | ||
|
||
/** | ||
* Returns the current runtime version. For GraalVM the values in this method will be fetched at | ||
* build time and the values should not differ from the runtime (executable) | ||
|
@@ -113,4 +127,26 @@ static String getRuntimeVersion() { | |
// with hyphens. | ||
return javaRuntimeInformation.replaceAll("[^0-9a-zA-Z_\\\\.]", "-"); | ||
} | ||
|
||
/** | ||
* Returns the current library version as reported by {BUNDLE_VERSION_KEY} in library's | ||
* META-INF/MANIFEST. This should only be used if MANIFEST file does not contain a widely | ||
* recognized version declaration such as Specific-Version OR Implementation-Version declared in | ||
* Manifest Specification | ||
* https://docs.oracle.com/javase/8/docs/technotes/guides/jar/jar.html#Manifest_Specification, | ||
* otherwise please use #getLibraryVersion | ||
*/ | ||
@VisibleForTesting | ||
static Optional<String> getBundleVersion(Class<?> clazz) { | ||
try { | ||
File file = new File(clazz.getProtectionDomain().getCodeSource().getLocation().toURI()); | ||
try (JarFile jar = new JarFile(file.getPath())) { | ||
Attributes attributes = jar.getManifest().getMainAttributes(); | ||
return Optional.ofNullable(attributes.getValue(BUNDLE_VERSION_KEY)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can inline BUNDLE_VERSION_KEY if it is not expected to be reused. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
} | ||
} catch (URISyntaxException | IOException e) { | ||
// Unable to read Bundle-Version from manifest. Recover gracefully. | ||
return Optional.empty(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,8 @@ | |
import com.google.common.collect.ImmutableMap; | ||
import java.io.Serializable; | ||
import java.util.Map; | ||
import java.util.regex.Matcher; | ||
import java.util.regex.Pattern; | ||
|
||
/** | ||
* Implementation of HeaderProvider that provides headers describing the API client library making | ||
|
@@ -41,8 +43,11 @@ | |
public class ApiClientHeaderProvider implements HeaderProvider, Serializable { | ||
ldetmer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private static final long serialVersionUID = -8876627296793342119L; | ||
static final String QUOTA_PROJECT_ID_HEADER_KEY = "x-goog-user-project"; | ||
static final String PROTOBUF_HEADER_VERSION_KEY = "protobuf"; | ||
|
||
public static final String API_VERSION_HEADER_KEY = "x-goog-api-version"; | ||
private static final String protobufVersionAppendValue = | ||
"--" + PROTOBUF_HEADER_VERSION_KEY + "-" + GaxProperties.getProtobufVersion(); | ||
|
||
private final Map<String, String> headers; | ||
|
||
|
@@ -57,8 +62,12 @@ protected ApiClientHeaderProvider(Builder builder) { | |
appendToken(apiClientHeaderValue, builder.getGeneratedLibToken()); | ||
appendToken(apiClientHeaderValue, builder.getGeneratedRuntimeToken()); | ||
appendToken(apiClientHeaderValue, builder.getTransportToken()); | ||
appendToken(apiClientHeaderValue, builder.getProtobufRuntimeToken()); | ||
|
||
if (apiClientHeaderValue.length() > 0) { | ||
headersBuilder.put(builder.getApiClientHeaderKey(), apiClientHeaderValue.toString()); | ||
headersBuilder.put( | ||
builder.getApiClientHeaderKey(), | ||
checkAndAppendProtobufVersionIfNecessary(apiClientHeaderValue)); | ||
} | ||
} | ||
|
||
|
@@ -76,6 +85,19 @@ protected ApiClientHeaderProvider(Builder builder) { | |
this.headers = headersBuilder.build(); | ||
} | ||
|
||
private static String checkAndAppendProtobufVersionIfNecessary( | ||
StringBuilder apiClientHeaderValue) { | ||
// TODO(b/366417603): appending protobuf version to existing client library token until resolved | ||
Pattern pattern = Pattern.compile("(gccl|gapic)\\S*"); | ||
Matcher matcher = pattern.matcher(apiClientHeaderValue); | ||
if (matcher.find()) { | ||
return apiClientHeaderValue.substring(0, matcher.end()) | ||
+ protobufVersionAppendValue | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this field can be inlined too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
+ apiClientHeaderValue.substring(matcher.end()); | ||
} | ||
return apiClientHeaderValue.toString(); | ||
} | ||
|
||
@Override | ||
public Map<String, String> getHeaders() { | ||
return headers; | ||
|
@@ -110,6 +132,7 @@ public static class Builder { | |
private String generatedRuntimeToken; | ||
private String transportToken; | ||
private String quotaProjectIdToken; | ||
private final String protobufRuntimeToken; | ||
|
||
private String resourceHeaderKey; | ||
private String resourceToken; | ||
|
@@ -125,11 +148,11 @@ protected Builder() { | |
setClientRuntimeToken(GaxProperties.getGaxVersion()); | ||
transportToken = null; | ||
quotaProjectIdToken = null; | ||
|
||
resourceHeaderKey = getDefaultResourceHeaderKey(); | ||
resourceToken = null; | ||
|
||
apiVersionToken = null; | ||
protobufRuntimeToken = | ||
constructToken(PROTOBUF_HEADER_VERSION_KEY, GaxProperties.getProtobufVersion()); | ||
} | ||
|
||
public String getApiClientHeaderKey() { | ||
|
@@ -224,6 +247,10 @@ public Builder setApiVersionToken(String apiVersionToken) { | |
return this; | ||
} | ||
|
||
public String getProtobufRuntimeToken() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know other fields have a public getter, but I don't think they have to be. Since this method is only used within Gax, can we change this to package private? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
return protobufRuntimeToken; | ||
} | ||
|
||
private String constructToken(String name, String version) { | ||
if (version == null) { | ||
return null; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ | |
*/ | ||
package com.google.api.gax.rpc; | ||
|
||
import static com.google.api.gax.rpc.ApiClientHeaderProvider.PROTOBUF_HEADER_VERSION_KEY; | ||
import static com.google.common.truth.Truth.assertThat; | ||
|
||
import org.junit.jupiter.api.Test; | ||
|
@@ -38,20 +39,31 @@ class ApiClientHeaderProviderTest { | |
private static final String X_GOOG_API_CLIENT = "x-goog-api-client"; | ||
private static final String CLOUD_RESOURCE_PREFIX = "google-cloud-resource-prefix"; | ||
|
||
private static final String PROTOBUF_HEADER = PROTOBUF_HEADER_VERSION_KEY + "/.*"; | ||
private static final String PROTOBUF_APPENDER = "--" + PROTOBUF_HEADER_VERSION_KEY + "-.*"; | ||
private static final String FULL_HEADER_MATCH = | ||
"^gl-java/.* gapic/4\\.5\\.6" | ||
+ PROTOBUF_APPENDER | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel these static strings add additional complexity in reading the code, can we try to inline most of them per style guide? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
+ " gax/.* grpc/1\\.2\\.3 " | ||
+ PROTOBUF_HEADER | ||
+ "$"; | ||
private static final String HEADER_WITHOUT_GAPIC_MATCH = | ||
"^gl-java/.* gccl/1\\.2\\.3" + PROTOBUF_APPENDER + " gax/.* " + PROTOBUF_HEADER + "$"; | ||
|
||
@Test | ||
void testServiceHeaderDefault() { | ||
ApiClientHeaderProvider provider = ApiClientHeaderProvider.newBuilder().build(); | ||
assertThat(provider.getHeaders().size()).isEqualTo(1); | ||
assertThat(provider.getHeaders().get(X_GOOG_API_CLIENT)).matches("^gl-java/.* gax/.*$"); | ||
assertThat(provider.getHeaders().get(X_GOOG_API_CLIENT)) | ||
.matches("^gl-java/.* gax/.* " + PROTOBUF_HEADER + "$"); | ||
} | ||
|
||
@Test | ||
void testServiceHeaderManual() { | ||
ApiClientHeaderProvider provider = | ||
ApiClientHeaderProvider.newBuilder().setClientLibToken("gccl", "1.2.3").build(); | ||
assertThat(provider.getHeaders().size()).isEqualTo(1); | ||
assertThat(provider.getHeaders().get(X_GOOG_API_CLIENT)) | ||
.matches("^gl-java/.* gccl/1\\.2\\.3 gax/.*$"); | ||
assertThat(provider.getHeaders().get(X_GOOG_API_CLIENT)).matches(HEADER_WITHOUT_GAPIC_MATCH); | ||
} | ||
|
||
@Test | ||
|
@@ -64,7 +76,13 @@ void testServiceHeaderManualGapic() { | |
.build(); | ||
assertThat(provider.getHeaders().size()).isEqualTo(1); | ||
assertThat(provider.getHeaders().get(X_GOOG_API_CLIENT)) | ||
.matches("^gl-java/.* gccl/4\\.5\\.6 gapic/7\\.8\\.9 gax/.* grpc/1\\.2\\.3$"); | ||
.matches( | ||
"^gl-java/.* gccl/4\\.5\\.6" | ||
+ PROTOBUF_APPENDER | ||
+ " gapic/7\\.8\\.9" | ||
+ " gax/.* grpc/1\\.2\\.3 " | ||
+ PROTOBUF_HEADER | ||
+ "$"); | ||
} | ||
|
||
@Test | ||
|
@@ -76,7 +94,12 @@ void testServiceHeaderManualGrpc() { | |
.build(); | ||
assertThat(provider.getHeaders().size()).isEqualTo(1); | ||
assertThat(provider.getHeaders().get(X_GOOG_API_CLIENT)) | ||
.matches("^gl-java/.* gccl/4\\.5\\.6 gax/.* grpc/1\\.2\\.3$"); | ||
.matches( | ||
"^gl-java/.* gccl/4\\.5\\.6" | ||
+ PROTOBUF_APPENDER | ||
+ " gax/.* grpc/1\\.2\\.3 " | ||
+ PROTOBUF_HEADER | ||
+ "$"); | ||
} | ||
|
||
@Test | ||
|
@@ -87,8 +110,7 @@ void testServiceHeaderGapic() { | |
.setGeneratedLibToken("gapic", "4.5.6") | ||
.build(); | ||
assertThat(provider.getHeaders().size()).isEqualTo(1); | ||
assertThat(provider.getHeaders().get(X_GOOG_API_CLIENT)) | ||
.matches("^gl-java/.* gapic/4\\.5\\.6 gax/.* grpc/1\\.2\\.3$"); | ||
assertThat(provider.getHeaders().get(X_GOOG_API_CLIENT)).matches(FULL_HEADER_MATCH); | ||
} | ||
|
||
@Test | ||
|
@@ -100,8 +122,7 @@ void testCloudResourcePrefixHeader() { | |
.setResourceToken("test-prefix") | ||
.build(); | ||
assertThat(provider.getHeaders().size()).isEqualTo(2); | ||
assertThat(provider.getHeaders().get(X_GOOG_API_CLIENT)) | ||
.matches("^gl-java/.* gapic/4\\.5\\.6 gax/.* grpc/1\\.2\\.3$"); | ||
assertThat(provider.getHeaders().get(X_GOOG_API_CLIENT)).matches(FULL_HEADER_MATCH); | ||
assertThat(provider.getHeaders().get(CLOUD_RESOURCE_PREFIX)).isEqualTo("test-prefix"); | ||
} | ||
|
||
|
@@ -116,8 +137,7 @@ void testCustomHeaderKeys() { | |
.setResourceHeaderKey("custom-header2") | ||
.build(); | ||
assertThat(provider.getHeaders().size()).isEqualTo(2); | ||
assertThat(provider.getHeaders().get("custom-header1")) | ||
.matches("^gl-java/.* gapic/4\\.5\\.6 gax/.* grpc/1\\.2\\.3$"); | ||
assertThat(provider.getHeaders().get("custom-header1")).matches(FULL_HEADER_MATCH); | ||
assertThat(provider.getHeaders().get("custom-header2")).isEqualTo("test-prefix"); | ||
} | ||
|
||
|
@@ -130,8 +150,7 @@ void testQuotaProjectHeader() { | |
.setQuotaProjectIdToken(quotaProjectHeaderValue) | ||
.build(); | ||
assertThat(provider.getHeaders().size()).isEqualTo(2); | ||
assertThat(provider.getHeaders().get(X_GOOG_API_CLIENT)) | ||
.matches("^gl-java/.* gccl/1\\.2\\.3 gax/.*$"); | ||
assertThat(provider.getHeaders().get(X_GOOG_API_CLIENT)).matches(HEADER_WITHOUT_GAPIC_MATCH); | ||
assertThat(provider.getHeaders().get(ApiClientHeaderProvider.QUOTA_PROJECT_ID_HEADER_KEY)) | ||
.matches(quotaProjectHeaderValue); | ||
} | ||
|
@@ -149,4 +168,22 @@ void testApiVersionHeader() { | |
assertThat( | ||
emptyProvider.getHeaders().get(ApiClientHeaderProvider.API_VERSION_HEADER_KEY).isEmpty()); | ||
} | ||
|
||
@Test | ||
void testNonGapicGeneratedLibToken_doesNotAppendProtobufVersion() { | ||
ApiClientHeaderProvider provider = | ||
ApiClientHeaderProvider.newBuilder().setGeneratedLibToken("other-token", "1.2.3").build(); | ||
|
||
assertThat(provider.getHeaders().get(X_GOOG_API_CLIENT)) | ||
.matches("^gl-java/.* other-token/1.2.3 gax/.* " + PROTOBUF_HEADER + "$"); | ||
} | ||
|
||
@Test | ||
void testNonGcclGeneratedLibToken_doesNotAppendProtobufVersion() { | ||
ApiClientHeaderProvider provider = | ||
ApiClientHeaderProvider.newBuilder().setClientLibToken("other-token", "1.2.3").build(); | ||
|
||
assertThat(provider.getHeaders().get(X_GOOG_API_CLIENT)) | ||
.matches("^gl-java/.* other-token/1.2.3 gax/.* " + PROTOBUF_HEADER + "$"); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mind adding a comment regarding where do we get this magic string "Bundle-Version"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added it to the getBundleVersion JavaDoc