-
Notifications
You must be signed in to change notification settings - Fork 54
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
PoC: Otel metrics #1807
PoC: Otel metrics #1807
Conversation
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use -- conventional-commit-lint bot |
@@ -101,6 +101,11 @@ public interface ApiTracer { | |||
/** Adds an annotation that the attempt succeeded. */ | |||
void attemptSucceeded(); | |||
|
|||
//This is for libraries to override to intended name | |||
default String attemptLatencyName() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're going to provide a default implementation, I wonder instead of override the attempt name, maybe we can provide a general class, something like ApiTracerRecorder
and provide functions like apiTracerInstrument.record(latency, attributes). So client libraries can define their own meter definition. Similar to https://github.com/googleapis/java-bigtable/pull/1796/files#diff-f2ab39ff606949b18fa0ea0a25b6149352064c8b1f775a78cb79eb8acfddde6f BigtableInsturment
and BuiltinInstruments
(I'm going to rename them to BgitableRecorder and BuiltinRecorder)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see the need of another abstraction layer for customizing some attributes(like extract this method to something like MetricNames
), however, I think most of things, like description
and unit
should be the same for all services, so that we can have a consistent experience for all services. Let me know if I'm missing something.
* <p>This class is thread safe. | ||
*/ | ||
@InternalApi("For google-cloud-java client use only") | ||
public class OpenTelemetryTracerFactory extends BaseApiTracerFactory { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, we can probably clean up BaseApiTracer and BaseApiTracerFactory, it was created in java7 days for backward compatibility: b810cc9
No description provided.