Skip to content

Commit 0bee6a3

Browse files
fix: adding is_monotonic flag to sum (#1793)
* fix: add is_monotonic flag to sum * add monotonic flags to otlp exporter * base is_monotonic on metric_data * dry and test * implement suggestions * making monotonic flag public interface a predicate * remove file oops --------- Co-authored-by: Kayla Reopelle <[email protected]>
1 parent d58388e commit 0bee6a3

File tree

8 files changed

+65
-7
lines changed

8 files changed

+65
-7
lines changed

exporter/otlp-metrics/lib/opentelemetry/exporter/otlp/metrics/metrics_exporter.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,8 @@ def as_otlp_metrics(metrics)
240240
aggregation_temporality: as_otlp_aggregation_temporality(metrics.aggregation_temporality),
241241
data_points: metrics.data_points.map do |ndp|
242242
number_data_point(ndp)
243-
end
243+
end,
244+
is_monotonic: metrics.is_monotonic
244245
)
245246
)
246247

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,9 @@
580580
counter = meter.create_counter('test_counter', unit: 'smidgen', description: 'a small amount of something')
581581
counter.add(5, attributes: { 'foo' => 'bar' })
582582

583+
up_down_counter = meter.create_up_down_counter('test_up_down_counter', unit: 'smidgen', description: 'a small amount of something')
584+
up_down_counter.add(5, attributes: { 'foo' => 'bar' })
585+
583586
histogram = meter.create_histogram('test_histogram', unit: 'smidgen', description: 'a small amount of something')
584587
histogram.record(10, attributes: { 'oof' => 'rab' })
585588

@@ -623,6 +626,27 @@
623626
exemplars: nil
624627
)
625628
],
629+
is_monotonic: true,
630+
aggregation_temporality: Opentelemetry::Proto::Metrics::V1::AggregationTemporality::AGGREGATION_TEMPORALITY_DELTA
631+
)
632+
),
633+
Opentelemetry::Proto::Metrics::V1::Metric.new(
634+
name: 'test_up_down_counter',
635+
description: 'a small amount of something',
636+
unit: 'smidgen',
637+
sum: Opentelemetry::Proto::Metrics::V1::Sum.new(
638+
data_points: [
639+
Opentelemetry::Proto::Metrics::V1::NumberDataPoint.new(
640+
attributes: [
641+
Opentelemetry::Proto::Common::V1::KeyValue.new(key: 'foo', value: Opentelemetry::Proto::Common::V1::AnyValue.new(string_value: 'bar'))
642+
],
643+
as_int: 5,
644+
start_time_unix_nano: 1_699_593_427_329_946_585,
645+
time_unix_nano: 1_699_593_427_329_946_586,
646+
exemplars: nil
647+
)
648+
],
649+
is_monotonic: false,
626650
aggregation_temporality: Opentelemetry::Proto::Metrics::V1::AggregationTemporality::AGGREGATION_TEMPORALITY_DELTA
627651
)
628652
),

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@ module Aggregation
1313
class Sum
1414
attr_reader :aggregation_temporality
1515

16-
def initialize(aggregation_temporality: ENV.fetch('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE', :delta))
16+
def initialize(aggregation_temporality: ENV.fetch('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE', :delta), monotonic: false)
1717
# TODO: the default should be :cumulative, see issue #1555
1818
@aggregation_temporality = aggregation_temporality.to_sym
19+
@monotonic = monotonic
1920
end
2021

2122
def collect(start_time, end_time, data_points)
@@ -38,7 +39,13 @@ def collect(start_time, end_time, data_points)
3839
end
3940
end
4041

42+
def monotonic?
43+
@monotonic
44+
end
45+
4146
def update(increment, attributes, data_points)
47+
return if @monotonic && increment < 0
48+
4249
ndp = data_points[attributes] || data_points[attributes] = NumberDataPoint.new(
4350
attributes,
4451
nil,

metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/counter.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ def add(increment, attributes: {})
4242
private
4343

4444
def default_aggregation
45-
OpenTelemetry::SDK::Metrics::Aggregation::Sum.new
45+
OpenTelemetry::SDK::Metrics::Aggregation::Sum.new(monotonic: true)
4646
end
4747
end
4848
end

metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/up_down_counter.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ def add(amount, attributes: {})
3535
private
3636

3737
def default_aggregation
38-
OpenTelemetry::SDK::Metrics::Aggregation::Sum.new
38+
OpenTelemetry::SDK::Metrics::Aggregation::Sum.new(monotonic: false)
3939
end
4040
end
4141
end

metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_data.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ module State
1818
:data_points, # Hash{Hash{String => String, Numeric, Boolean, Array<String, Numeric, Boolean>} => Numeric}
1919
:aggregation_temporality, # Symbol
2020
:start_time_unix_nano, # Integer nanoseconds since Epoch
21-
:time_unix_nano) # Integer nanoseconds since Epoch
21+
:time_unix_nano, # Integer nanoseconds since Epoch
22+
:is_monotonic) # Boolean
2223
end
2324
end
2425
end

metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_stream.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ def update(value, attributes)
6767

6868
def aggregate_metric_data(start_time, end_time, aggregation: nil)
6969
aggregator = aggregation || @default_aggregation
70+
is_monotonic = aggregator.respond_to?(:monotonic?) ? aggregator.monotonic? : nil
71+
7072
MetricData.new(
7173
@name,
7274
@description,
@@ -77,7 +79,8 @@ def aggregate_metric_data(start_time, end_time, aggregation: nil)
7779
aggregator.collect(start_time, end_time, @data_points),
7880
aggregator.aggregation_temporality,
7981
start_time,
80-
end_time
82+
end_time,
83+
is_monotonic
8184
)
8285
end
8386

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@
88

99
describe OpenTelemetry::SDK::Metrics::Aggregation::Sum do
1010
let(:data_points) { {} }
11-
let(:sum_aggregation) { OpenTelemetry::SDK::Metrics::Aggregation::Sum.new(aggregation_temporality: aggregation_temporality) }
11+
let(:sum_aggregation) { OpenTelemetry::SDK::Metrics::Aggregation::Sum.new(aggregation_temporality: aggregation_temporality, monotonic: monotonic) }
1212
let(:aggregation_temporality) { :delta }
13+
let(:monotonic) { false }
1314

1415
# Time in nano
1516
let(:start_time) { (Time.now.to_r * 1_000_000_000).to_i }
@@ -58,6 +59,14 @@
5859
_(ndps[1].attributes).must_equal('foo' => 'bar')
5960
end
6061

62+
it 'aggregates and collects negative values' do
63+
sum_aggregation.update(1, {}, data_points)
64+
sum_aggregation.update(-2, {}, data_points)
65+
66+
ndps = sum_aggregation.collect(start_time, end_time, data_points)
67+
_(ndps[0].value).must_equal(-1)
68+
end
69+
6170
it 'does not aggregate between collects' do
6271
sum_aggregation.update(1, {}, data_points)
6372
sum_aggregation.update(2, {}, data_points)
@@ -94,4 +103,17 @@
94103
_(ndps[0].value).must_equal(4)
95104
end
96105
end
106+
107+
describe 'when sum type is monotonic' do
108+
let(:aggregation_temporality) { :not_delta }
109+
let(:monotonic) { true }
110+
111+
it 'does not allow negative values to accumulate' do
112+
sum_aggregation.update(1, {}, data_points)
113+
sum_aggregation.update(-2, {}, data_points)
114+
ndps = sum_aggregation.collect(start_time, end_time, data_points)
115+
116+
_(ndps[0].value).must_equal(1)
117+
end
118+
end
97119
end

0 commit comments

Comments
 (0)