-
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 13 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,24 @@ 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, otherwise | ||
burkedavison marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* 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 Protobuf runtime version. Recover gracefully. | ||
burkedavison marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return Optional.empty(); | ||
} | ||
} | ||
} |
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