Skip to content

Apply new uri to resource normalization rules#928

Merged
labbati merged 16 commits intomasterfrom
labbati/curl-outgoing-resources
Jun 22, 2020
Merged

Apply new uri to resource normalization rules#928
labbati merged 16 commits intomasterfrom
labbati/curl-outgoing-resources

Conversation

@labbati
Copy link
Copy Markdown
Member

@labbati labbati commented Jun 19, 2020

Description

This PR provide a new method to customize uri to resource mapping.

Problem
Http requests that corresponds to URI GET http://example.com/user/123/permissions has a resource name in the format of GET /user/?/permissions. Identifying the variable part 123 and converting it into ? is what we call resource normalization. With resource normalization in place we make sure that requests to GET http://example.com/user/123/permissions and GET http://example.com/user/456/permissions are grouped under the very same resource name GET http://example.com/user/?/permissions in our UI.

We adopt 2 strategies to normalize GET /user/123/permissions into GET /user/?/permissions:

  • known patterns for path fragments (e.g. digits, uuids)
  • routing info we extract from we frameworks we have deep instrumentation for.

There are 2 use cases that the above strategies do not fully cover:

  • custom web frameworks with non common resource identifiers (anything other then digits and uudis)
  • web frameworks for which we haven't implemented routing parsing, yet.

Assume you have the following set of uris

You have You want
/always/using/<7 CHARS HEX>/as/id/everywhere /always/using/?/as/id/everywhere
/cities/london/rivers /cities/?/rivers
/articles/slug-of-title /articles/?

This PR offers the possibility to cover the remaining cases above via new settings.

DD_TRACE_RESOURCE_URI_FRAGMENT_REGEX: lets you provide a list of regex that are applied to every FULL path fragment. By FULL path fragment we mean what is inside to / chars in a uri path. E.g. in /cities/london/rivers we have three path fragments: cities, london, rivers. A list of regexes can be provided separated by comma ,. With env you would address the first use case. DD_TRACE_RESOURCE_URI_FRAGMENT_REGEX=^[a-f0-9]{7}$ will find and replace ALL path fragments that are a 7 chars HEX word with ?.
This setting applies to both incoming and outgoing requests.

DD_TRACE_RESOURCE_URI_MAPPING_INCOMING: lets you cover the cases when you cannot provide a regex (e.g. to normalize london and rome) but you know that a given identifier appears always after a known word, e.g. in this case cities. In this case you would normalize the latter two examples in the table above via DD_TRACE_RESOURCE_URI_MAPPING_INCOMING=cities/*,articles/*. Note that in this case the provided values are NOT regexes, but they recognize the * replacement char.

DD_TRACE_RESOURCE_URI_MAPPING_OUTGOING: Same as DD_TRACE_RESOURCE_URI_MAPPING_INCOMING but for outgoing requests.

*NOTE: THIS PR DEPRECATES SETTING DD_TRACE_RESOURCE_URI_MAPPING

Readiness checklist

  • (only for Members) Changelog has been added to the appropriate release draft. Create one if necessary.
  • Tests added for this feature/bug.

Reviewer checklist

  • Appropriate labels assigned.
  • Milestone is set.
  • Changelog has been added to the appropriate release draft. For community contributors the reviewer is in charge of this task.

Comment thread bridge/configuration.php
return array_map(
function ($entry) {
return strtolower(trim($entry));
return trim($entry);
Copy link
Copy Markdown
Member Author

@labbati labbati Jun 19, 2020

Choose a reason for hiding this comment

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

This is a generic getter and should not make assumptions about case sensitive vs case insensitive. Also, this function was not used by other settings so it is good to be changed as it is only used by the newly added functions.

@labbati labbati added this to the 0.47.0 milestone Jun 19, 2020
@labbati labbati marked this pull request as ready for review June 19, 2020 12:50
@labbati labbati added the 🏆 enhancement A new feature or improvement label Jun 19, 2020
Copy link
Copy Markdown
Contributor

@SammyK SammyK left a comment

Choose a reason for hiding this comment

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

Looks great @labbati! I just left two questions.

Comment thread src/private/functions.php Outdated
Comment thread tests/Unit/private/UriTest.php Outdated
Comment thread src/private/functions.php
Comment thread tests/Unit/private/UriTest.php
Copy link
Copy Markdown
Contributor

@SammyK SammyK left a comment

Choose a reason for hiding this comment

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

Nice work @labbati! 💯

@labbati labbati merged commit 16abc20 into master Jun 22, 2020
@labbati labbati deleted the labbati/curl-outgoing-resources branch June 22, 2020 15:00
@morrisonlevi morrisonlevi mentioned this pull request Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏆 enhancement A new feature or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants