feat: transaction_ignore_urls wildcard matching#1876
Merged
Conversation
astorm
commented
Nov 17, 2020
8706e62 to
a85320a
Compare
bmorelli25
reviewed
Nov 17, 2020
Member
bmorelli25
left a comment
There was a problem hiding this comment.
Nice! One small typo, otherwise docs LGTM 🎉
9830a26 to
cd82d99
Compare
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
|
trentm
requested changes
Nov 17, 2020
f8911e9 to
887a4fc
Compare
trentm
approved these changes
Nov 19, 2020
Member
trentm
left a comment
There was a problem hiding this comment.
Just a spelling typo in the docs and a typo. Soft approving.
|
|
||
| Array or comma-separated string used to restrict requests for certain URLs from being instrumented. | ||
|
|
||
| This property should be set to an array containing one or more string patterns. |
Member
There was a problem hiding this comment.
nit: Could probably drop this sentence given the one before it?
| When an incoming HTTP request is detected, its URL pathname will be tested against each element | ||
| in this list. The `transactionIgnoreUrls` property supports exact string matches, | ||
| simple wildcard (`*`) matches, and may not include commas. Wildcard matches are | ||
| case insensative by default. You may make wildcard searches case sensative by |
Member
There was a problem hiding this comment.
Suggested change
| case insensative by default. You may make wildcard searches case sensative by | |
| case-insensitive by default. You may make wildcard searches case-sensitive by |
| */ | ||
| const escapeStringRegexp = require('escape-string-regexp') | ||
|
|
||
| // coverts elastic-wildcard pattern into a |
Member
There was a problem hiding this comment.
nit:
Suggested change
| // coverts elastic-wildcard pattern into a | |
| // Converts elastic-wildcard pattern into a |
Contributor
Author
|
Merging, and will handle the docs typos in a quick follow-on PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Will Fix: #1689
Implements
ELASTIC_TRANSACTION_IGNORE_URL/transactionIgnoreUrlsconfiguration for wildcard URL ignoring. The intent of this new configuration is to gives users the ability to skip generating transactions for a request to a URL that matches an exact string, or a URL that matches simple wildcard pattern. This wildcard pattern will behave consistently across language agents.This PR leaves the existing
ignoreUrlsconfiguration in place to allow users access to more powerful regular expression matching and to preserve existing functionality for users using this older configuration setting.This PR also introduces a new dependency,
escape-string-regexp. In order to implement the suggested wildcard matching algorithm we needed the ability to escape a string for use in a regular expression. Since Node.js doesn't have this functionality natively, I pulled inescape-string-regexprather than attempt to write (easy) and maintain (harder) this functionality ourselves. Happy to revisit this decision if we'd rather implement this functionality ourselves.Open items for consideration/completion
escape-string-regexpan acceptable dependency for ignore regular expressions?/be assumed? (we'll prepend this, meaning a configured value of 'foo*' and '/foo*' will match the same)(?-i)and(?+i)prefixes across agentsisRequestBlacklistedis only called ininstrumentRequest, andinstrumentRequestis only used when wrapping http(s).Server'semitmethod (i.e. it's used in transaction starting)Checklist