Skip to content

Commit

Permalink
fix: Apply Google Java Code Clarity suggestions (#1044)
Browse files Browse the repository at this point in the history
* fix: Apply Google Java Code Clarity suggestions

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: Ben Creech <[email protected]>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
  • Loading branch information
3 people committed Sep 7, 2022
1 parent ad38188 commit 79e9d8d
Show file tree
Hide file tree
Showing 49 changed files with 586 additions and 639 deletions.
12 changes: 12 additions & 0 deletions google-cloud-logging/clirr-ignored-differences.xml
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<differences>
<difference>
<differenceType>3003</differenceType>
<className>com/google/cloud/logging/Instrumentation</className>
</difference>
<difference>
<differenceType>7009</differenceType>
<className>com/google/cloud/logging/Instrumentation</className>
<method>Instrumentation()</method>
</difference>
</differences>
5 changes: 5 additions & 0 deletions google-cloud-logging/pom.xml
Expand Up @@ -127,6 +127,11 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava-testlib</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>com.google.api.grpc</groupId>
Expand Down
Expand Up @@ -17,8 +17,12 @@
package com.google.cloud.logging;

import com.google.cloud.logging.HttpRequest.RequestMethod;
import com.google.common.base.Ascii;
import com.google.common.base.MoreObjects;
import com.google.common.base.Splitter;
import com.google.common.collect.Iterables;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.List;
import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -125,7 +129,7 @@ public Builder setSpanId(String spanId) {
@CanIgnoreReturnValue
public Builder loadCloudTraceContext(String cloudTrace) {
if (cloudTrace != null) {
cloudTrace = cloudTrace.split(";")[0];
cloudTrace = Iterables.get(Splitter.on(';').split(cloudTrace), 0);
int split = cloudTrace.indexOf('/');
if (split >= 0) {
String traceId = cloudTrace.substring(0, split);
Expand Down Expand Up @@ -157,16 +161,16 @@ public Builder loadCloudTraceContext(String cloudTrace) {
* the format version is not supported.
*/
@CanIgnoreReturnValue
public Builder loadW3CTraceParentContext(String traceParent) throws IllegalArgumentException {
public Builder loadW3CTraceParentContext(String traceParent) {
if (traceParent != null) {
Matcher validator = W3C_TRACE_CONTEXT_FORMAT.matcher(traceParent.toLowerCase());
Matcher validator = W3C_TRACE_CONTEXT_FORMAT.matcher(Ascii.toLowerCase(traceParent));
if (!validator.matches()) {
throw new IllegalArgumentException(
"Invalid format of the header value. The value does not match W3C Trace Context version \"00\"");
}
String[] fields = traceParent.split("-");
setTraceId(fields[1]);
setSpanId(fields[2]);
List<String> fields = Splitter.on('-').splitToList(traceParent);
setTraceId(fields.get(1));
setSpanId(fields.get(2));
// fields[3] contains flag(s)
}
return this;
Expand Down
Expand Up @@ -23,7 +23,6 @@
import com.google.logging.v2.LogExclusion;
import com.google.protobuf.Timestamp;
import java.util.Objects;
import org.jspecify.nullness.Nullable;

/**
* Specifies a set of log entries that are not to be stored in Logging. If your GCP resource
Expand All @@ -35,27 +34,21 @@
public class Exclusion {

static final Function<LogExclusion, Exclusion> FROM_PROTOBUF_FUNCTION =
new Function<LogExclusion, Exclusion>() {
@Override
public @Nullable Exclusion apply(LogExclusion exclusionPb) {
return exclusionPb != null ? Exclusion.fromProtobuf(exclusionPb) : null;
}
(LogExclusion exclusionPb) -> {
return exclusionPb != null ? Exclusion.fromProtobuf(exclusionPb) : null;
};

static final Function<Exclusion, LogExclusion> TO_PROTOBUF_FUNCTION =
new Function<Exclusion, LogExclusion>() {
@Override
public @Nullable LogExclusion apply(Exclusion exclusion) {
return exclusion != null ? exclusion.toProtobuf() : null;
}
(Exclusion exclusion) -> {
return exclusion != null ? exclusion.toProtobuf() : null;
};

private String name;
private String description;
private String filter;
private boolean disabled;
private Timestamp createTime;
private Timestamp updateTime;
private final String name;
private final String description;
private final String filter;
private final boolean disabled;
private final Timestamp createTime;
private final Timestamp updateTime;

/** A builder for {@code Exclusion} objects. */
public static class Builder {
Expand Down Expand Up @@ -165,7 +158,7 @@ public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
if (!(o instanceof Exclusion)) {
return false;
}
Exclusion exclusion = (Exclusion) o;
Expand Down Expand Up @@ -253,7 +246,7 @@ LogExclusion toProtobuf() {
static Exclusion fromProtobuf(LogExclusion exclusionPb) {
Exclusion.Builder builder = newBuilder(exclusionPb.getName(), exclusionPb.getFilter());
builder.setDisabled(exclusionPb.getDisabled());
if (!exclusionPb.getDescription().equals("")) {
if (!exclusionPb.getDescription().isEmpty()) {
builder.setDescription(exclusionPb.getDescription());
}
if (exclusionPb.hasCreateTime()) {
Expand Down
Expand Up @@ -20,7 +20,6 @@
import com.google.cloud.StringEnumType;
import com.google.cloud.StringEnumValue;
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.io.Serializable;
import java.util.Objects;
Expand Down Expand Up @@ -71,7 +70,7 @@ public RequestMethod apply(String constant) {
};

private static final StringEnumType<RequestMethod> type =
new StringEnumType<RequestMethod>(RequestMethod.class, CONSTRUCTOR);
new StringEnumType<>(RequestMethod.class, CONSTRUCTOR);

public static final RequestMethod GET = type.createAndRegister("GET");
public static final RequestMethod HEAD = type.createAndRegister("HEAD");
Expand Down Expand Up @@ -527,10 +526,10 @@ public static Builder newBuilder() {

static HttpRequest fromPb(com.google.logging.type.HttpRequest requestPb) {
Builder builder = newBuilder();
if (!Strings.isNullOrEmpty(requestPb.getRequestMethod())) {
if (!requestPb.getRequestMethod().isEmpty()) {
builder.setRequestMethod(RequestMethod.valueOf(requestPb.getRequestMethod()));
}
if (!Strings.isNullOrEmpty(requestPb.getRequestUrl())) {
if (!requestPb.getRequestUrl().isEmpty()) {
builder.setRequestUrl(requestPb.getRequestUrl());
}
if (requestPb.getRequestSize() != 0L) {
Expand All @@ -542,16 +541,16 @@ static HttpRequest fromPb(com.google.logging.type.HttpRequest requestPb) {
if (requestPb.getResponseSize() != 0L) {
builder.setResponseSize(requestPb.getResponseSize());
}
if (!Strings.isNullOrEmpty(requestPb.getUserAgent())) {
if (!requestPb.getUserAgent().isEmpty()) {
builder.setUserAgent(requestPb.getUserAgent());
}
if (!Strings.isNullOrEmpty(requestPb.getServerIp())) {
if (!requestPb.getServerIp().isEmpty()) {
builder.setServerIp(requestPb.getServerIp());
}
if (!Strings.isNullOrEmpty(requestPb.getRemoteIp())) {
if (!requestPb.getRemoteIp().isEmpty()) {
builder.setRemoteIp(requestPb.getRemoteIp());
}
if (!Strings.isNullOrEmpty(requestPb.getReferer())) {
if (!requestPb.getReferer().isEmpty()) {
builder.setReferer(requestPb.getReferer());
}
builder.setCacheLookup(requestPb.getCacheLookup());
Expand Down
Expand Up @@ -21,18 +21,17 @@
import com.google.cloud.Tuple;
import com.google.cloud.logging.Logging.WriteOption;
import com.google.cloud.logging.Payload.JsonPayload;
import com.google.cloud.logging.Payload.Type;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.protobuf.ListValue;
import com.google.protobuf.Struct;
import com.google.protobuf.Value;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import org.jspecify.nullness.Nullable;

public class Instrumentation {
public final class Instrumentation {
public static final String DIAGNOSTIC_INFO_KEY = "logging.googleapis.com/diagnostic";
public static final String INSTRUMENTATION_SOURCE_KEY = "instrumentation_source";
public static final String INSTRUMENTATION_NAME_KEY = "name";
Expand All @@ -43,14 +42,14 @@ public class Instrumentation {
public static final int MAX_DIAGNOSTIC_VALUE_LENGTH = 14;
public static final int MAX_DIAGNOSTIC_ENTIES = 3;
private static boolean instrumentationAdded = false;
private static Object instrumentationLock = new Object();
private static final Object instrumentationLock = new Object();

/**
* Populates entries with instrumentation info which is added in separate log entry
*
* @param logEntries {Iterable<LogEntry>} The list of entries to be populated
* @return {Tuple<Boolean, Iterable<LogEntry>>} containing a flag if instrumentation info was
* added or not and a modified list of log entries
* @param logEntries {@code Iterable<LogEntry>} The list of entries to be populated
* @return {@code Tuple<Boolean, Iterable<LogEntry>>} containing a flag if instrumentation info
* was added or not and a modified list of log entries
*/
public static Tuple<Boolean, Iterable<LogEntry>> populateInstrumentationInfo(
Iterable<LogEntry> logEntries) {
Expand All @@ -61,7 +60,7 @@ public static Tuple<Boolean, Iterable<LogEntry>> populateInstrumentationInfo(
for (LogEntry logEntry : logEntries) {
// Check if LogEntry has a proper payload and also contains a diagnostic entry
if (!isWritten
&& logEntry.getPayload().getType() == Type.JSON
&& logEntry.getPayload().getType() == Payload.Type.JSON
&& logEntry
.<Payload.JsonPayload>getPayload()
.getData()
Expand All @@ -77,7 +76,7 @@ public static Tuple<Boolean, Iterable<LogEntry>> populateInstrumentationInfo(
.getListValue();
entries.add(createDiagnosticEntry(null, null, infoList));
isWritten = true;
} catch (Exception ex) {
} catch (RuntimeException ex) {
System.err.println("ERROR: unexpected exception in populateInstrumentationInfo: " + ex);
}
} else {
Expand All @@ -99,8 +98,8 @@ public static Tuple<Boolean, Iterable<LogEntry>> populateInstrumentationInfo(
*/
public static WriteOption @Nullable [] addPartialSuccessOption(WriteOption[] options) {
if (options == null) return options;
List<WriteOption> writeOptions = new ArrayList<WriteOption>();
writeOptions.addAll(Arrays.asList(options));
List<WriteOption> writeOptions = new ArrayList<>();
Collections.addAll(writeOptions, options);
// Make sure we remove all partial success flags if any exist
writeOptions.removeIf(
option -> option.getOptionType() == WriteOption.OptionType.PARTIAL_SUCCESS);
Expand Down Expand Up @@ -133,18 +132,16 @@ private static LogEntry createDiagnosticEntry(
generateLibrariesList(libraryName, libraryVersion, existingLibraryList))
.build()))
.build();
LogEntry entry =
LogEntry.newBuilder(
JsonPayload.of(
Struct.newBuilder()
.putAllFields(
ImmutableMap.of(
DIAGNOSTIC_INFO_KEY,
Value.newBuilder().setStructValue(instrumentation).build()))
.build()))
.setLogName(INSTRUMENTATION_LOG_NAME)
.build();
return entry;
return LogEntry.newBuilder(
JsonPayload.of(
Struct.newBuilder()
.putAllFields(
ImmutableMap.of(
DIAGNOSTIC_INFO_KEY,
Value.newBuilder().setStructValue(instrumentation).build()))
.build()))
.setLogName(INSTRUMENTATION_LOG_NAME)
.build();
}

private static ListValue generateLibrariesList(
Expand All @@ -171,7 +168,7 @@ private static ListValue generateLibrariesList(
libraryList.addValues(
Value.newBuilder().setStructValue(createInfoStruct(name, version)).build());
if (libraryList.getValuesCount() == MAX_DIAGNOSTIC_ENTIES) break;
} catch (Exception ex) {
} catch (RuntimeException ex) {
}
}
}
Expand Down Expand Up @@ -222,4 +219,6 @@ private static String truncateValue(String value) {
if (Strings.isNullOrEmpty(value) || value.length() < MAX_DIAGNOSTIC_VALUE_LENGTH) return value;
return value.substring(0, MAX_DIAGNOSTIC_VALUE_LENGTH) + "*";
}

private Instrumentation() {}
}

0 comments on commit 79e9d8d

Please sign in to comment.