[go: nahoru, domu]

Skip to content
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

Closed
wants to merge 18 commits into from
Closed

PoC: Otel metrics #1807

wants to merge 18 commits into from

Conversation

blakeli0
Copy link
Collaborator

No description provided.

@conventional-commit-lint-gcf
Copy link
conventional-commit-lint-gcf bot commented Jun 26, 2023

🤖 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 automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Jun 26, 2023
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Jun 27, 2023
@blakeli0 blakeli0 changed the title POC: OTEL with attempt latency. POC: OTEL Jun 27, 2023
@@ -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() {
Copy link
Contributor
@mutianf mutianf Jun 27, 2023

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)

Copy link
Collaborator Author

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 {
Copy link
Contributor

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

@blakeli0 blakeli0 changed the title POC: OTEL POC: Otel metrics Jul 18, 2023
@blakeli0 blakeli0 closed this Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants