Skip to content
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

[fix/mashape-analytics] ALF serializer and Buffer #425

Merged
merged 8 commits into from
Jul 29, 2015

Conversation

thibaultcha
Copy link
Member

Addressing various issues reported in #423

  • more descriptive logging in error.log
  • clientIPAddress track client IP in analytics plugin #424
  • fix an issue when log_body was true but the request had no body (GET
    request for example)
  • fix an issue when nginx is retrying against upstream after a timeout
    and resulting in multiple upstream_response_time values
  • more robust buffering (payload size and better flushing)
  • don't buffer aborted requests (refused by analytics anyways)

Also handle an edgecase where `res` was being accessed while it was nil.
- fix an issue when `log_body` was true but the request had no body (GET
  request for example)
- fix an issue when nginx is retrying against upstream after a timeout
  and resulting in multiple `upstream_response_time` values

  addressing #423
@thibaultcha thibaultcha added the pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release) label Jul 23, 2015
The refactor was initially due to the fact that the ALF Serializer
couldn't have the `clientIPAddress` at the root level. Thus the plugin
doesn't send one ALF with n entries but n ALFs instead.

Fix an edge-case when some ALFs could be sent several times to the
socket server. The only way to solve this is to have each buffer (one
per API) sending itself its own data and be aware of what exactly has
been received by the socket server already and what hasn't.
@thibaultcha thibaultcha force-pushed the fix/mashape-analytics branch from 6bb5068 to 97644ea Compare July 28, 2015 23:00
- buffer now watches its size (bytes) to avoid getting bigger than
analytics's payload limit.
- buffer is now responsible for sending its data
- buffer will queue batches to be sent and those will never exceed the
  configured batch maximum size or socket maximum payload size
@thibaultcha thibaultcha force-pushed the fix/mashape-analytics branch from 97644ea to ca68324 Compare July 28, 2015 23:05
@thibaultcha thibaultcha changed the title [fix/mashape-analytics] ALF serializer [fix/mashape-analytics] ALF serializer and Buffer Jul 28, 2015
thibaultcha added a commit that referenced this pull request Jul 29, 2015
[fix/mashape-analytics] ALF serializer and Buffer
@thibaultcha thibaultcha merged commit 086c7dd into master Jul 29, 2015
@thibaultcha thibaultcha deleted the fix/mashape-analytics branch July 29, 2015 00:29
ctranxuan pushed a commit to streamdataio/kong that referenced this pull request Aug 25, 2015
[fix/mashape-analytics] ALF serializer and Buffer

Former-commit-id: a73eb4470595441affa717f8207cc523a62a2b88
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants