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

add railtie based RUM Rack instrumentation #26

Merged
merged 12 commits into from
Jul 19, 2022
Merged

Conversation

tsloughter-splunk
Copy link
Contributor

No description provided.

@tsloughter-splunk
Copy link
Contributor Author

I did the same exporter stuff as the rack test but it fails in rails_test.

instead of creating a new exporter in RumRailsTest and
RackRailsTest have EXPORTER instance created globally
and reset it in the teardown method of the testcases.

It may be useful to figure out in the future why creating
a new exporter in each testcase was resulting in no spans
making it to the exporter in the second test to run.
class Railtie < ::Rails::Railtie
# TODO: use an explicit ordering to be after the Otel middleware
# instead of just adding it to the end of the list of middlewares
config.before_initialize do |app|
Copy link
Contributor

@jtmal-signalfx jtmal-signalfx Jul 14, 2022

Choose a reason for hiding this comment

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

          config.before_initialize do |app|
            app.middleware.insert_before(
              ActionDispatch::ServerTiming,
              Splunk::Otel::Rack::RumMiddleware
            )
          end

the order is reversed. ActionDispatch::ServerTiming will overwrite server-timing, so we need to append after to their values.

@jtmal-signalfx
Copy link
Contributor

Added a couple of comments, I think that documentation would be good to have, even if it's very basic, e.g. without any troubleshooting tips.

@jtmal-signalfx jtmal-signalfx merged commit 3c45033 into main Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants