Skip to content
New issue

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

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

Already on GitHub? # to your account

Return errors from span processor creation #6488

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

yurishkuro
Copy link
Member

Which problem is this PR solving?

Description of the changes

  • Creation of exporterhelper may result in an error, so extend the processor construction signature to return the error.

How was this change tested?

  • CI

Signed-off-by: Yuri Shkuro <github@ysh.us>
Copy link

codecov bot commented Jan 6, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 96.25%. Comparing base (b1153a0) to head (f13f6d7).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
cmd/collector/app/collector.go 40.00% 2 Missing and 1 partial ⚠️
cmd/collector/app/span_processor.go 50.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6488      +/-   ##
==========================================
- Coverage   96.30%   96.25%   -0.06%     
==========================================
  Files         372      372              
  Lines       21277    21282       +5     
==========================================
- Hits        20491    20485       -6     
- Misses        601      609       +8     
- Partials      185      188       +3     
Flag Coverage Δ
badger_v1 10.67% <ø> (ø)
badger_v2 2.78% <ø> (ø)
cassandra-4.x-v1-manual 16.51% <ø> (ø)
cassandra-4.x-v2-auto 2.71% <ø> (ø)
cassandra-4.x-v2-manual 2.71% <ø> (ø)
cassandra-5.x-v1-manual 16.51% <ø> (ø)
cassandra-5.x-v2-auto 2.71% <ø> (ø)
cassandra-5.x-v2-manual 2.71% <ø> (ø)
elasticsearch-6.x-v1 20.18% <ø> (ø)
elasticsearch-7.x-v1 20.25% <ø> (-0.01%) ⬇️
elasticsearch-8.x-v1 20.40% <ø> (ø)
elasticsearch-8.x-v2 2.77% <ø> (-0.01%) ⬇️
grpc_v1 12.31% <ø> (ø)
grpc_v2 9.09% <ø> (ø)
kafka-3.x-v1 10.35% <ø> (ø)
kafka-3.x-v2 2.78% <ø> (ø)
memory_v2 2.78% <ø> (ø)
opensearch-1.x-v1 20.29% <ø> (ø)
opensearch-2.x-v1 20.30% <ø> (+<0.01%) ⬆️
opensearch-2.x-v2 2.77% <ø> (ø)
tailsampling-processor 0.51% <ø> (ø)
unittests 95.13% <50.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yurishkuro yurishkuro changed the title Returm errors from span processor creation Return errors from span processor creation Jan 6, 2025
@yurishkuro yurishkuro marked this pull request as ready for review January 6, 2025 00:58
@yurishkuro yurishkuro requested a review from a team as a code owner January 6, 2025 00:58
@yurishkuro yurishkuro requested a review from joe-elliott January 6, 2025 00:58
@@ -36,7 +36,7 @@ type SpanHandlers struct {
}

// BuildSpanProcessor builds the span processor to be used with the handlers
func (b *SpanHandlerBuilder) BuildSpanProcessor(additional ...ProcessSpan) processor.SpanProcessor {
func (b *SpanHandlerBuilder) BuildSpanProcessor(additional ...ProcessSpan) (processor.SpanProcessor, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@yurishkuro why are we adding error to the signature here if this function isn't returning an error? is there going to be a follow-up PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes - as explained in the PR description, introducing OTEL components will introduce errors

@yurishkuro yurishkuro merged commit 27af7b0 into jaegertracing:main Jan 6, 2025
54 of 56 checks passed
@yurishkuro yurishkuro deleted the processor branch January 6, 2025 03:00
adityachopra29 pushed a commit to adityachopra29/jaeger that referenced this pull request Jan 9, 2025
## Which problem is this PR solving?
- Part of jaegertracing#6487

## Description of the changes
- Creation of exporterhelper may result in an error, so extend the
processor construction signature to return the error.

## How was this change tested?
- CI

Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: adityachopra29 <adityachopra2912@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants