-
Notifications
You must be signed in to change notification settings - Fork 181
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
Customising Rack instrumentation inside a Grape service #1011
Comments
cc: @muripic |
What is the current sentiment of forwarding config options? If grape is taking responsibility for installing rack instrumentation, can we also supply rack config opts through grape? Possible no code change alternatively, have you tried configuring rack via env vars? If the env vars are present at install time it should still apply them whether its the SDK installing it or grape. |
@robertlaurin Yes, using envars is the most reliable way to configure the app but we do not do a good job of documenting how to customize the instrumentations using envars on Using "nested" configurations becomes complicated in applications where you have multiple frameworks involved all trying to configure the Rack instrumentation, including auto-inserting the Rack middleware. In my case, the ActionPack and Sinatra instrumentation compete with each other in the GitHub monolith to insert the Rack middleware. I have a hack here to have Sinatra disable Rack installation so I have a PR here to actually optionally disable the installation: This has come up several times before and it may be resolved by either introducing some sort of dependency tree or ordering in the Registry to ensure no matter the order you declare the instrumentations in the DSL, the registry installs them in the appropriate order with the correct configuration options. |
@arielvalentin a similar change to the grape instrumentation would also help my use case as we're often mounting Grape within rails. I can see about raising a PR for this. |
@arielvalentin, I've raised this PR for it: #1043 |
@chrisholmes - Does #1043 resolve this bug or is there more that needs to be done? |
👋 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 |
hi @kaylareopelle, I missed your message. #1043 works for us. |
@chrisholmes, Great to hear! I'll close this issue. |
Description of the bug
When using a Grape service it is not possible to customise the behaviour of the rack instrumentation. This is due to the load order of instrumentations (alphabetical) and the approach used by the grape, rails, and sinatra web frameworks to initialise the rack instrumentation with default parameters. When we attempt to load the rack instrumentation later with custom parameters it returns early as the instrumentation was loaded already with default configruation by grape's instrumentation.
This is problematic for us as we'd likely to use the rack instrumentation configuration to suppress HTTP healthcheck spans and potentially other features.
This problem doesn't occur with the rails or sinatra instrumentation as they're loaded after the rack instrumentation becuase of the alphabetical ordering.
I'm can think of a few options for solutions, but I'd like to hold off to see what the manitainers think.
Share details about your runtime
Applies to all runtimes
Share a simplified reproduction if possible
Can share if required, but the simplest version is a change to add a configuration to rack here: https://github.com/open-telemetry/opentelemetry-ruby-contrib/blob/main/instrumentation/grape/example/trace_demonstration.rb#L18
The text was updated successfully, but these errors were encountered: