This repository was archived by the owner on Nov 29, 2021. It is now read-only.
Add handling for keyword fields with a maximum length of 1024 characters #103
Conversation
This file contains 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
mb_substr is used to perform substring extraction; ElasticSearch uses UTf-8 exclusively, so this is the encoding given to mb_substr. In addition, in order to check string length, strlen is first used (it is constant time), and only if strlen exceeds the character limit is mb_substr used (which is linear time). The keyword handing is added to most of the locations it is found in the Python agent (https://github.com/elastic/apm-agent-python/search?q=keyword_field&unscoped_q=keyword_field); a few are omitted where it doesn't seem to make any reasonable sense (e.g. the PHP version, and exceptions codes; while both technically can exceed the character limit, they should really only do so in extremely atypical situations). The keyword handling was also omitted from tag names (simply because I wasn't 100% confident in modifying that code).
philkra
suggested changes
Oct 14, 2019
| /* | ||
| * Functions to convert values for transmission to ElasticSearch. | ||
| */ | ||
| class Encoding |
There was a problem hiding this comment.
please add test for this class, thanks in advance
There was a problem hiding this comment.
I've added PhilKra\Tests\Helper\EncodingTest with commit 8de0b22. Let me know if this looks good.
Regards,
JTP
|
thank you for your contribution 💪 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
When the ElasticSearch APM agent sends values for certain fields that are longer than 1024 characters, an exception is thrown:
The particular implementation here is as follows:
mb_substr is used to perform substring extraction; ElasticSearch uses UTF-8 exclusively, so this is the encoding given to mb_substr. In addition, in order to check string length, strlen is first used (it is constant time), and only if strlen exceeds the character limit is mb_substr used (which is linear time).
The keyword handing is added to most of the locations it is found in the Python agent (https://github.com/elastic/apm-agent-python/search?q=keyword_field&unscoped_q=keyword_field); a few are omitted where it doesn't seem to make any reasonable sense (e.g. the PHP version, and exceptions codes; while both technically can exceed the character limit, they should really only do so in extremely atypical situations).
The keyword handling was also omitted from tag names (simply because I wasn't 100% confident in modifying that code).