chore(o11y): remove beta api from observability classes#12747
chore(o11y): remove beta api from observability classes#12747
Conversation
…ervability classes and make factory mutable
1038937 to
5a29bdb
Compare
There was a problem hiding this comment.
Code Review
This pull request promotes OpenTelemetryMetricsFactory to the public API and refactors its unit tests to use concrete objects instead of mocks. The review identifies a potential thread-safety issue with the clientLevelTracerContext field and suggests caution when removing API stability annotations. Additionally, the reviewer noted that the test refactoring removed a verification step for context merging, which should be restored to maintain adequate test coverage.
I am having trouble creating individual review comments. Click here to see my feedback.
sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java (41-42)
Removing @InternalApi and @BetaApi promotes this class to the public stable API. Avoid introducing breaking changes to public APIs, even if they have not been part of a public release. Additionally, the clientLevelTracerContext field is not volatile, which can lead to thread-safety issues if the factory is shared across threads. Please ensure the field is volatile or that access is properly synchronized.
References
- Avoid introducing breaking changes to public APIs, even if they have not been part of a public release.
sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactoryTest.java (82)
The test coverage has been weakened by removing the verify call. The current assertion only checks that the returned object is an instance of OpenTelemetryMetricsTracer, but it no longer verifies that the clientLevelTracerContext and methodLevelTracerContext are correctly merged. Please restore the verification logic to ensure the merging logic is correctly exercised.
There was a problem hiding this comment.
qq: should the recorder also be marked as non-beta?
This PR remove
@BetaApifrom observability classes.@InternalApiannotations are also removed because they are either package private or need to be used externally.