-
Notifications
You must be signed in to change notification settings - Fork 134
Grape support #562
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
Grape support #562
Conversation
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.
Code looks good!
We also need to add the public api (and the Rails.start
one too) to docs/api.asciidoc
btw. And a changelog entry.
930bea4
to
ed2709a
Compare
Codecov Report
@@ Coverage Diff @@
## master #562 +/- ##
==========================================
- Coverage 94.95% 94.84% -0.11%
==========================================
Files 87 87
Lines 2695 2696 +1
==========================================
- Hits 2559 2557 -2
- Misses 136 139 +3
Continue to review full report at Codecov.
|
docs/configuration.asciidoc
Outdated
@@ -22,7 +22,7 @@ secret_token: <%= ENV["VERY_SECRET_TOKEN"] %> | |||
Options are applied in the following order (last one wins): | |||
|
|||
1. Defaults | |||
2. Arguments to `ElasticAPM.start` / `Config.new` | |||
2. Arguments to `ElasticAPM.start` / `ElasticAPM::Grape.start` / `ElasticAPM::Rails.start` / `Config.new` |
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.
Could Arguments to .start(…)
be explanatory enough? I'm afraid mentioning too many different frameworks here will confuse more than help?
dcaafb7
to
104e15a
Compare
@mikker Since your approval, I've made significant changes to this PR. It'd be great if you could review the following:
|
41f1c1a
to
75673ce
Compare
170052c
to
f81df98
Compare
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.
This is good to go 👍
4b6651d
to
42d30e4
Compare
This PR adds support for Grape. The changes are:
Grape.start
method so that the agent can be hooked up to a Grape appendpoint_run
Grape.start
Further work:
Maybe add normalizerformat_response
Consider not instrumenting the filter and validator endpoints, as they can be very noisy. There are a lot of internal filters and validators called and there's no way to distinguish between library and user-specified ones.Todo:
Closes #206, #321