Feat: Add Custom OpenTelemetry Exporter in for Service Metrics #2272

lszinv · 2025-04-02T22:27:38Z

Description

This change adds an implementation for an OpenTelemetry exporter to send the following service metrics that are already present in the Python, Go and Java clients:

operation_latencies operation_count attempt_latencies attempt_count gfe_latency gfe_missing_header_count

Files added to a metrics folder in /src/metrics/ folder:

constants.ts - Constant values such as metric and label names to be used by both the exporter and eventual implementing code. spanner-metrics-exporter.ts - The definition for the exporter external-types.ts - Type definition for ExporterOptions, MetricKind, and ValueType transform.ts - Methods that help create a GCM timeseries list from scope metrics.

Based on regular client Google Cloud Monitoring exporter found here

Impact

Integrates Built-in Metrics support into Node.js spanner client library.

Testing

Unit tests were added in /test/metrics

Checklist

Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
Ensure the tests and linter pass
Code coverage does not decrease
Appropriate docs were updated
Appropriate comments were added, particularly in complex areas or places that require background
No new warnings or issues will be generated from this change

lszinv · 2025-04-02T22:27:44Z

Note This is a re-submission of #2270 with some additional fixes done, please close #2270 and I'll continue to address issues from CI tests & review comments here instead (as @ravjotbrar is away on vacation, and I don't have access to his fork).

src/metrics/constants.ts

surbhigarg92 · 2025-04-03T17:09:44Z

src/metrics/spanner-metrics-exporter.ts

+} from '@opentelemetry/core';
+import {ExporterOptions} from './external-types';
+import {GoogleAuth, JWT} from 'google-auth-library';
+import {monitoring_v3} from 'googleapis';


Node support google-cloud-monitoring package which has MetricServiceClient , could we use that instead?

When I was implementing this, I referenced the original exporter, which also uses monitoring_v3 instead of MetricServiceClient.

However, if the expectation is to use MetricServiceClient, I can focus on making those changes.

Yes . MetricServiceClient is high level client provided to interact with Cloud Monitoring APIs. monitoring_v3 is low level and should be used within the node sdk.

With this change, I believe we should not be needing googleapis package and we should be using @google-cloud/monitoring

It looks like there's no createServiceTimeSeries method in MetricServiceClient. Should we keep this as is for now?

https://github.com/googleapis/google-cloud-node/blob/main/packages/google-cloud-monitoring/src/v3/metric_service_client.ts#L345-L354

I wasn't aware about this, I am checking with the opentelemetry team internally. Lets keep the monitoring_v3 package till then

surbhigarg92 · 2025-04-04T09:16:28Z

src/metrics/spanner-metrics-exporter.ts

+/**
+ * Format and sends metrics information to Google Cloud Monitoring.
+ */
+export class CloudMonitoringMetricsExporter implements PushMetricExporter {


Instead of writing the CloudMonitoringMetricsExporter from scratch, can we extend the available cloud monitoring exporter and just override the "export" method

Thanks for the suggestion @surbhigarg92. I considered this, but based on the amount of refactoring we've done with the existing cloud monitoring exporter, this would involve undoing a lot of that work.

We discussed this amongst ourselves. We'll can change the class to extend the existing Exporter. However, given that we will need to override the constructor connection logic to use the MetricServiceClient and override the Export functionality, there will be virtually no logical changes from the parent class remaining. It will however allow us to not need to re-define duplicate definitions exporter out of external-types.

Even using monitoring_v3, we'll need to define our own connection logic, since we now pass in an auth object instead of authOptions. In addition, the original exporter does not expose many of its variables requiring us to redefine anyways. However, if we still want to go that route, it is definitely possible.

@ravjotbrar Lets skip extending the exusting Exporter then if it is not adding any value. Thanks for considering.

src/metrics/spanner-metrics-exporter.ts

src/metrics/transform.ts

test/metrics/spanner-metrics-exporter.test.ts

src/metrics/transform.ts

surbhigarg92 · 2025-04-08T06:24:05Z

src/metrics/transform.ts

+    dataPointType === DataPointType.EXPONENTIAL_HISTOGRAM
+  ) {
+    return ValueType.DISTRIBUTION;
+  } else if (name.includes('count')) {


Except this everything else is generic and auto detected. If possible , we could make this generic, if this would add a lot to code maintainability please skip.

At the moment, I'm unable to think of solutions that would make this generic. If I get an idea, I will definitely make the change.

src/metrics/transform.ts

src/metrics/README.md

surbhigarg92 · 2025-04-08T06:51:05Z

src/metrics/spanner-metrics-exporter.ts

+} from '@opentelemetry/core';
+import {ExporterOptions} from './external-types';
+import {GoogleAuth, JWT} from 'google-auth-library';
+import {monitoring_v3} from 'googleapis';


Yes . MetricServiceClient is high level client provided to interact with Cloud Monitoring APIs. monitoring_v3 is low level and should be used within the node sdk.

test/metrics/spanner-metrics-exporter.test.ts

src/metrics/spanner-metrics-exporter.ts

src/metrics/README.md

surbhigarg92 · 2025-04-08T07:08:29Z

src/metrics/external-types.ts

+   *
+   * Optional, default is `workload.googleapis.com`.
+   */
+  prefix?: string;


This should not be configurable for our use case as metrics will always go to "spanner.googleapis.com"

Got it, removed.

src/metrics/external-types.ts

lszinv · 2025-04-14T23:09:20Z

Airlock is erroring on not being able to find @grpc/grpc-js@^1.13.3

edit: Removed grpc-js, and airlock eerrors on "googleapis": "146.0.0"

gcf-merge-on-green · 2025-04-17T04:22:19Z

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

lszinv requested review from a team as code owners April 2, 2025 22:27

product-auto-label bot added size: xl Pull request size is extra large. api: spanner Issues related to the googleapis/nodejs-spanner API. labels Apr 2, 2025

lszinv mentioned this pull request Apr 2, 2025

chore: Add Custom OpenTelemetry Exporter in for Service Metrics #2270

Closed

6 tasks

surbhigarg92 reviewed Apr 4, 2025

View reviewed changes

test/metrics/spanner-metrics-exporter.test.ts Outdated Show resolved Hide resolved

surbhigarg92 reviewed Apr 8, 2025

View reviewed changes

ravjotbrar and others added 7 commits April 9, 2025 16:53

chore: Add Custom OpenTelemetry Exporter in for Service Metrics

1348201

Add readme

056b138

removed unused new packages

f0b2371

Added headers

84f26db

Address review comments

01cbb18

Address review comments

7bb9649

Remove env var from .mocharc.js

bdb207d

ravjotbrar force-pushed the custom_exporter branch from a2c33ff to bdb207d Compare April 10, 2025 18:08

surbhigarg92 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 11, 2025

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 11, 2025

surbhigarg92 approved these changes Apr 11, 2025

View reviewed changes

Merge branch 'main' into custom_exporter

51ae5c0

sakthivelmanii approved these changes Apr 11, 2025

View reviewed changes

surbhigarg92 and others added 2 commits April 11, 2025 16:53

Merge branch 'main' into custom_exporter

dd36dcf

Add grpc-js package

1cf0af2

surbhigarg92 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 14, 2025

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 14, 2025

surbhigarg92 added the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 14, 2025

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 14, 2025

Fix tests

1a427a8

lszinv force-pushed the custom_exporter branch from cef2934 to 1a427a8 Compare April 15, 2025 16:58

Merge branch 'main' into custom_exporter

8eb2a32

surbhigarg92 added the automerge Merge the pull request once unit tests and other checks pass. label Apr 16, 2025

Merge branch 'main' into custom_exporter

4434fda

gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Apr 17, 2025

Merge branch 'main' into custom_exporter

83c2076

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Add Custom OpenTelemetry Exporter in for Service Metrics #2272

Feat: Add Custom OpenTelemetry Exporter in for Service Metrics #2272

lszinv commented Apr 2, 2025

lszinv commented Apr 2, 2025 •

edited

Loading

surbhigarg92 Apr 3, 2025

ravjotbrar Apr 7, 2025

surbhigarg92 Apr 8, 2025

surbhigarg92 Apr 8, 2025

ravjotbrar Apr 9, 2025 •

edited

Loading

surbhigarg92 Apr 10, 2025 •

edited

Loading

surbhigarg92 Apr 4, 2025

ravjotbrar Apr 7, 2025

lszinv Apr 9, 2025 •

edited

Loading

ravjotbrar Apr 9, 2025

surbhigarg92 Apr 10, 2025

surbhigarg92 Apr 8, 2025

ravjotbrar Apr 8, 2025

surbhigarg92 Apr 8, 2025

surbhigarg92 Apr 8, 2025

ravjotbrar Apr 8, 2025

lszinv commented Apr 14, 2025 •

edited

Loading

gcf-merge-on-green bot commented Apr 17, 2025

Feat: Add Custom OpenTelemetry Exporter in for Service Metrics #2272

Are you sure you want to change the base?

Feat: Add Custom OpenTelemetry Exporter in for Service Metrics #2272

Conversation

lszinv commented Apr 2, 2025

Description

Impact

Testing

Checklist

lszinv commented Apr 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ravjotbrar Apr 9, 2025 • edited Loading

Choose a reason for hiding this comment

surbhigarg92 Apr 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lszinv Apr 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lszinv commented Apr 14, 2025 • edited Loading

gcf-merge-on-green bot commented Apr 17, 2025

lszinv commented Apr 2, 2025 •

edited

Loading

ravjotbrar Apr 9, 2025 •

edited

Loading

surbhigarg92 Apr 10, 2025 •

edited

Loading

lszinv Apr 9, 2025 •

edited

Loading

lszinv commented Apr 14, 2025 •

edited

Loading