search

Kong / galileo-agent-node

Node.js agent for Galileo - API Analytics
https://getgalileo.io
MIT License
9 stars 4 forks source link

Agent Spec Compliance #25

Closed therebelrobot closed 8 years ago

therebelrobot commented 8 years ago

Building off #24, this brings the agent up to galileo-agent-spec as of April 1 2016 1pm PT

changes

type scope description
refactor alf spec refactored agent to ALF v1.1.0
refactor alf spec made optional portions of the alf actually optional
feat queue added flush timer
feat queue added retry and write errors to disk
feat queue added check to see if another flush is running already
fix bodies made sure to not convert to string before base64ing
fix clientIPAddress making sure to capture correct IP
fix fail log made fail logs single line json entries
fix request bodies ensured body capture
misc examples updated examples with new naming
misc travis added node 4 and 5 for travis testing
test unit replaced tests with tap
test unit getting tests most of the way there
test unit cleanup - logs, commented code, etc
nijikokun commented 8 years ago

On line 82 in helpers.js we see a variable being initialized every loop:

      var header = this.parseHeaderLine(lines[i])

we should initialize outside and assign

nijikokun commented 8 years ago

You want to avoid using for x in y because it's simply not optimized the variable is initialized every loop and you cannot avoid doing so:

https://github.com/petkaantonov/bluebird/wiki/Optimization-killers#51-the-key-is-not-a-local-variable

as seen on line 159 of helpers.js

nijikokun commented 8 years ago 
/Users/user/git/galileo-agent-node/lib/queue.js:190
      responses.push(err.message)
               ^
TypeError: Cannot read property 'push' of undefined
    at ClientRequest.<anonymous> (/Users/user/git/galileo-agent-node/lib/queue.js:190:16)
    at ClientRequest.emit (events.js:107:17)
    at Socket.socketErrorListener (_http_client.js:271:9)
    at Socket.emit (events.js:107:17)
    at net.js:459:14
    at process._tickCallback (node.js:355:11)
therebelrobot commented 8 years ago

The three comments above are taken care of in that last commit (if they were listed in the compare rather than comments they'd collapse :P )