fix: add kind enum type for aggregation_temporality that only allow t…

…wo types

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,69 @@
+# frozen_string_literal: true
+
+# Copyright The OpenTelemetry Authors
+#
+# SPDX-License-Identifier: Apache-2.0
+
+module OpenTelemetry
+  module SDK
+    module Metrics
+      module Aggregation
+        # Status represents the status of a finished {Span}. It is composed of a
+        # status code in conjunction with an optional descriptive message.
+        class AggregationTemporality
+          class << self
+            private :new
+
+            # Returns a newly created {Status} with code == UNSET and an optional
+            # description.
+            #
+            # @param [String] description
+            # @return [Status]
+            def delta
+              new(DELTA)
+            end
+
+            # Returns a newly created {Status} with code == OK and an optional
+            # description.
+            #
+            # @param [String] description
+            # @return [Status]
+            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 {unset}, {error}, or {ok} factory methods to
+          # obtain a {Status} instance.
+          #
+          # @param [Integer] code One of the status codes below
+          # @param [String] description
+          def initialize(temporality)
+            @temporality = temporality
+          end
+
+          def delta?
+            @temporality == :delta
+          end
+
+          def cumulative?
+            @temporality == :cumulative
+          end
+
+          # The following represents the set of status codes of a
+          # finished {Span}
+
+          # The operation completed successfully.
+          DELTA = :delta
+
+          # The default status.
+          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
+            @aggregation_temporality = aggregation_temporality == :delta ? AggregationTemporality.delta : AggregationTemporality.cumulative
           end
 
           def collect(start_time, end_time, data_points)
@@ -30,6 +28,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/explicit_bucket_histogram.rb

@@ -14,25 +14,23 @@ 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
@@ -88,6 +86,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,15 +11,12 @@ 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))
-            # 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))
+            @aggregation_temporality = aggregation_temporality.to_sym == :delta ? AggregationTemporality.delta : AggregationTemporality.cumulative
           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
@@ -50,6 +47,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/opentelemetry/sdk/metrics/aggregation/drop_test.rb

@@ -6,7 +6,7 @@
 
 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: aggregation_temporality) }
   let(:aggregation_temporality) { :delta }
@@ -15,6 +15,10 @@
   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]

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

@@ -25,21 +25,67 @@
   describe '#initialize' do
     it 'defaults to the delta aggregation temporality' do
       exp = OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new
-      _(exp.instance_variable_get(:@aggregation_temporality)).must_equal :delta
+      _(exp.aggregation_temporality).must_equal :cumulative
+    end
+
+    it 'sets parameters from the environment to cumulative' do
+      exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE' => 'cumulative') do
+        OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new
+      end
+      _(exp.aggregation_temporality).must_equal :cumulative
+    end
+
+    it 'sets parameters from the environment to delta' do
+      exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE' => 'delta') do
+        OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new
+      end
+      _(exp.aggregation_temporality).must_equal :delta
     end
 
     it 'sets parameters from the environment and converts them to symbols' do
       exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE' => 'potato') do
         OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new
       end
-      _(exp.instance_variable_get(:@aggregation_temporality)).must_equal :potato
+      _(exp.aggregation_temporality).must_equal :cumulative
+    end
+
+    it 'invalid aggregation_temporality from parameters return default to cumulative' do
+      exp = OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(aggregation_temporality: 'pickles')
+      _(exp.aggregation_temporality).must_equal :cumulative
+    end
+
+    it 'valid aggregation_temporality delta from parameters' do
+      exp = OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(aggregation_temporality: 'delta')
+      _(exp.aggregation_temporality).must_equal :delta
+    end
+
+    it 'valid aggregation_temporality cumulative from parameters' do
+      exp = OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(aggregation_temporality: 'cumulative')
+      _(exp.aggregation_temporality).must_equal :cumulative
+    end
+
+    it 'valid aggregation_temporality delta as symbol from parameters' do
+      exp = OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(aggregation_temporality: :delta)
+      _(exp.aggregation_temporality).must_equal :delta
+    end
+
+    it 'valid aggregation_temporality cumulative as symbol from parameters' do
+      exp = OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(aggregation_temporality: :cumulative)
+      _(exp.aggregation_temporality).must_equal :cumulative
     end
 
     it 'prefers explicit parameters rather than the environment and converts them to symbols' do
       exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE' => 'potato') do
         OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(aggregation_temporality: 'pickles')
       end
-      _(exp.instance_variable_get(:@aggregation_temporality)).must_equal :pickles
+      _(exp.aggregation_temporality).must_equal :cumulative
+    end
+
+    it 'function arguments have higher priority than environment' do
+      exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE' => 'cumulative') do
+        OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(aggregation_temporality: :delta)
+      end
+      _(exp.aggregation_temporality).must_equal :delta
     end
   end
 

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

@@ -15,6 +15,28 @@
   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
+    it 'defaults to the delta aggregation temporality' do
+      exp = OpenTelemetry::SDK::Metrics::Aggregation::LastValue.new
+      _(exp.aggregation_temporality).must_equal :delta
+    end
+
+    it 'valid aggregation_temporality delta as symbol from parameters' do
+      exp = OpenTelemetry::SDK::Metrics::Aggregation::LastValue.new(aggregation_temporality: :delta)
+      _(exp.aggregation_temporality).must_equal :delta
+    end
+
+    it 'valid aggregation_temporality cumulative as symbol from parameters' do
+      exp = OpenTelemetry::SDK::Metrics::Aggregation::LastValue.new(aggregation_temporality: :cumulative)
+      _(exp.aggregation_temporality).must_equal :cumulative
+    end
+
+    it 'invalid aggregation_temporality pickles as symbol from parameters return to defaults delta' do
+      exp = OpenTelemetry::SDK::Metrics::Aggregation::LastValue.new(aggregation_temporality: :pickles)
+      _(exp.aggregation_temporality).must_equal :delta
+    end
+  end
+
   it 'sets the timestamps' do
     last_value_aggregation.update(0, {}, data_points)
     ndp = last_value_aggregation.collect(start_time, end_time, data_points)[0]