feat(obs): introduce gpc.client.repo span attribute#4111
feat(obs): introduce gpc.client.repo span attribute#4111diegomarquezp wants to merge 61 commits intomainfrom
Conversation
diegomarquezp
commented
Feb 12, 2026
diegomarquezp
commented
- feat(obs): generate gapic.properties file with repo property
- feat: implement repo handling in gax
- test: add tests for repo property
- chore: speed up image building
- build: introduce repo property to generation config
- chore: update showcase module
This reverts commit b7b9e31.
- Rename SpanHandle to GaxSpan - Simplify adding op and attempt attributes - Use HashMap instead of ConcurrentHashMap - Remove error handling (for now)
…icTracer.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…om/googleapis/sdk-platform-java into observability/initial-tracing-impl
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new gpc.client.repo span attribute for observability, propagating repository information from build configuration to tracing spans. However, a significant logic error exists where telemetry keys are swapped, leading to data corruption in audit logs. Furthermore, vulnerabilities related to insecure data handling (property injection and option injection) have been identified in the code processing repository information. There is also a high-severity bug in a build script and a medium-severity opportunity for code simplification using modern Java features.
gax-java/gax/src/main/java/com/google/api/gax/tracing/AppCentricTracer.java
Outdated
Show resolved
Hide resolved
gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java
Outdated
Show resolved
Hide resolved
| static Optional<String> parseRepo(CodeGeneratorRequest request) { | ||
| return parseConfigArgument(request.getParameter(), KEY_REPO); | ||
| } |
There was a problem hiding this comment.
The parseRepo method uses parseConfigArgument, which parses plugin arguments by splitting the input string by commas. If the repo value contains a comma, it will be split, potentially allowing an attacker to inject additional plugin flags or options (e.g., injecting ,metadata to enable metadata generation).
There was a problem hiding this comment.
I don't think that's how the argument parser will work. If repo contains a comma then what's after the comma will be ignored and treated as a sperate flag not handled by the generator.
gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java
Outdated
Show resolved
Hide resolved
…googleapis/sdk-platform-java into observability/tracing-attr/repo
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new span attribute gpc.client.repo to include repository information in traces. This is achieved by adding a --repo flag to the generator, which is then passed through the build process and used to generate a service-specific ApiTracerContext class. The GAX library is updated to use this new context. The changes are well-implemented and include necessary tests and updates to golden files. I've found one minor issue with a type hint in a Python script, which I've commented on.
|
|
2 participants
