chore(Spanner)!: upgrade OpenTelemetry to v2 - #9329
alkatrivedi wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request upgrades OpenTelemetry dependencies to v2, removes the deprecated @google-cloud/opentelemetry-resource-util package in favor of @opentelemetry/resource-detector-gcp, and migrates Resource instantiations to resourceFromAttributes. However, the changes to metric views in constants.ts incorrectly use plain ViewOptions instead of instantiating View classes, which will cause runtime or compilation errors. The feedback correctly advises importing Aggregation and View and instantiating the views properly using Aggregation.ExplicitBucketHistogram.
| View, | ||
| ExplicitBucketHistogramAggregation, | ||
| } from '@opentelemetry/sdk-metrics'; | ||
| import { AggregationType, ViewOptions } from '@opentelemetry/sdk-metrics'; |
There was a problem hiding this comment.
In OpenTelemetry JS v2, View is still a class and should be instantiated to be passed to the MeterProvider. Additionally, ExplicitBucketHistogramAggregation has been replaced by the static Aggregation.ExplicitBucketHistogram factory method. Let's import Aggregation and View instead of AggregationType and ViewOptions.
| import { AggregationType, ViewOptions } from '@opentelemetry/sdk-metrics'; | |
| import { Aggregation, View } from '@opentelemetry/sdk-metrics'; |
| const HISTOGRAM_AGGREGATION = { | ||
| type: AggregationType.EXPLICIT_BUCKET_HISTOGRAM as const, | ||
| options: { boundaries: HISTOGRAM_BUCKET_BOUNDARIES }, | ||
| }; | ||
|
|
||
| export const OPERATION_LATENCY_VIEW: ViewOptions = { | ||
| instrumentName: METRIC_NAME_OPERATION_LATENCIES, | ||
| aggregation: new ExplicitBucketHistogramAggregation( | ||
| HISTOGRAM_BUCKET_BOUNDARIES, | ||
| ), | ||
| }); | ||
| aggregation: HISTOGRAM_AGGREGATION, | ||
| }; | ||
|
|
||
| export const ATTEMPT_LATENCY_VIEW = new View({ | ||
| export const ATTEMPT_LATENCY_VIEW: ViewOptions = { | ||
| instrumentName: METRIC_NAME_ATTEMPT_LATENCIES, | ||
| aggregation: new ExplicitBucketHistogramAggregation( | ||
| HISTOGRAM_BUCKET_BOUNDARIES, | ||
| ), | ||
| }); | ||
| aggregation: HISTOGRAM_AGGREGATION, | ||
| }; | ||
|
|
||
| export const GFE_LATENCY_VIEW = new View({ | ||
| export const GFE_LATENCY_VIEW: ViewOptions = { | ||
| instrumentName: METRIC_NAME_GFE_LATENCIES, | ||
| aggregation: new ExplicitBucketHistogramAggregation( | ||
| HISTOGRAM_BUCKET_BOUNDARIES, | ||
| ), | ||
| }); | ||
| aggregation: HISTOGRAM_AGGREGATION, | ||
| }; | ||
|
|
||
| export const AFE_LATENCY_VIEW = new View({ | ||
| export const AFE_LATENCY_VIEW: ViewOptions = { | ||
| instrumentName: METRIC_NAME_AFE_LATENCIES, | ||
| aggregation: new ExplicitBucketHistogramAggregation( | ||
| HISTOGRAM_BUCKET_BOUNDARIES, | ||
| ), | ||
| }); | ||
| aggregation: HISTOGRAM_AGGREGATION, | ||
| }; |
There was a problem hiding this comment.
In OpenTelemetry JS v2, MeterProvider expects an array of View instances rather than plain ViewOptions objects. Passing plain ViewOptions objects will result in runtime errors or compilation failures. We should instantiate View using the new Aggregation.ExplicitBucketHistogram static factory method.
| const HISTOGRAM_AGGREGATION = { | |
| type: AggregationType.EXPLICIT_BUCKET_HISTOGRAM as const, | |
| options: { boundaries: HISTOGRAM_BUCKET_BOUNDARIES }, | |
| }; | |
| export const OPERATION_LATENCY_VIEW: ViewOptions = { | |
| instrumentName: METRIC_NAME_OPERATION_LATENCIES, | |
| aggregation: new ExplicitBucketHistogramAggregation( | |
| HISTOGRAM_BUCKET_BOUNDARIES, | |
| ), | |
| }); | |
| aggregation: HISTOGRAM_AGGREGATION, | |
| }; | |
| export const ATTEMPT_LATENCY_VIEW = new View({ | |
| export const ATTEMPT_LATENCY_VIEW: ViewOptions = { | |
| instrumentName: METRIC_NAME_ATTEMPT_LATENCIES, | |
| aggregation: new ExplicitBucketHistogramAggregation( | |
| HISTOGRAM_BUCKET_BOUNDARIES, | |
| ), | |
| }); | |
| aggregation: HISTOGRAM_AGGREGATION, | |
| }; | |
| export const GFE_LATENCY_VIEW = new View({ | |
| export const GFE_LATENCY_VIEW: ViewOptions = { | |
| instrumentName: METRIC_NAME_GFE_LATENCIES, | |
| aggregation: new ExplicitBucketHistogramAggregation( | |
| HISTOGRAM_BUCKET_BOUNDARIES, | |
| ), | |
| }); | |
| aggregation: HISTOGRAM_AGGREGATION, | |
| }; | |
| export const AFE_LATENCY_VIEW = new View({ | |
| export const AFE_LATENCY_VIEW: ViewOptions = { | |
| instrumentName: METRIC_NAME_AFE_LATENCIES, | |
| aggregation: new ExplicitBucketHistogramAggregation( | |
| HISTOGRAM_BUCKET_BOUNDARIES, | |
| ), | |
| }); | |
| aggregation: HISTOGRAM_AGGREGATION, | |
| }; | |
| export const OPERATION_LATENCY_VIEW = new View({ | |
| instrumentName: METRIC_NAME_OPERATION_LATENCIES, | |
| aggregation: Aggregation.ExplicitBucketHistogram(HISTOGRAM_BUCKET_BOUNDARIES), | |
| }); | |
| export const ATTEMPT_LATENCY_VIEW = new View({ | |
| instrumentName: METRIC_NAME_ATTEMPT_LATENCIES, | |
| aggregation: Aggregation.ExplicitBucketHistogram(HISTOGRAM_BUCKET_BOUNDARIES), | |
| }); | |
| export const GFE_LATENCY_VIEW = new View({ | |
| instrumentName: METRIC_NAME_GFE_LATENCIES, | |
| aggregation: Aggregation.ExplicitBucketHistogram(HISTOGRAM_BUCKET_BOUNDARIES), | |
| }); | |
| export const AFE_LATENCY_VIEW = new View({ | |
| instrumentName: METRIC_NAME_AFE_LATENCIES, | |
| aggregation: Aggregation.ExplicitBucketHistogram(HISTOGRAM_BUCKET_BOUNDARIES), | |
| }); |
a2ba841 to
936f2e9
Compare
936f2e9 to
c4c35bc
Compare
No description provided.