From ee810ca0442a15d9fd667dda851524865007f881 Mon Sep 17 00:00:00 2001 From: Sam Handler Date: Tue, 16 Jan 2024 15:35:20 -0700 Subject: [PATCH 01/10] add context to metrics reporting of buffer-full events --- .../opentelemetry/sdk/trace/export/batch_span_processor.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb index a00d9df52d..a295756df3 100644 --- a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb +++ b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb @@ -82,7 +82,7 @@ def on_finish(span) n = spans.size + 1 - max_queue_size if n.positive? spans.shift(n) - report_dropped_spans(n, reason: 'buffer-full') + report_dropped_spans(n, reason: 'buffer-full', context: 'on_finish') end spans << span @condition.signal if spans.size > batch_size @@ -123,7 +123,7 @@ def force_flush(timeout: nil) # rubocop:disable Metrics/MethodLength n = spans.size + snapshot.size - max_queue_size if n.positive? snapshot.shift(n) - report_dropped_spans(n, reason: 'buffer-full') + report_dropped_spans(n, reason: 'buffer-full', context: "force_flush") end spans.unshift(snapshot) unless snapshot.empty? @condition.signal if spans.size > max_queue_size / 2 From fec17cf28218df8e876ca597013925b5e2316e20 Mon Sep 17 00:00:00 2001 From: Sam <370182+plantfansam@users.noreply.github.com> Date: Wed, 17 Jan 2024 08:23:21 -0700 Subject: [PATCH 02/10] Update sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb Co-authored-by: Francis Bogsanyi --- sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb index a295756df3..cd9e18b1d0 100644 --- a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb +++ b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb @@ -123,7 +123,7 @@ def force_flush(timeout: nil) # rubocop:disable Metrics/MethodLength n = spans.size + snapshot.size - max_queue_size if n.positive? snapshot.shift(n) - report_dropped_spans(n, reason: 'buffer-full', context: "force_flush") + report_dropped_spans(n, reason: 'buffer-full', context: 'force_flush') end spans.unshift(snapshot) unless snapshot.empty? @condition.signal if spans.size > max_queue_size / 2 From 03fb66cd841d488d2d80c24e954d3aa30e6d6280 Mon Sep 17 00:00:00 2001 From: Sam Handler Date: Wed, 17 Jan 2024 08:55:31 -0700 Subject: [PATCH 03/10] Use semantic convention for code function label --- .../sdk/trace/export/batch_span_processor.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb index cd9e18b1d0..2f6f499a26 100644 --- a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb +++ b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb @@ -82,7 +82,7 @@ def on_finish(span) n = spans.size + 1 - max_queue_size if n.positive? spans.shift(n) - report_dropped_spans(n, reason: 'buffer-full', context: 'on_finish') + report_dropped_spans(n, reason => 'buffer-full', OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => 'on_finish') end spans << span @condition.signal if spans.size > batch_size @@ -123,7 +123,7 @@ def force_flush(timeout: nil) # rubocop:disable Metrics/MethodLength n = spans.size + snapshot.size - max_queue_size if n.positive? snapshot.shift(n) - report_dropped_spans(n, reason: 'buffer-full', context: 'force_flush') + report_dropped_spans(n, reason => 'buffer-full', OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => 'force_flush') end spans.unshift(snapshot) unless snapshot.empty? @condition.signal if spans.size > max_queue_size / 2 @@ -204,8 +204,8 @@ def report_result(result_code, batch) end end - def report_dropped_spans(count, reason:) - @metrics_reporter.add_to_counter('otel.bsp.dropped_spans', increment: count, labels: { 'reason' => reason }) + def report_dropped_spans(count, labels = {}) + @metrics_reporter.add_to_counter('otel.bsp.dropped_spans', increment: count, labels: labels) end def fetch_batch From dfab14e06e961a925be24be69d4ae91d0d463814 Mon Sep 17 00:00:00 2001 From: Sam Handler Date: Wed, 17 Jan 2024 12:35:44 -0700 Subject: [PATCH 04/10] Kwargs are better; use kwargs --- .../sdk/trace/export/batch_span_processor.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb index 2f6f499a26..228ec9155e 100644 --- a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb +++ b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb @@ -82,7 +82,7 @@ def on_finish(span) n = spans.size + 1 - max_queue_size if n.positive? spans.shift(n) - report_dropped_spans(n, reason => 'buffer-full', OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => 'on_finish') + report_dropped_spans(n, reason: 'buffer-full', function: 'on_finish') end spans << span @condition.signal if spans.size > batch_size @@ -123,7 +123,7 @@ def force_flush(timeout: nil) # rubocop:disable Metrics/MethodLength n = spans.size + snapshot.size - max_queue_size if n.positive? snapshot.shift(n) - report_dropped_spans(n, reason => 'buffer-full', OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => 'force_flush') + report_dropped_spans(n, reason: 'buffer-full', function: 'force_flush') end spans.unshift(snapshot) unless snapshot.empty? @condition.signal if spans.size > max_queue_size / 2 @@ -204,8 +204,8 @@ def report_result(result_code, batch) end end - def report_dropped_spans(count, labels = {}) - @metrics_reporter.add_to_counter('otel.bsp.dropped_spans', increment: count, labels: labels) + def report_dropped_spans(count, reason:, function: nil) + @metrics_reporter.add_to_counter('otel.bsp.dropped_spans', increment: count, labels: { 'reason' => reason, OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => function }.compact ) end def fetch_batch From 452f96a4014a85b1987f8b8c171659527cedba87 Mon Sep 17 00:00:00 2001 From: Sam Handler Date: Wed, 17 Jan 2024 15:30:47 -0700 Subject: [PATCH 05/10] stray paren --- sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb index 228ec9155e..0a9faa1c04 100644 --- a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb +++ b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb @@ -205,7 +205,7 @@ def report_result(result_code, batch) end def report_dropped_spans(count, reason:, function: nil) - @metrics_reporter.add_to_counter('otel.bsp.dropped_spans', increment: count, labels: { 'reason' => reason, OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => function }.compact ) + @metrics_reporter.add_to_counter('otel.bsp.dropped_spans', increment: count, labels: { 'reason' => reason, OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => function }.compact) end def fetch_batch From 68fe04049da23578ce41001e393d49b42507db7e Mon Sep 17 00:00:00 2001 From: Sam Handler Date: Fri, 19 Jan 2024 13:55:37 -0700 Subject: [PATCH 06/10] use __method__ to access current method --- .../opentelemetry/sdk/trace/export/batch_span_processor.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb index 0a9faa1c04..8fbc4c05a3 100644 --- a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb +++ b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb @@ -82,7 +82,7 @@ def on_finish(span) n = spans.size + 1 - max_queue_size if n.positive? spans.shift(n) - report_dropped_spans(n, reason: 'buffer-full', function: 'on_finish') + report_dropped_spans(n, reason: 'buffer-full', function: __method__) end spans << span @condition.signal if spans.size > batch_size @@ -205,7 +205,7 @@ def report_result(result_code, batch) end def report_dropped_spans(count, reason:, function: nil) - @metrics_reporter.add_to_counter('otel.bsp.dropped_spans', increment: count, labels: { 'reason' => reason, OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => function }.compact) + @metrics_reporter.add_to_counter('otel.bsp.dropped_spans', increment: count, labels: { 'reason' => reason, OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => __method__ }.compact) end def fetch_batch From 6a3c754b4d4ba51bb3012d281647d39570aae250 Mon Sep 17 00:00:00 2001 From: Sam Handler Date: Fri, 19 Jan 2024 13:57:21 -0700 Subject: [PATCH 07/10] code better --- .../opentelemetry/sdk/trace/export/batch_span_processor.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb index 8fbc4c05a3..e92be7cf9e 100644 --- a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb +++ b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb @@ -123,7 +123,7 @@ def force_flush(timeout: nil) # rubocop:disable Metrics/MethodLength n = spans.size + snapshot.size - max_queue_size if n.positive? snapshot.shift(n) - report_dropped_spans(n, reason: 'buffer-full', function: 'force_flush') + report_dropped_spans(n, reason: 'buffer-full', function: __method__) end spans.unshift(snapshot) unless snapshot.empty? @condition.signal if spans.size > max_queue_size / 2 @@ -205,7 +205,7 @@ def report_result(result_code, batch) end def report_dropped_spans(count, reason:, function: nil) - @metrics_reporter.add_to_counter('otel.bsp.dropped_spans', increment: count, labels: { 'reason' => reason, OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => __method__ }.compact) + @metrics_reporter.add_to_counter('otel.bsp.dropped_spans', increment: count, labels: { 'reason' => reason, OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => function }.compact) end def fetch_batch From 6b178804451d8b90339273c34efd4b21536d5909 Mon Sep 17 00:00:00 2001 From: Sam Handler Date: Fri, 19 Jan 2024 13:59:41 -0700 Subject: [PATCH 08/10] method to string --- .../opentelemetry/sdk/trace/export/batch_span_processor.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb index e92be7cf9e..ebec229a27 100644 --- a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb +++ b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb @@ -82,7 +82,7 @@ def on_finish(span) n = spans.size + 1 - max_queue_size if n.positive? spans.shift(n) - report_dropped_spans(n, reason: 'buffer-full', function: __method__) + report_dropped_spans(n, reason: 'buffer-full', function: __method__.to_s) end spans << span @condition.signal if spans.size > batch_size @@ -123,7 +123,7 @@ def force_flush(timeout: nil) # rubocop:disable Metrics/MethodLength n = spans.size + snapshot.size - max_queue_size if n.positive? snapshot.shift(n) - report_dropped_spans(n, reason: 'buffer-full', function: __method__) + report_dropped_spans(n, reason: 'buffer-full', function: __method__.to_s) end spans.unshift(snapshot) unless snapshot.empty? @condition.signal if spans.size > max_queue_size / 2 From 40e5e89cdef78019d05d112606b17893ae943626 Mon Sep 17 00:00:00 2001 From: Sam Handler Date: Fri, 19 Jan 2024 14:07:16 -0700 Subject: [PATCH 09/10] pass spans as arg to report_dropped_spans --- .../sdk/trace/export/batch_span_processor.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb index ebec229a27..c6312ed88d 100644 --- a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb +++ b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb @@ -81,8 +81,8 @@ def on_finish(span) reset_on_fork n = spans.size + 1 - max_queue_size if n.positive? - spans.shift(n) - report_dropped_spans(n, reason: 'buffer-full', function: __method__.to_s) + dropped_spans = spans.shift(n) + report_dropped_spans(dropped_spans, reason: 'buffer-full', function: __method__.to_s) end spans << span @condition.signal if spans.size > batch_size @@ -122,8 +122,8 @@ def force_flush(timeout: nil) # rubocop:disable Metrics/MethodLength lock do n = spans.size + snapshot.size - max_queue_size if n.positive? - snapshot.shift(n) - report_dropped_spans(n, reason: 'buffer-full', function: __method__.to_s) + dropped_spans = snapshot.shift(n) + report_dropped_spans(dropped_spans, reason: 'buffer-full', function: __method__.to_s) end spans.unshift(snapshot) unless snapshot.empty? @condition.signal if spans.size > max_queue_size / 2 @@ -146,7 +146,7 @@ def shutdown(timeout: nil) thread&.join(timeout) force_flush(timeout: OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time)) - dropped_spans = lock { spans.size } + dropped_spans = lock { spans.shift(spans.length) } report_dropped_spans(dropped_spans, reason: 'terminating') if dropped_spans.positive? @exporter.shutdown(timeout: OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time)) end @@ -200,12 +200,12 @@ def report_result(result_code, batch) else OpenTelemetry.handle_error(exception: ExportError.new("Unable to export #{batch.size} spans")) @metrics_reporter.add_to_counter('otel.bsp.export.failure') - report_dropped_spans(batch.size, reason: 'export-failure') + report_dropped_spans(batch, reason: 'export-failure') end end - def report_dropped_spans(count, reason:, function: nil) - @metrics_reporter.add_to_counter('otel.bsp.dropped_spans', increment: count, labels: { 'reason' => reason, OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => function }.compact) + def report_dropped_spans(dropped_spans, reason:, function: nil) + @metrics_reporter.add_to_counter('otel.bsp.dropped_spans', increment: dropped_spans.size, labels: { 'reason' => reason, OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => function }.compact) end def fetch_batch From e5166d2a95c1f7eb16006334f78803f3911121b5 Mon Sep 17 00:00:00 2001 From: Sam Handler Date: Fri, 19 Jan 2024 14:10:53 -0700 Subject: [PATCH 10/10] cleanup --- sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb | 2 +- .../opentelemetry/sdk/trace/export/batch_span_processor_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb index c6312ed88d..b619d479be 100644 --- a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb +++ b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb @@ -147,7 +147,7 @@ def shutdown(timeout: nil) thread&.join(timeout) force_flush(timeout: OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time)) dropped_spans = lock { spans.shift(spans.length) } - report_dropped_spans(dropped_spans, reason: 'terminating') if dropped_spans.positive? + report_dropped_spans(dropped_spans, reason: 'terminating') if dropped_spans.any? @exporter.shutdown(timeout: OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time)) end diff --git a/sdk/test/opentelemetry/sdk/trace/export/batch_span_processor_test.rb b/sdk/test/opentelemetry/sdk/trace/export/batch_span_processor_test.rb index 04a749716e..d86def23ab 100644 --- a/sdk/test/opentelemetry/sdk/trace/export/batch_span_processor_test.rb +++ b/sdk/test/opentelemetry/sdk/trace/export/batch_span_processor_test.rb @@ -222,7 +222,7 @@ def to_span_data _(test_exporter.failed_batches.size).must_equal(0) _(test_exporter.batches.size).must_equal(0) - _(bsp.instance_variable_get(:@spans).size).must_equal(1) + _(bsp.instance_variable_get(:@spans).size).must_equal(0) end it 'works if the thread is not running' do