Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add context to metrics reporting of buffer-full events #1566

Merged
merged 12 commits into from
Jan 22, 2024
Merged

Add context to metrics reporting of buffer-full events #1566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ee810ca
add context to metrics reporting of buffer-full events
plantfansam Jan 16, 2024
fec17cf
Update sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb
plantfansam Jan 17, 2024
03fb66c
Use semantic convention for code function label
plantfansam Jan 17, 2024
dfab14e
Kwargs are better; use kwargs
plantfansam Jan 17, 2024
452f96a
stray paren
plantfansam Jan 17, 2024
bc427b7
Merge branch 'main' into buffer-full-context
plantfansam Jan 19, 2024
68fe040
use __method__ to access current method
plantfansam Jan 19, 2024
6a3c754
code better
plantfansam Jan 19, 2024
6b17880
method to string
plantfansam Jan 19, 2024
40e5e89
pass spans as arg to report_dropped_spans
plantfansam Jan 19, 2024
e5166d2
cleanup
plantfansam Jan 19, 2024
ae04ce1
Merge branch 'main' into buffer-full-context
fbogsany Jan 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')
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
Expand Down Expand Up @@ -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')
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
Expand All @@ -146,8 +146,8 @@ def shutdown(timeout: nil)

thread&.join(timeout)
force_flush(timeout: OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time))
dropped_spans = lock { spans.size }
report_dropped_spans(dropped_spans, reason: 'terminating') if dropped_spans.positive?
dropped_spans = lock { spans.shift(spans.length) }
report_dropped_spans(dropped_spans, reason: 'terminating') if dropped_spans.any?
@exporter.shutdown(timeout: OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time))
end

Expand Down Expand Up @@ -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:)
@metrics_reporter.add_to_counter('otel.bsp.dropped_spans', increment: count, labels: { 'reason' => reason })
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
Expand Down
2 changes: 1 addition & 1 deletion sdk/test/opentelemetry/sdk/trace/export/batch_span_processor_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are now properly dropping spans during shutdown we change the test expectation

end

it 'works if the thread is not running' do
Expand Down
Loading