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.

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

Couldn't catch exception from Rails controller #635

Closed
xuan-cao-swi opened this issue Aug 28, 2023 · 6 comments
Closed

Couldn't catch exception from Rails controller #635

xuan-cao-swi opened this issue Aug 28, 2023 · 6 comments
Labels
bug Something isn't working keep Ensures stale-bot keeps this issue/PR open

Comments

@xuan-cao-swi
Copy link
Contributor

Description of the bug

If rails controller has error, then there is no exception_event from span.

For example, below is a sample rails controller, and create_error is not defined from User, so it should create error

class UsersController < ApplicationController
  # curl http://0.0.0.0:8002/users/new
  def new
    @user = User.new
    @user.create_error
  end
end

The sample span from above call

span_data: #<struct OpenTelemetry::SDK::Trace::SpanData name="UsersController#new", 
kind=:server, status=#<OpenTelemetry::Trace::Status:0x0000ffffa17e7588 @code=2, @description="">,
parent_span_id="\x00\x00\x00\x00\x00\x00\x00\x00", total_recorded_attributes=12, total_recorded_events=0,
total_recorded_links=0, start_timestamp=1693241683805542134, end_timestamp=1693241684087133009, 
attributes={"http.method"=>"GET", "http.host"=>"0.0.0.0:8002", "http.scheme"=>"http", "http.target"=>"/users/new",
 "http.user_agent"=>"curl/7.74.0", "code.namespace"=>"UsersController", "code.function"=>"new", 
"http.status_code"=>500}, links=nil, events=nil, resource=#<OpenTelemetry::SDK::Resources::Resource:0x0000ffffa4c8bf50 
@attributes={"service.name"=>"unknown_service", "process.pid"=>151, "process.command"=>"bin/rails", 
"process.runtime.name"=>"ruby", "process.runtime.version"=>"3.1.0", "process.runtime.description"=>"ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [aarch64-linux]", 
"telemetry.sdk.name"=>"opentelemetry", "telemetry.sdk.language"=>"ruby", "telemetry.sdk.version"=>"1.2.0"}>, 
instrumentation_scope=#<struct OpenTelemetry::SDK::InstrumentationScope 
name="OpenTelemetry::Instrumentation::Rack", version="0.22.1">, span_id="F\x7F#\xA7\x80D\e%", 
trace_id="\x83\x14\n\x9D\xB68M\x9B\x00\xD3x\xFE\xDF\xE0)0", trace_flags=#<OpenTelemetry::Trace::TraceFlags:0x0000ffffa5c08bb8 @flags=1>, 
tracestate=#<OpenTelemetry::Trace::Tracestate:0x0000ffffa4bd91c0 @hash={"sw"=>"0000000000000000-01"}>>

Even though the status_code is 500, but there is no exception.

I checked the opentelemetry-instrumentation-rack between L81-L82, there is no record_exception even the status is 500. I added request_span.record_exception(Exception.new) if status == 500 between 81 and 82, and rack span can create exception_event.

I tried to produce error from action_view, and the exception was caught in span.

I am wondering is there particular reason or the error should be handled in somewhere else?

Similar issue I found is #56

Share details about your runtime

Operating system details: Debian GNU/Linux aarch64 GNU/Linux
RUBY_ENGINE: "ruby"
RUBY_VERSION: "3.1.0"
RUBY_DESCRIPTION: "ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [aarch64-linux]"

Share a simplified reproduction if possible

opentelemetry-version

opentelemetry-api (1.2.2)
opentelemetry-common (0.19.6)
opentelemetry-instrumentation-action_pack (0.5.0)
opentelemetry-instrumentation-action_view (0.4.0)
opentelemetry-instrumentation-active_job (0.4.0)
opentelemetry-instrumentation-active_model_serializers (0.19.1)
opentelemetry-instrumentation-active_record (0.5.0)
opentelemetry-instrumentation-active_support (0.3.0)
opentelemetry-instrumentation-all (0.31.0)
opentelemetry-instrumentation-rack (0.22.1)
opentelemetry-instrumentation-rails (0.25.0)
opentelemetry-registry (0.3.0)
opentelemetry-sdk (1.2.0)
opentelemetry-semantic_conventions (1.10.0)
@arielvalentin
Copy link
Collaborator

Thanks for opening this issue.

Would you be interested in opening a PR to add functionality that records handled errors on the rack spans?

@xuan-cao-swi
Copy link
Contributor Author

Would love to. Just need to find right place to throw the exception.
It seems like from opentelemetry-instrumentation-rack call doesn't give any useful exception object like sinatra's sinatra.error

@arielvalentin
Copy link
Collaborator

You may want to have a look at dd-trace-rb or other vendor specific implementations to see if they figured out some way of dealing with this scenario.

I assert that it's one of the Rails libraries that is capturing the exception and reporting it back to rack in some way.

I think that also varies in what mode the rails app is running in, e.g. in development ActionDispatch::DebugExceptions intercepts the raised exception and shows a debugging page.

Error handlers like airbrake or sentry may intercept them and handle them in a different way.

Application developers may also use rescue_from and choose to suppress the exception altogether and they will be difficult to intercept unless rails exposes a hook of some sort.

Please let us know what you find out and thank you again for being willing to help.

@cheempz
Copy link
Contributor

cheempz commented Aug 29, 2023

If the application is handling the exception (whether using rescue_from, rails mode or third party library), IMHO it's a lesser use case where we don't require the instrumentation to report it. The main use case that got us looking into this is an unhandled exception, where we are able to show the HTTP status 5xx but unable to provide more details on the error.

@github-actions
Copy link
Contributor

👋 This issue has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this issue will be closed eventually by the stale bot.

@github-actions github-actions bot added the stale Marks an issue/PR stale label Sep 29, 2023
@arielvalentin arielvalentin added keep Ensures stale-bot keeps this issue/PR open and removed stale Marks an issue/PR stale labels Sep 29, 2023
@xuan-cao-swi
Copy link
Contributor Author

resolved in #703

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working keep Ensures stale-bot keeps this issue/PR open
Projects
None yet
Development

No branches or pull requests

3 participants