Feat: client side metrics #1840

milaGGL · 2024-09-19T16:33:04Z

Integrate Opentelemetry API and SDK into the Firestore SDK
Implement BuiltinMetricsProvider class to collect 4 GAX layer and 4 SDK layer metrics
Integrate GoogleCloudMetricExporter to export metrics data to CloudMonitoring custom.googleapis.com namespace (Waiting for BE setup)
Create public API to enable/disable client side metrics (the API is set to private, and disaled for now.)

BEGIN_COMMIT_OVERRIDE
refactor: client side metrics implementation
END_COMMIT_OVERRIDE

milaGGL · 2024-09-27T19:26:48Z

...ud-firestore/src/main/java/com/google/cloud/firestore/telemetry/BuiltinMetricsConstants.java

+  static final String FIRESTORE_LIBRARY_NAME = "firestore_java";
+
+  // TODO: change to firestore.googleapis.com
+  public static final String METER_NAME = "custom.googleapis.com/internal/client";


Opentelemetry uses dot separation in metrics naming, while monarch uses slash separation. exporting the built-in metrics to other backends might raise naming conflict.

we could have named the metrics with dot seperation, use view to reformat it with slash separation in the default OTEL instance for monarch. But the metrics from Gax uses slash already, using dot separation inside firestore doesn't solves the problem.

ehsannas

Great work @milaGGL. A few minor comments/nits below.

I'd like us to disable the feature and land this PR

...e-cloud-firestore/src/main/java/com/google/cloud/firestore/telemetry/TelemetryConstants.java

google-cloud-firestore/src/main/java/com/google/cloud/firestore/telemetry/ClientIdentifier.java

google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreOptions.java

...oud-firestore/src/main/java/com/google/cloud/firestore/telemetry/BuiltinMetricsProvider.java

...e-cloud-firestore/src/main/java/com/google/cloud/firestore/telemetry/EnabledMetricsUtil.java

google-cloud-firestore/src/main/java/com/google/cloud/firestore/telemetry/MetricsUtil.java

google-cloud-firestore/src/main/java/com/google/cloud/firestore/StreamableQuery.java

google-cloud-firestore/clirr-ignored-differences.xml

google-cloud-firestore/src/main/java/com/google/cloud/firestore/telemetry/MetricsUtil.java

milaGGL · 2024-10-23T18:24:54Z

google-cloud-firestore/clirr-ignored-differences.xml

+  </difference>
+
+  <!-- Change the parameter type of internalStream function -->
+  <difference>


waiting for #1896 to lift the clirr alert

I believe the decision for #1896 was to use internalApi rather than making the class package-private. So we'll need to keep this clirr change as well.

they are changing the internalStreamfrom protected to default . So it would affect the public access of that method, which could remove the need to add clirr for the parameter change.

ehsannas

LGTM. Only nit is to add a test to verify the behavior of MonitoredStreamResponseObserver in face of a retryable or non-retryable error.

google-cloud-firestore/src/main/java/com/google/cloud/firestore/StreamableQuery.java

milaGGL added 13 commits August 14, 2024 17:09

enable 4 metrics from GAX

30a7011

format

ed85413

add default otel

57084c4

format

571c095

move constants out

f4b8416

format

da0fe82

add additional attributes

61432eb

Merge branch 'main' into mila/client-side-metrics-demo

b5bfe5c

add attributes to gax metrics

f0a029d

Update BuiltinMetricsConstants.java

5456038

remove current metrics implementation

5efaba6

add custom exporter, add latency recorder

c0b4cc1

use 2 otel instances

3979331

product-auto-label bot added size: l Pull request size is large. api: firestore Issues related to the googleapis/java-firestore API. labels Sep 19, 2024

set custom apiTracerFactory to gax

a9d08b5

product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Sep 20, 2024

milaGGL added 7 commits September 23, 2024 12:15

Merge branch 'main' into mila/client-side-metrics-demo2

029bd8c

fix opentelemtry dependency version conflict issue

50e05fb

use enabled/disabled style

53cd48a

modify access modifiers

7adb9d6

format

690e4ee

rename boolean

e6f8ba6

Merge branch 'main' into mila/client-side-metrics-demo2

c4ca4ae

milaGGL commented Sep 27, 2024

View reviewed changes

cloud-java-bot and others added 4 commits September 27, 2024 19:27

chore: generate libraries at Fri Sep 27 19:25:02 UTC 2024

887221d

Update pom.xml

a2c715a

resolve comments from proto

ed78182

e2E latency for runTransaction

e0d46be

Update BuiltinMetricsProvider.java

1999ff7

ehsannas reviewed Oct 11, 2024

View reviewed changes

milaGGL added 6 commits October 16, 2024 12:34

Merge branch 'main' into mila/client-side-metrics-demo2

81e349b

resolve comments

f4e36d9

Update pom.xml

05e8efb

remove duplicated tracing dependency

b0b1687

add opentelemetry-sdk-metrics dependency

4abbcf0

adjust end-to-end latency in streaming queries

328855d

milaGGL commented Oct 21, 2024

View reviewed changes

google-cloud-firestore/src/main/java/com/google/cloud/firestore/StreamableQuery.java Show resolved Hide resolved

milaGGL assigned ehsannas and unassigned milaGGL Oct 21, 2024

milaGGL requested a review from ehsannas October 21, 2024 15:20

milaGGL commented Oct 21, 2024

View reviewed changes

google-cloud-firestore/clirr-ignored-differences.xml Show resolved Hide resolved

fix clirr error

5e1fb4b

milaGGL commented Oct 23, 2024

View reviewed changes

google-cloud-firestore/src/main/java/com/google/cloud/firestore/telemetry/MetricsUtil.java Show resolved Hide resolved

add exportBuiltinMetricsToGoogleCloudMonitoring flag

4fa8874

milaGGL commented Oct 23, 2024

View reviewed changes

ehsannas reviewed Oct 28, 2024

View reviewed changes

google-cloud-firestore/src/main/java/com/google/cloud/firestore/StreamableQuery.java Show resolved Hide resolved

google-cloud-firestore/src/main/java/com/google/cloud/firestore/StreamableQuery.java Show resolved Hide resolved

Merge branch 'main' into mila/client-side-metrics-demo2

7deb0d6

ehsannas approved these changes Oct 29, 2024

View reviewed changes

Merge branch 'main' into mila/client-side-metrics-demo2

637f878

milaGGL assigned milaGGL and unassigned ehsannas Nov 4, 2024

milaGGL added 2 commits November 4, 2024 13:04

Update clirr-ignored-differences.xml

8480fa8

add internalStream clirr change back

ef9baa8

milaGGL merged commit 77763c0 into main Nov 5, 2024
23 of 24 checks passed

milaGGL deleted the mila/client-side-metrics-demo2 branch November 5, 2024 17:49

release-please bot mentioned this pull request Nov 5, 2024

chore(main): release 3.29.0 #1919

Merged

MarkDuckworth added the release-please:force-run To run release-please label Nov 5, 2024

release-please bot removed the release-please:force-run To run release-please label Nov 5, 2024

Feat: client side metrics #1840

Feat: client side metrics #1840

Uh oh!

Conversation

milaGGL commented Sep 19, 2024 • edited by MarkDuckworth Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

milaGGL Sep 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

milaGGL Sep 27, 2024

Choose a reason for hiding this comment

Uh oh!

ehsannas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

milaGGL Oct 23, 2024

Choose a reason for hiding this comment

Uh oh!

ehsannas Oct 29, 2024

Choose a reason for hiding this comment

Uh oh!

milaGGL Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ehsannas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

milaGGL commented Sep 19, 2024 •

edited by MarkDuckworth

Loading

milaGGL Sep 27, 2024 •

edited

Loading

milaGGL Oct 30, 2024 •

edited

Loading