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

Log execution time of all CRUD APIs #214

Closed
vestrel00 opened this issue Jun 9, 2022 · 3 comments
Closed

Log execution time of all CRUD APIs #214

vestrel00 opened this issue Jun 9, 2022 · 3 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@vestrel00
Copy link
Owner

vestrel00 commented Jun 9, 2022

Problem

Users of this library have no easy way of knowing how long each query find() or insert/update/delete commit() functions take to execute, which would be very useful to know when dealing with very large data sets (e.g. 100,000 contacts).

Solution

Log the execution time in the LoggerRegistry, which currently looks like;

class LoggerRegistry(logger: Logger) {
    ...
        override fun onPreExecute(api: CrudApi) {
            logRedactable(api)
        }

        override fun onPostExecute(result: CrudApi.Result) {
            logRedactable(result)
        }

        private fun logRedactable(redactable: Redactable) {
            logger.log(redactable.redactedCopyOrThis(logger.redactMessages).toString())
        }
    ...
}

I'm thinking of adding private val apiExecutionStartTimeMillis: MutableMap<CrudApi, Long>. We can do something like...

override fun onPreExecute(api: CrudApi) {
    apiExecutionStartTimeMillis[api] = System.currentTimeMillis()
    logger.log(api.redactedCopyOrThis(logger.redactMessages).toString())
}

override fun onPostExecute(api: CrudApi, result: CrudApi.Result) {
    val executionTime = System.currentTimeMillis() - apiExecutionStartTimeMillis.remove(api)

    logger.log(
        """
           ${result.redactedCopyOrThis(logger.redactMessages)}
           execution time: $executionTime
        """.trimIndent()
        )
}

This will require refactoring ALL calls to onPostExecute to add the api: CrudApi as a parameter. It's a lot of redundant work but having the api and the result available in the same function will be very useful for a lot of other things and should have been the initial implementation anyways 😅

@vestrel00 vestrel00 added the good first issue Good for newcomers label Jun 9, 2022
@vestrel00 vestrel00 assigned vestrel00 and unassigned vestrel00 Jun 10, 2022
@jafri-zz
Copy link
Contributor

Gotta get that timer in there!

@vestrel00
Copy link
Owner Author

vestrel00 commented Jun 10, 2022

Gotta get that timer in there!

Hey! Been long time my dude! Thanks for helping me out with my passion project! Was also good catching up with you on Zoom 😁

@vestrel00 vestrel00 moved this from To do to In progress in General maintenance Jun 10, 2022
@vestrel00 vestrel00 moved this from In progress to In review in General maintenance Jun 10, 2022
vestrel00 added a commit that referenced this issue Jun 10, 2022
Closes #214: Log execution time of all CRUD APIs
@vestrel00 vestrel00 moved this from In review to Done in General maintenance Jun 10, 2022
vestrel00 added a commit that referenced this issue Jun 10, 2022
@vestrel00
Copy link
Owner Author

This is included in release 0.2.1!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Development

No branches or pull requests

2 participants