Skip to content

Commit

Permalink
fix: protobuf version not always getting set in headers (#3322)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ldetmer authored Nov 7, 2024
1 parent a1db8b7 commit 7f6e470
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}

Expand Down Expand Up @@ -148,4 +148,29 @@ static Optional<String> 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");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[
{
"name": "com.google.protobuf.RuntimeVersion",
"fields" : [
{ "name" : "MAJOR" },
{ "name" : "MINOR" },
{ "name" : "PATCH" }
]
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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("\\.");
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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<String> headerValues =
(ArrayList<String>)
Expand Down

0 comments on commit 7f6e470

Please sign in to comment.