-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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.
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
remove ErrorBusy, it essentially duplicates SpansDropped #1091
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay in review, but looks like there are build errors that need to be addressed, and you will need to rebase your PR.
cmd/collector/app/span_processor.go
Outdated
@@ -141,8 +140,6 @@ func (sp *spanProcessor) enqueueSpan(span *model.Span, originalFormat string) bo | |||
span: span, | |||
} | |||
addedToQueue := sp.queue.Produce(item) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The addedToQueue
is no longer required, so could just have return sp.queue.Produce(item)
Signed-off-by: Callum Styan <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1091 +/- ##
======================================
Coverage 100% 100%
======================================
Files 162 162
Lines 7263 7259 -4
======================================
- Hits 7263 7259 -4
Continue to review full report at Codecov.
|
@objectiser sorry for the delay. Removed |
@yurishkuro Are you ok with this duplicate metric being removed? If so, we just need to list it as a breaking change for next release. |
Fixes #1065
Signed-off-by: Callum Styan [email protected]
Which problem is this PR solving?
Removes ErrorBusy. Having not looked at the code before, I assume that if a span can't be added to the queue it is just dropped (hence SpansDropped). If that's the case, then ErrorBusy would duplicate SpansDropped but the name provides less context.
Short description of the changes
Removes ErrorBusy.