From 7f6e470fea1673a5cf50fe3b49263615a172afde Mon Sep 17 00:00:00 2001 From: ldetmer <1771267+ldetmer@users.noreply.github.com> Date: Thu, 7 Nov 2024 17:24:35 -0500 Subject: [PATCH] fix: protobuf version not always getting set in headers (#3322) In order to maximize amount of times protobuf version is logged we have updated logic as follows: 1) First try to use protbuf RuntimeVersion which is available in protobuf 4.x 2) if not available try to read using manifest file 3) if manifest file doesn't not exist then default to "3" as we know runtime < 4 Also updated showcase test to fail in the case protobuf version is missing Tested: 1) maven build using client library directly and overriding protobuf 2) spring boot started project + google cloud storage client client lib, validated that protobuf header was sent --- .../google/api/gax/core/GaxProperties.java | 27 ++++++++++- .../api/gax/rpc/ApiClientHeaderProvider.java | 2 +- .../com.google.api/gax/reflect-config.json | 10 ++++ .../api/gax/core/GaxPropertiesTest.java | 46 ++++++++++++++++--- .../showcase/v1beta1/it/ITVersionHeaders.java | 4 +- 5 files changed, 79 insertions(+), 10 deletions(-) create mode 100644 gax-java/gax/src/main/resources/META-INF/native-image/com.google.api/gax/reflect-config.json diff --git a/gax-java/gax/src/main/java/com/google/api/gax/core/GaxProperties.java b/gax-java/gax/src/main/java/com/google/api/gax/core/GaxProperties.java index f15046afcb..994ba2eb82 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/core/GaxProperties.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/core/GaxProperties.java @@ -49,7 +49,7 @@ public class GaxProperties { 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); + getProtobufVersion(Any.class, "com.google.protobuf.RuntimeVersion");; private GaxProperties() {} @@ -148,4 +148,29 @@ static Optional getBundleVersion(Class clazz) { return Optional.empty(); } } + + /** + * Returns the Protobuf runtime version as reported by com.google.protobuf.RuntimeVersion, if + * class is available, otherwise by reading from MANIFEST file. If niether option is available + * defaults to protobuf version 3 as RuntimeVersion class is available in protobuf version 4+ + */ + @VisibleForTesting + static String getProtobufVersion(Class clazz, String protobufRuntimeVersionClassName) { + try { + Class protobufRuntimeVersionClass = Class.forName(protobufRuntimeVersionClassName); + return protobufRuntimeVersionClass.getField("MAJOR").get(null) + + "." + + protobufRuntimeVersionClass.getField("MINOR").get(null) + + "." + + protobufRuntimeVersionClass.getField("PATCH").get(null); + } catch (ClassNotFoundException + | NoSuchFieldException + | IllegalAccessException + | SecurityException + | NullPointerException e) { + // If manifest file is not available default to protobuf generic version 3 as we know + // RuntimeVersion class is available in protobuf jar 4+. + return getBundleVersion(clazz).orElse("3"); + } + } } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java index 35307764d2..52f923e111 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java @@ -88,7 +88,7 @@ private static String checkAndAppendProtobufVersionIfNecessary( // 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()) { + if (matcher.find() && GaxProperties.getProtobufVersion() != null) { return apiClientHeaderValue.substring(0, matcher.end()) + "--" + PROTOBUF_HEADER_VERSION_KEY diff --git a/gax-java/gax/src/main/resources/META-INF/native-image/com.google.api/gax/reflect-config.json b/gax-java/gax/src/main/resources/META-INF/native-image/com.google.api/gax/reflect-config.json new file mode 100644 index 0000000000..3a38d53361 --- /dev/null +++ b/gax-java/gax/src/main/resources/META-INF/native-image/com.google.api/gax/reflect-config.json @@ -0,0 +1,10 @@ +[ + { + "name": "com.google.protobuf.RuntimeVersion", + "fields" : [ + { "name" : "MAJOR" }, + { "name" : "MINOR" }, + { "name" : "PATCH" } + ] + } +] \ No newline at end of file diff --git a/gax-java/gax/src/test/java/com/google/api/gax/core/GaxPropertiesTest.java b/gax-java/gax/src/test/java/com/google/api/gax/core/GaxPropertiesTest.java index 1369ec35ae..6560df4bc1 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/core/GaxPropertiesTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/core/GaxPropertiesTest.java @@ -35,6 +35,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.common.base.Strings; +import com.google.protobuf.Any; import java.io.IOException; import java.util.Optional; import java.util.regex.Pattern; @@ -160,12 +161,8 @@ void testGetJavaRuntimeInfo_nullJavaVersion() { @Test public void testGetProtobufVersion() throws IOException { - Version version = readVersion(GaxProperties.getProtobufVersion()); - - assertTrue(version.major >= 3); - if (version.major == 3) { - assertTrue(version.minor >= 25); - } + assertTrue( + Pattern.compile("^\\d+\\.\\d+\\.\\d+").matcher(GaxProperties.getProtobufVersion()).find()); } @Test @@ -175,6 +172,36 @@ public void testGetBundleVersion_noManifestFile() throws IOException { assertFalse(version.isPresent()); } + @Test + void testGetProtobufVersion_success() { + String version = + GaxProperties.getProtobufVersion( + Any.class, "com.google.api.gax.core.GaxPropertiesTest$RuntimeVersion"); + + assertEquals("3.13.6", version); + } + + @Test + void testGetProtobufVersion_classNotFoundException() throws Exception { + String version = GaxProperties.getProtobufVersion(Any.class, "foo.NonExistantClass"); + + assertTrue(Pattern.compile("^\\d+\\.\\d+\\.\\d+").matcher(version).find()); + } + + @Test + void testgetProtobufVersion_noSuchFieldException() throws Exception { + String version = GaxProperties.getProtobufVersion(Any.class, "java.lang.Class"); + + assertTrue(Pattern.compile("^\\d+\\.\\d+\\.\\d+").matcher(version).find()); + } + + @Test + void testGetProtobufVersion_noManifest() throws Exception { + String version = GaxProperties.getProtobufVersion(GaxProperties.class, "foo.NonExistantClass"); + + assertEquals("3", version); + } + private Version readVersion(String version) { assertTrue(Pattern.compile("^\\d+\\.\\d+\\.\\d+").matcher(version).find()); String[] versionComponents = version.split("\\."); @@ -194,4 +221,11 @@ public Version(int major, int minor) { this.minor = minor; } } + + // Test class that emulates com.google.protobuf.RuntimeVersion for reflection lookup of fields + class RuntimeVersion { + public static final int MAJOR = 3; + public static final int MINOR = 13; + public static final int PATCH = 6; + } } diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITVersionHeaders.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITVersionHeaders.java index 09255fe278..303cab98ec 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITVersionHeaders.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITVersionHeaders.java @@ -241,7 +241,7 @@ void testHttpJsonCompliance_userApiVersionSetSuccess() throws IOException { @Test void testGrpcCall_sendsCorrectApiClientHeader() { Pattern defautlGrpcHeaderPattern = - Pattern.compile("gl-java/.* gapic/.*?--protobuf-.* gax/.* grpc/.* protobuf/.*"); + Pattern.compile("gl-java/.* gapic/.*?--protobuf-\\d.* gax/.* grpc/.* protobuf/\\d.*"); grpcClient.echo(EchoRequest.newBuilder().build()); String headerValue = grpcInterceptor.metadata.get(API_CLIENT_HEADER_KEY); assertTrue(defautlGrpcHeaderPattern.matcher(headerValue).matches()); @@ -250,7 +250,7 @@ void testGrpcCall_sendsCorrectApiClientHeader() { @Test void testHttpJson_sendsCorrectApiClientHeader() { Pattern defautlHttpHeaderPattern = - Pattern.compile("gl-java/.* gapic/.*?--protobuf-.* gax/.* rest/ protobuf/.*"); + Pattern.compile("gl-java/.* gapic/.*?--protobuf-\\d.* gax/.* rest/ protobuf/\\d.*"); httpJsonClient.echo(EchoRequest.newBuilder().build()); ArrayList headerValues = (ArrayList)