From 3fde77b4aa376f2c5c8d5a1242c443747c7c8ad9 Mon Sep 17 00:00:00 2001 From: "Kayla Reopelle (she/her)" <87386821+kaylareopelle@users.noreply.github.com> Date: Tue, 6 Feb 2024 07:30:38 -0800 Subject: [PATCH] feat: Add aggregation_temporality to MetricData (#1554) * feat: Add aggregation_temporality to MetricData Currently, @aggregation_temporality is set for Sum and ExplicitBucketHistorgram, but is not passed to MetricData, so it cannot reach the exporter. Now, @aggregation_temporality is included in MetricData and can be set using OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE. * Update comment for rubocop * Update metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/explicit_bucket_histogram.rb * Update metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb * feat: Add attr_reader for aggregation_temporality --------- Co-authored-by: Robert --- .../sdk/metrics/aggregation/explicit_bucket_histogram.rb | 4 +++- metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb | 5 ++++- .../lib/opentelemetry/sdk/metrics/state/metric_data.rb | 1 + .../lib/opentelemetry/sdk/metrics/state/metric_stream.rb | 1 + .../test/integration/in_memory_metric_pull_exporter_test.rb | 2 ++ .../opentelemetry/sdk/metrics/instrument/counter_test.rb | 1 + .../opentelemetry/sdk/metrics/instrument/histogram_test.rb | 1 + .../sdk/metrics/instrument/up_down_counter_test.rb | 1 + 8 files changed, 14 insertions(+), 2 deletions(-) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/explicit_bucket_histogram.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/explicit_bucket_histogram.rb index 58d87ccaa4..5af469b61d 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/explicit_bucket_histogram.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/explicit_bucket_histogram.rb @@ -14,12 +14,14 @@ 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: :delta, + aggregation_temporality: ENV.fetch('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE', :delta), # TODO: the default should be :cumulative, see issue #1555 boundaries: DEFAULT_BOUNDARIES, record_min_max: true ) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb index 31f8ac6681..16cbccc430 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb @@ -11,7 +11,10 @@ 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 - def initialize(aggregation_temporality: :delta) + 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 @data_points = {} end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_data.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_data.rb index 9885deba71..e07c301295 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_data.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_data.rb @@ -16,6 +16,7 @@ module State :resource, # OpenTelemetry::SDK::Resources::Resource :instrumentation_scope, # OpenTelemetry::SDK::InstrumentationScope :data_points, # Hash{Hash{String => String, Numeric, Boolean, Array} => Numeric} + :aggregation_temporality, # Symbol :start_time_unix_nano, # Integer nanoseconds since Epoch :time_unix_nano) # Integer nanoseconds since Epoch end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_stream.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_stream.rb index d79d4a51ba..7f98115dce 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_stream.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_stream.rb @@ -45,6 +45,7 @@ def collect(start_time, end_time) @meter_provider.resource, @instrumentation_scope, @aggregation.collect(start_time, end_time), + @aggregation.aggregation_temporality, start_time, end_time ) diff --git a/metrics_sdk/test/integration/in_memory_metric_pull_exporter_test.rb b/metrics_sdk/test/integration/in_memory_metric_pull_exporter_test.rb index f3e0dc0c82..bed4bdf935 100644 --- a/metrics_sdk/test/integration/in_memory_metric_pull_exporter_test.rb +++ b/metrics_sdk/test/integration/in_memory_metric_pull_exporter_test.rb @@ -46,6 +46,8 @@ _(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) end end end diff --git a/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/counter_test.rb b/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/counter_test.rb index d7e0b91f02..c00e2eba7b 100644 --- a/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/counter_test.rb +++ b/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/counter_test.rb @@ -28,5 +28,6 @@ _(last_snapshot[0].instrumentation_scope.name).must_equal('test') _(last_snapshot[0].data_points[0].value).must_equal(1) _(last_snapshot[0].data_points[0].attributes).must_equal('foo' => 'bar') + _(last_snapshot[0].aggregation_temporality).must_equal(:delta) end end diff --git a/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/histogram_test.rb b/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/histogram_test.rb index c167615d1e..bddc7f5568 100644 --- a/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/histogram_test.rb +++ b/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/histogram_test.rb @@ -33,5 +33,6 @@ _(last_snapshot[0].data_points[0].max).must_equal(6) _(last_snapshot[0].data_points[0].bucket_counts).must_equal([0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0]) _(last_snapshot[0].data_points[0].attributes).must_equal('foo' => 'bar') + _(last_snapshot[0].aggregation_temporality).must_equal(:delta) end end diff --git a/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/up_down_counter_test.rb b/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/up_down_counter_test.rb index 609ca93298..456c2c5052 100644 --- a/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/up_down_counter_test.rb +++ b/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/up_down_counter_test.rb @@ -29,5 +29,6 @@ _(last_snapshot[0].instrumentation_scope.name).must_equal('test') _(last_snapshot[0].data_points[0].attributes).must_equal('foo' => 'bar') _(last_snapshot[0].data_points[0].value).must_equal(-1) + _(last_snapshot[0].aggregation_temporality).must_equal(:delta) end end