Skip to content
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

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
31ebd95
track protobuf version
ldetmer Sep 11, 2024
e6aea43
add support for gccl
ldetmer Sep 13, 2024
a64e14b
formatting fixes
ldetmer Sep 13, 2024
3a7b27d
Merge branch 'main' into protobuf-metric
ldetmer Sep 13, 2024
0037a0a
formatting fixes
ldetmer Sep 13, 2024
ea7e7ec
feat(gax): add protobuf version to headers to track
ldetmer Sep 13, 2024
4e18aa6
feat(gax): add protobuf version to headers to track
ldetmer Sep 13, 2024
b1a8960
Merge branch 'main' into protobuf-metric
ldetmer Sep 13, 2024
8d6065c
feat(gax): add protobuf version to headers to track
ldetmer Sep 14, 2024
2dbc3a4
feat(gax): add protobuf version to headers to track
ldetmer Sep 14, 2024
b69684b
feat(gax): add protobuf version to headers to track
ldetmer Sep 14, 2024
02051b3
feat(gax): add protobuf version to headers to track
ldetmer Sep 15, 2024
31da0e5
feat(gax): add protobuf version to headers to track
ldetmer Sep 16, 2024
5a918f4
fixed todo wording
ldetmer Sep 16, 2024
94d3f45
cleaned up append token logic
ldetmer Sep 16, 2024
822ac0c
cleaned up append token logic
ldetmer Sep 16, 2024
390c922
cleaned up append token logic
ldetmer Sep 16, 2024
01eb41a
fixed language
ldetmer Sep 16, 2024
92e901b
added showcase tests + moved strings inline
ldetmer Sep 18, 2024
a202b8b
formatting fix
ldetmer Sep 18, 2024
f9fac2d
Merge branch 'main' into protobuf-metric
ldetmer Sep 18, 2024
9849efd
formatting fix
ldetmer Sep 18, 2024
604f00d
print header to debug native image
ldetmer Sep 18, 2024
9c51ec7
print header to debug native image
ldetmer Sep 18, 2024
a99c4b1
fixed native showcase test
ldetmer Sep 18, 2024
c7b3d2b
fixed formatting
ldetmer Sep 18, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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";
Copy link
Collaborator

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"?

Copy link
Contributor Author

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


private GaxProperties() {}

Expand Down Expand Up @@ -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)
Expand All @@ -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));
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand Up @@ -41,13 +41,18 @@
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 static String tokensToAppendProfobufVersionTo = "";

private final Map<String, String> headers;

protected ApiClientHeaderProvider(Builder builder) {
ImmutableMap.Builder<String, String> headersBuilder = ImmutableMap.builder();
tokensToAppendProfobufVersionTo = "(gccl|gapic).*";
burkedavison marked this conversation as resolved.
Show resolved Hide resolved

if (builder.getApiClientHeaderKey() != null) {
StringBuilder apiClientHeaderValue = new StringBuilder();
Expand All @@ -57,6 +62,7 @@ protected ApiClientHeaderProvider(Builder builder) {
appendToken(apiClientHeaderValue, builder.getGeneratedLibToken());
appendToken(apiClientHeaderValue, builder.getGeneratedRuntimeToken());
appendToken(apiClientHeaderValue, builder.getTransportToken());
appendToken(apiClientHeaderValue, builder.protobufRuntimeToken);
burkedavison marked this conversation as resolved.
Show resolved Hide resolved
if (apiClientHeaderValue.length() > 0) {
headersBuilder.put(builder.getApiClientHeaderKey(), apiClientHeaderValue.toString());
}
Expand Down Expand Up @@ -87,6 +93,18 @@ protected static void appendToken(StringBuilder sb, String token) {
sb.append(' ');
}
sb.append(token);
checkAndAppendProtobufVersionIfNecessary(sb, token);
}
}

private static void checkAndAppendProtobufVersionIfNecessary(StringBuilder sb, String token) {
// TODO(b:/366417603): appending protobuf version to existing client library column is a
// temporary fix while waiting for dedicated field to be added in concord
if (token.matches(tokensToAppendProfobufVersionTo)) {
sb.append(protobufVersionAppendValue);
// once protobuf version as been appended to a token do not need to append to any additional
// tokens
tokensToAppendProfobufVersionTo = "";
}
}

Expand All @@ -110,6 +128,7 @@ public static class Builder {
private String generatedRuntimeToken;
private String transportToken;
private String quotaProjectIdToken;
private final String protobufRuntimeToken;

private String resourceHeaderKey;
private String resourceToken;
Expand All @@ -125,11 +144,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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@
*/
package com.google.api.gax.core;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static com.google.api.gax.core.GaxProperties.getBundleVersion;
import static org.junit.jupiter.api.Assertions.*;

import com.google.common.base.Strings;
import java.io.IOException;
import java.util.Optional;
import java.util.regex.Pattern;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
Expand All @@ -41,17 +43,11 @@ class GaxPropertiesTest {

@Test
void testGaxVersion() {
String gaxVersion = GaxProperties.getGaxVersion();
assertTrue(Pattern.compile("^\\d+\\.\\d+\\.\\d+").matcher(gaxVersion).find());
String[] versionComponents = gaxVersion.split("\\.");
// This test was added in version 1.56.0, so check that the major and minor numbers are greater
// than that.
int major = Integer.parseInt(versionComponents[0]);
int minor = Integer.parseInt(versionComponents[1]);
Version version = readVersion(GaxProperties.getGaxVersion());

assertTrue(major >= 1);
if (major == 1) {
assertTrue(minor >= 56);
assertTrue(version.major >= 1);
if (version.major == 1) {
assertTrue(version.minor >= 56);
}
}

Expand Down Expand Up @@ -159,4 +155,41 @@ void testGetJavaRuntimeInfo_nullJavaVersion() {
String runtimeInfo = GaxProperties.getRuntimeVersion();
assertEquals("null__oracle__20.0.1", runtimeInfo);
}

@Test
public void testGetProtobufVersion() throws IOException {
Version version = readVersion(GaxProperties.getProtobufVersion());

assertTrue(version.major >= 3);
if (version.major == 3) {
assertTrue(version.minor >= 25);
}
}

@Test
public void testGetBundleVersion_noManifestFile() throws IOException {
Optional<String> version = getBundleVersion(GaxProperties.class);

assertFalse(version.isPresent());
}

private Version readVersion(String version) {
assertTrue(Pattern.compile("^\\d+\\.\\d+\\.\\d+").matcher(version).find());
String[] versionComponents = version.split("\\.");
// This test was added in version 1.56.0, so check that the major and minor numbers are greater
// than that.
int major = Integer.parseInt(versionComponents[0]);
int minor = Integer.parseInt(versionComponents[1]);
return new Version(major, minor);
}

private static class Version {
public int major;
public int minor;

public Version(int major, int minor) {
this.major = major;
this.minor = minor;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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");
}

Expand All @@ -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");
}

Expand All @@ -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);
}
Expand All @@ -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 + "$");
}
}
Loading