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 :metrics option to capture headers #625

Merged
merged 24 commits into from
Mar 29, 2022
Merged

Add :metrics option to capture headers #625

merged 24 commits into from
Mar 29, 2022

Conversation

elersong
Copy link
Contributor

@elersong elersong commented Mar 24, 2022

Notes

Current limitations of the JS driver prevent easy access to usage metrics provided in the response headers from the DB. This PR adds a :metrics option to allow programmatic access to those metrics.

How to test

yarn test

src/_util.js Outdated Show resolved Hide resolved
Copy link
Contributor

@faunaee faunaee left a comment

Choose a reason for hiding this comment

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

Nice! Many JS driver users will appreciate this feature. I made a few suggestions.

src/Client.js Outdated Show resolved Hide resolved
src/Client.js Outdated Show resolved Hide resolved
@ptpaterson
Copy link
Contributor

ptpaterson commented Mar 25, 2022

Can we add another method to Client as a shorthand for setting the withMetrics option? This would harmonize it with the JVM drivers. Something like:

/**
 * Executes a query via the FaunaDB Query API.  
 * See the [docs](https://app.fauna.com/documentation/reference/queryapi),
 * and the query functions in this documentation.
 * @param expression {module:query~ExprArg}
 *   The query to execute. Created from {@link module:query} functions.
 * @param {?Object} options
 *   Object that configures the current query, overriding FaunaDB client options.
 * @param {?string} options.secret FaunaDB secret (see [Reference Documentation](https://app.fauna.com/documentation/intro/security))
 * @return {external:Promise<Object>} An object containing the FaunaDB response object and the list of query metrics incurred by the request.
 */
Client.prototype.queryWithMetrics = function(expression, options) {
  options = Object.assign({}, options, { withMetrics: true })
  return this._execute('POST', '', query.wrap(expression), null, options)
}

@elersong elersong changed the title Add :withFaunaMetrics option to capture headers Add :metrics option to capture headers Mar 25, 2022
src/Client.js Outdated Show resolved Hide resolved
src/Client.js Outdated Show resolved Hide resolved
Copy link
Contributor

@faunaee faunaee left a comment

Choose a reason for hiding this comment

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

A couple of minor suggestions.

Also, we should include a test and an update to README.md describing this feature.

src/Client.js Show resolved Hide resolved
src/Client.js Outdated Show resolved Hide resolved
test/client.test.js Outdated Show resolved Hide resolved
src/Client.js Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
src/Client.js Outdated Show resolved Hide resolved
test/client.test.js Show resolved Hide resolved
Co-authored-by: Cleve Stuart <[email protected]>
src/Client.js Outdated Show resolved Hide resolved
@elersong elersong merged commit 2dd164e into v4 Mar 29, 2022
@elersong elersong deleted the queryWithStats branch March 29, 2022 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants