-
Notifications
You must be signed in to change notification settings - Fork 169
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
fix: add exceptions to sinatra spans. #133
fix: add exceptions to sinatra spans. #133
Conversation
edit: the screenshot is not accurate anymore; as of now a span is marked as in error with addition of the exception to the span whenever it has a server error, HTTP > 500 |
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.
Thanks for your contribution! Please see my comments about limiting what exceptions we record.
...mentation/sinatra/lib/opentelemetry/instrumentation/sinatra/middlewares/tracer_middleware.rb
Outdated
Show resolved
Hide resolved
instrumentation/sinatra/test/opentelemetry/instrumentation/sinatra_test.rb
Outdated
Show resolved
Hide resolved
instrumentation/sinatra/test/opentelemetry/instrumentation/sinatra_test.rb
Outdated
Show resolved
Hide resolved
...mentation/sinatra/lib/opentelemetry/instrumentation/sinatra/middlewares/tracer_middleware.rb
Show resolved
Hide resolved
…atra_test.rb Co-authored-by: Ariel Valentin <arielvalentin@users.noreply.github.com>
…atra_test.rb Co-authored-by: Ariel Valentin <arielvalentin@users.noreply.github.com>
…s/opentelemetry-ruby-contrib into 56-add-sinatra-exceptions
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.
LGTM!
This will add any caught Sinatra exceptions to the Span.
Fixes: #56
(edit: updated screenshot so that it correctly reflects outcome of this PR: only server side errors will mark a span as in error and attach an exception to the span.)