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

Use AbstractMiddleware for Rails middleware #1107

Merged
merged 5 commits into from
Jun 27, 2024
Merged

Use AbstractMiddleware for Rails middleware #1107

merged 5 commits into from
Jun 27, 2024

Commits on Jun 25, 2024

  1. Fix private API declarations 

    Define what is a private API on the classes themselves, and not the
parent module for Rack middlewares.
    @tombruijn
    tombruijn committed Jun 25, 2024
    Configuration menu
    Copy the full SHA
    6f92d80 View commit details
    Browse the repository at this point in the history

  2. Guard against request method throwing an error 

    We've seen in the Rails instrumentation that the
`ActionDispatch::Request` class can raise an error on an invalid request
method type. Guard against it and don't try to set a value on error.
    @tombruijn
    tombruijn committed Jun 25, 2024
    Configuration menu
    Copy the full SHA
    0bb2980 View commit details
    Browse the repository at this point in the history

  3. Add a test for setting custom params 

    Make sure the AbstractMiddleware allows apps to set custom params, and
that they're not overwritten by the middleware.
    @tombruijn
    tombruijn committed Jun 25, 2024
    Configuration menu
    Copy the full SHA
    df96f2c View commit details
    Browse the repository at this point in the history

  4. Use AbstractMiddleware for Rails middleware 

    Use our new AbstractMiddleware as a basis for the Rails instrumentation
middleware. This moves all responsibility for handling nested
instrumentation middleware to the AbstractMiddleware.

The Rails middleware can now focus on only the part it needs to do: the
action name and some additional metadata.

The RailsInstrumentation middleware previously didn't create an event,
but the AbstractMiddleware does do that now. It will report a new
`middleware.rails` event now.

I didn't add a changeset for this change as it's mostly an internal
refactor.
    @tombruijn
    tombruijn committed Jun 25, 2024
    Configuration menu
    Copy the full SHA
    487330d View commit details
    Browse the repository at this point in the history

  5. Fix error reporting for Rails instrumentation 

    Updating the RailsInstrumentation middleware to inherit from the
AbstractMiddleware broke the error reporting for the middleware.
It relied on the EventHandler to report errors, but before it gets
there, Rails's exception handling middleware rescues the error and shows
an error page instead.

Add a config option to the AbstractMiddleware to configure if it should
report errors or not. Enable it for the RailsInstrumentation to restore
the error reporting functionality.
    @tombruijn
    tombruijn committed Jun 25, 2024
    Configuration menu
    Copy the full SHA
    0cba389 View commit details
    Browse the repository at this point in the history