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

Add query-specific uuid to enable finite metrics. #1510

Merged
merged 9 commits into from Jun 17, 2016

Conversation

Projects
None yet
2 participants
@funnylookinhat
Copy link
Contributor

funnylookinhat commented Jun 17, 2016

When trying to profile queries throughout an application, binding to query, query-response, and query-error isn't quite as useful as it could be because there's no way to uniquely identify individual queries that are sent off to the connection.

This adds a (hopefully appropriately) named field of __knexQueryUid to both raw and query-builder queries.

For example:

knex.on('query', function (obj) {
  // Start profiling obj.__knexQueryUid
});
knex.on('query-response', function (res, obj) {
  // Record metrics for obj.__knexQueryUid
});
knex.on('query-error', function (err, obj) {
  // Record metrics for obj.__knexQueryUid
});

This should hopefully resolve this feature request: #1502

I didn't see any tests specific to the events that are emitted when running queries - so I'm not sure if/where I should add tests for this PR.

@wubzz

This comment has been minimized.

Copy link
Collaborator

wubzz commented Jun 17, 2016

The tests for the events are awkwardly placed here, in case you wanted to write some.

@funnylookinhat

This comment has been minimized.

Copy link
Contributor Author

funnylookinhat commented Jun 17, 2016

@wubzz Ah thanks :) I've added what I think should be necessary now.

@funnylookinhat

This comment has been minimized.

Copy link
Contributor Author

funnylookinhat commented Jun 17, 2016

@wubzz Let me know if you have any feedback. Sorry to spam the PR - but I couldn't get the tests working locally so I just abused Travis. :-P

@wubzz wubzz merged commit acbc8a1 into tgriesser:master Jun 17, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

SuperSpyTX added a commit to SuperSpyTX/knex that referenced this pull request Jun 19, 2016

Fix dependency issue with 'node-uuid'
It appears that in Issue tgriesser#1510, unique IDs were added to queries.  The dependency that was used was a development dependency, which isn't installed as a default.  This change makes sure the dependency is installed by default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.