fix: add kind enum type for aggregation_temporality that only allow two types (#1769)

* fix: add kind enum type for aggregation_temporality that only allow two types

* update the doc for aggregation_temporality

* force drop aggregation temp to nil

* test case update

---------

Co-authored-by: Kayla Reopelle <[email protected]>

Author: xuan-cao-swi

exporter/otlp-metrics/test/opentelemetry/exporter/otlp/metrics/metrics_exporter_test.rb

@@ -652,7 +652,7 @@
                           )
                         ],
                         is_monotonic: true,
-                        aggregation_temporality: Opentelemetry::Proto::Metrics::V1::AggregationTemporality::AGGREGATION_TEMPORALITY_DELTA
+                        aggregation_temporality: Opentelemetry::Proto::Metrics::V1::AggregationTemporality::AGGREGATION_TEMPORALITY_CUMULATIVE
                       )
                     ),
                     Opentelemetry::Proto::Metrics::V1::Metric.new(
@@ -672,7 +672,7 @@
                           )
                         ],
                         is_monotonic: false,
-                        aggregation_temporality: Opentelemetry::Proto::Metrics::V1::AggregationTemporality::AGGREGATION_TEMPORALITY_DELTA
+                        aggregation_temporality: Opentelemetry::Proto::Metrics::V1::AggregationTemporality::AGGREGATION_TEMPORALITY_CUMULATIVE
                       )
                     ),
                     Opentelemetry::Proto::Metrics::V1::Metric.new(
@@ -696,7 +696,7 @@
                             max: 10
                           )
                         ],
-                        aggregation_temporality: Opentelemetry::Proto::Metrics::V1::AggregationTemporality::AGGREGATION_TEMPORALITY_DELTA
+                        aggregation_temporality: Opentelemetry::Proto::Metrics::V1::AggregationTemporality::AGGREGATION_TEMPORALITY_CUMULATIVE
                       )
                     ),
                     Opentelemetry::Proto::Metrics::V1::Metric.new(

metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation.rb

@@ -15,6 +15,7 @@ module Aggregation
   end
 end
 
+require 'opentelemetry/sdk/metrics/aggregation/aggregation_temporality'
 require 'opentelemetry/sdk/metrics/aggregation/number_data_point'
 require 'opentelemetry/sdk/metrics/aggregation/histogram_data_point'
 require 'opentelemetry/sdk/metrics/aggregation/explicit_bucket_histogram'

metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/aggregation_temporality.rb

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+# Copyright The OpenTelemetry Authors
+#
+# SPDX-License-Identifier: Apache-2.0
+
+module OpenTelemetry
+  module SDK
+    module Metrics
+      module Aggregation
+        # AggregationTemporality represents the temporality of
+        # data point ({NumberDataPoint} and {HistogramDataPoint}) in {Metrics}.
+        # It determine whether the data point will be cleared for each metrics pull/export.
+        class AggregationTemporality
+          class << self
+            private :new
+
+            # Returns a newly created {AggregationTemporality} with temporality == DELTA
+            #
+            # @return [AggregationTemporality]
+            def delta
+              new(DELTA)
+            end
+
+            # Returns a newly created {AggregationTemporality} with temporality == CUMULATIVE
+            #
+            # @return [AggregationTemporality]
+            def cumulative
+              new(CUMULATIVE)
+            end
+          end
+
+          attr_reader :temporality
+
+          # @api private
+          # The constructor is private and only for use internally by the class.
+          # Users should use the {delta} and {cumulative} factory methods to obtain
+          # a {AggregationTemporality} instance.
+          #
+          # @param [Integer] temporality One of the status codes below
+          def initialize(temporality)
+            @temporality = temporality
+          end
+
+          def delta?
+            @temporality == :delta
+          end
+
+          def cumulative?
+            @temporality == :cumulative
+          end
+
+          # delta: data point will be cleared after each metrics pull/export.
+          DELTA = :delta
+
+          # cumulative: data point will NOT be cleared after metrics pull/export.
+          CUMULATIVE = :cumulative
+        end
+      end
+    end
+  end
+end

metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/drop.rb

@@ -10,10 +10,8 @@ module Metrics
       module Aggregation
         # Contains the implementation of the Drop aggregation
         class Drop
-          attr_reader :aggregation_temporality
-
-          def initialize(aggregation_temporality: :delta)
-            @aggregation_temporality = aggregation_temporality
+          def initialize
+            @aggregation_temporality = nil
           end
 
           def collect(start_time, end_time, data_points)
@@ -30,6 +28,10 @@ def update(increment, attributes, data_points)
             )
             nil
           end
+
+          def aggregation_temporality
+            nil
+          end
         end
       end
     end

metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/explicit_bucket_histogram.rb

@@ -14,24 +14,22 @@ class ExplicitBucketHistogram
           DEFAULT_BOUNDARIES = [0, 5, 10, 25, 50, 75, 100, 250, 500, 1000].freeze
           private_constant :DEFAULT_BOUNDARIES
 
-          attr_reader :aggregation_temporality
-
           # The default value for boundaries represents the following buckets:
           # (-inf, 0], (0, 5.0], (5.0, 10.0], (10.0, 25.0], (25.0, 50.0],
           # (50.0, 75.0], (75.0, 100.0], (100.0, 250.0], (250.0, 500.0],
           # (500.0, 1000.0], (1000.0, +inf)
           def initialize(
-            aggregation_temporality: ENV.fetch('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE', :delta), # TODO: the default should be :cumulative, see issue #1555
+            aggregation_temporality: ENV.fetch('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE', :cumulative),
             boundaries: DEFAULT_BOUNDARIES,
             record_min_max: true
           )
-            @aggregation_temporality = aggregation_temporality.to_sym
+            @aggregation_temporality = aggregation_temporality.to_sym == :delta ? AggregationTemporality.delta : AggregationTemporality.cumulative
             @boundaries = boundaries && !boundaries.empty? ? boundaries.sort : nil
             @record_min_max = record_min_max
           end
 
           def collect(start_time, end_time, data_points)
-            if @aggregation_temporality == :delta
+            if @aggregation_temporality.delta?
               # Set timestamps and 'move' data point values to result.
               hdps = data_points.values.map! do |hdp|
                 hdp.start_time_unix_nano = start_time
@@ -87,6 +85,10 @@ def update(amount, attributes, data_points)
             nil
           end
 
+          def aggregation_temporality
+            @aggregation_temporality.temporality
+          end
+
           private
 
           def empty_bucket_counts

metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/last_value.rb

@@ -10,14 +10,12 @@ module Metrics
       module Aggregation
         # Contains the implementation of the LastValue aggregation
         class LastValue
-          attr_reader :aggregation_temporality
-
           def initialize(aggregation_temporality: :delta)
-            @aggregation_temporality = aggregation_temporality
+            @aggregation_temporality = aggregation_temporality == :cumulative ? AggregationTemporality.cumulative : AggregationTemporality.delta
           end
 
           def collect(start_time, end_time, data_points)
-            if @aggregation_temporality == :delta
+            if @aggregation_temporality.delta?
               # Set timestamps and 'move' data point values to result.
               ndps = data_points.values.map! do |ndp|
                 ndp.start_time_unix_nano = start_time
@@ -46,6 +44,10 @@ def update(increment, attributes, data_points)
             )
             nil
           end
+
+          def aggregation_temporality
+            @aggregation_temporality.temporality
+          end
         end
       end
     end

metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb

@@ -11,16 +11,13 @@ module Aggregation
         # Contains the implementation of the Sum aggregation
         # https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#sum-aggregation
         class Sum
-          attr_reader :aggregation_temporality
-
-          def initialize(aggregation_temporality: ENV.fetch('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE', :delta), monotonic: false)
-            # TODO: the default should be :cumulative, see issue #1555
-            @aggregation_temporality = aggregation_temporality.to_sym
+          def initialize(aggregation_temporality: ENV.fetch('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE', :cumulative), monotonic: false)
+            @aggregation_temporality = aggregation_temporality.to_sym == :delta ? AggregationTemporality.delta : AggregationTemporality.cumulative
             @monotonic = monotonic
           end
 
           def collect(start_time, end_time, data_points)
-            if @aggregation_temporality == :delta
+            if @aggregation_temporality.delta?
               # Set timestamps and 'move' data point values to result.
               ndps = data_points.values.map! do |ndp|
                 ndp.start_time_unix_nano = start_time
@@ -57,6 +54,10 @@ def update(increment, attributes, data_points)
             ndp.value += increment
             nil
           end
+
+          def aggregation_temporality
+            @aggregation_temporality.temporality
+          end
         end
       end
     end

metrics_sdk/test/integration/in_memory_metric_pull_exporter_test.rb

@@ -47,7 +47,7 @@
       _(last_snapshot[0].data_points[3].value).must_equal(4)
       _(last_snapshot[0].data_points[3].attributes).must_equal('d' => 'e')
 
-      _(last_snapshot[0].aggregation_temporality).must_equal(:delta)
+      _(last_snapshot[0].aggregation_temporality).must_equal(:cumulative)
     end
   end
 end

metrics_sdk/test/integration/periodic_metric_reader_test.rb

@@ -9,7 +9,15 @@
 
 describe OpenTelemetry::SDK do
   describe '#periodic_metric_reader' do
-    before { reset_metrics_sdk }
+    before do
+      reset_metrics_sdk
+      @original_temp = ENV['OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE']
+      ENV['OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE'] = 'delta'
+    end
+
+    after do
+      ENV['OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE'] = @original_temp
+    end
 
     # OTLP cannot export a metric without data points
     it 'does not export metrics without data points' do

metrics_sdk/test/opentelemetry/sdk/metrics/aggregation/drop_test.rb

@@ -6,15 +6,19 @@
 
 require 'test_helper'
 
-describe OpenTelemetry::SDK::Metrics::Aggregation::LastValue do
+describe OpenTelemetry::SDK::Metrics::Aggregation::Drop do
   let(:data_points) { {} }
-  let(:drop_aggregation) { OpenTelemetry::SDK::Metrics::Aggregation::Drop.new(aggregation_temporality:) }
+  let(:drop_aggregation) { OpenTelemetry::SDK::Metrics::Aggregation::Drop.new }
   let(:aggregation_temporality) { :delta }
 
   # Time in nano
   let(:start_time) { (Time.now.to_r * 1_000_000_000).to_i }
   let(:end_time) { ((Time.now + 60).to_r * 1_000_000_000).to_i }
 
+  describe '#initialize' do
+    # drop aggregation doesn't care about aggregation_temporality since all data will be dropped
+  end
+
   it 'sets the timestamps' do
     drop_aggregation.update(0, {}, data_points)
     ndp = drop_aggregation.collect(start_time, end_time, data_points)[0]