chore: port changes in google-cloud-java since 2026-03-24#4170
chore: port changes in google-cloud-java since 2026-03-24#4170diegomarquezp wants to merge 10 commits intomainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request enhances observability and resource name extraction by introducing heuristic-based canonical name extraction and a composite tracing architecture with a new LoggingTracer for actionable error reporting. It also enables trace context propagation and response size recording across gRPC and HTTP/JSON transports. Review feedback identifies several performance optimization opportunities, such as pre-compiling regular expressions and using Set for lookups, alongside correctness improvements like recursive exception unwrapping and defensive null checks.
| } else if (seg.kind() == SegmentKind.LITERAL) { | ||
| String value = seg.value(); | ||
| // Skipping version literals such as v1, v1beta1 | ||
| if (value.matches("^v\\d+[a-zA-Z0-9]*$")) { |
There was a problem hiding this comment.
The matches method compiles the regular expression every time it is called. Since this is inside a loop, it can be inefficient. It is recommended to use a pre-compiled static final Pattern for better performance.
| Segment seg = segments.get(i); | ||
| if (seg.kind() == SegmentKind.LITERAL) { | ||
| String value = seg.value(); | ||
| if (value.matches("^v\\d+[a-zA-Z0-9]*$")) { |
There was a problem hiding this comment.
The matches method compiles the regular expression every time it is called. It is recommended to use a pre-compiled static final Pattern for better performance.
| private static final List<String> AIP_STANDARDS_METHODS = | ||
| ImmutableList.of( | ||
| "Get", "List", "Create", "Delete", "Update", "Patch", "Insert", "AggregatedList"); |
There was a problem hiding this comment.
Using a Set instead of a List for AIP_STANDARDS_METHODS would allow for more efficient lookups. Additionally, storing the strings in lowercase would simplify case-insensitive matching in isAipStandardMethod.
References
- Prefer ImmutableSet for constant sets of strings. (link)
- Use appropriate data structures for efficient lookups.
| if (!Pattern.compile("\\{" + httpBinding.name() + "(?:=.*?)?\\}") | ||
| .matcher(canonicalPath) | ||
| .find()) { | ||
| continue; |
There was a problem hiding this comment.
Compiling a Pattern inside a loop is inefficient. Use a simple string containment check for {name} and {name= to avoid regex overhead.
References
- Avoid expensive operations like regex compilation inside loops.
| private static boolean isAipStandardMethod(List<String> standards, String methodName) { | ||
| return standards.stream().anyMatch(standard -> standard.equalsIgnoreCase(methodName)); | ||
| } |
There was a problem hiding this comment.
This method can be optimized to use a Set for O(1) lookup instead of streaming over a List.
References
- Use appropriate data structures for efficient lookups.
| if (t.getCause() == null || !(t instanceof UncheckedExecutionException)) { | ||
| return t; | ||
| } | ||
| return t.getCause(); | ||
| } | ||
|
|
There was a problem hiding this comment.
The getRealCause method only unwraps a single level of UncheckedExecutionException. It is recommended to unwrap recursively using a loop to reach the actual root cause.
References
- Ensure root causes are correctly identified when unwrapping exceptions.
| static String sanitizeUrlFull(final String url) { | ||
| try { | ||
| java.net.URI uri = new java.net.URI(url); | ||
| String sanitizedUserInfo = | ||
| uri.getRawUserInfo() != null ? REDACTED_VALUE + ":" + REDACTED_VALUE : null; | ||
| String sanitizedQuery = redactSensitiveQueryValues(uri.getRawQuery()); | ||
| java.net.URI sanitizedUri = | ||
| new java.net.URI( | ||
| uri.getScheme(), | ||
| sanitizedUserInfo, | ||
| uri.getHost(), | ||
| uri.getPort(), | ||
| uri.getRawPath(), | ||
| sanitizedQuery, | ||
| uri.getRawFragment()); | ||
| return sanitizedUri.toString(); | ||
| } catch (java.net.URISyntaxException | IllegalArgumentException ex) { | ||
| return ""; | ||
| } | ||
| } |
There was a problem hiding this comment.
The sanitizeUrlFull method should handle null input for url to avoid a NullPointerException. Add a defensive null check.
References
- Add defensive null checks for fields that are initialized outside the constructor to improve robustness against incorrect usage.
|
|
This PR contains the changes backported from the
google-cloud-javamonorepo (sdk-platform-javadirectory) to this standalone repository.Included Commits Range:
Manual Resolutions & Rejected Changes:
gapic-generator-java,gax,api-common, andgoogle-cloud-shared-dependencieshave been kept at (or reverted to) the original snapshot versions existing in this repo before the patch (e.g.,2.68.1-SNAPSHOTfor gapic-generator-java).sdk-platform-java/prefix inmvncommands.COPY --from=ggj-buildto use correct path inold-sdk.