Add ignore_urls to central config #872
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
|
There was a problem hiding this comment.
Looking good!
A couple of additional bits are needed:
- In
newTracer, another call tot.setLocalInstrumentationConfigis needed to set the initial local config. This is required so if remote config is set, and then later unset, the tracer will revert to the local config. - In
config_test.go, add a subtest to TestTracerCentralConfigUpdate for "transaction_ignore_urls"
module/apmhttp/ignorer.go
Outdated
| @@ -53,6 +56,15 @@ func DefaultServerRequestIgnorer() RequestIgnorerFunc { | |||
| return defaultServerRequestIgnorer | |||
| } | |||
|
|
|||
| // DynamicServerRequestIgnorer returns the RequestIgnorer to use in | |||
| // handlers. The list of wildcard patterns comes from central config | |||
| func DynamicServerRequestIgnorer(t *apm.Tracer) RequestIgnorerFunc { | |||
There was a problem hiding this comment.
| func DynamicServerRequestIgnorer(t *apm.Tracer) RequestIgnorerFunc { | |
| func NewDynamicServerRequestIgnorer(t *apm.Tracer) RequestIgnorerFunc { |
Let's align the naming with the other functions below, since DefaultServerRequestIgnorer will be going away eventually.
There was a problem hiding this comment.
In fact, if you implement the suggestion above about changing Tracer.IgnoredTransactionURLMatchers to IgnoredTransactionURL, then we can just pass tracer.IgnoredTransactionURL directly in as it will satisfy the function signature -- no need for (New)DynamicServerRequestIgnorer then.
There was a problem hiding this comment.
I realise now that RequestIgnorerFunc takes an *http.Request, whereas IgnoredTransactionURL takes a URL, so my second comment doesn't work. But let's please align the naming, and add New to the front.
module/apmhttp/ignorer.go
Outdated
| @@ -53,6 +56,15 @@ func DefaultServerRequestIgnorer() RequestIgnorerFunc { | |||
| return defaultServerRequestIgnorer | |||
| } | |||
|
|
|||
| // DynamicServerRequestIgnorer returns the RequestIgnorer to use in | |||
| // handlers. The list of wildcard patterns comes from central config | |||
| func DynamicServerRequestIgnorer(t *apm.Tracer) RequestIgnorerFunc { | |||
There was a problem hiding this comment.
I realise now that RequestIgnorerFunc takes an *http.Request, whereas IgnoredTransactionURL takes a URL, so my second comment doesn't work. But let's please align the naming, and add New to the front.
Co-authored-by: Andrew Wilkins <axwalk@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #872 +/- ##
=======================================
Coverage 83.70% 83.70%
=======================================
Files 142 142
Lines 7934 7959 +25
=======================================
+ Hits 6641 6662 +21
- Misses 885 889 +4
Partials 408 408
Continue to review full report at Codecov.
|
Closes #814