apollographql / apollo-kotlin

:rocket:  A strongly-typed, caching GraphQL client for the JVM, Android, and Kotlin multiplatform.
https://www.apollographql.com/docs/kotlin
MIT License
3.76k stars 653 forks source link

[3.x] Add hooks for monitoring request speed/active requests #3221

Closed martinbonnin closed 1 year ago

martinbonnin commented 3 years ago

Follow up from https://github.com/apollographql/apollo-android/pull/2674.

@AdamMTGreenberg if you already have an API in mind, feel free to comment here, else we can update as we go.

AdamMTGreenberg commented 3 years ago

No APIs in mind at this very second other than a start/stop for parsing objects, but thank you for opening a dialog.

At the moment ApolloCall.StatusEvent feels a bit too limited in scope. For generic events this is great, but for each, I would love to be able to drill down into details about each piece. Eg, for FETCH_CACHE would love to also be able to get additional info on which cache, start time, stop time, if-failure, and potential cache result information.

This would all be super helpful for performance metrics we want to track.

If you think it would be more helpful, at some point this week I can open a sample diff for what I would have in mind as a great-to-have.

martinbonnin commented 3 years ago

Eg, for FETCH_CACHE would love to also be able to get additional info on which cache, start time, stop time, if-failure, and potential cache result information.

We could make each piece of the pipeline report their information in the ExecutionContext. Right now there is response.isFromCache but there could be more. For an example:

class CacheInfo(
  val startMillis: Long,
  val endMillis: Long,
  val hit: Boolean
)

And for network:

class NetworkInfo(
  val startMillis: Long,
  val endMillis: Long,
)

on which cache

Results might be composed from multiple caches but I guess we could take the "deepest" one.

If you think it would be more helpful, at some point this week I can open a sample diff for what I would have in mind as a great-to-have.

Samples are super welcome!

AdamMTGreenberg commented 3 years ago

Created two rough sample PRs from some ideas I was spitballing

martinbonnin commented 3 years ago

3.0.0-alpha07 now has:

class HttpInfo(
    val millisStart: Long,
    val millisEnd: Long,
    val statusCode: Int,
    val headers: List<HttpHeader>
)
class CacheInfo(
    val millisStart: Long,
    val millisEnd: Long,
    val hit: Boolean,
    val missedKey: String?,
    val missedField: String?,
)

It also has FlowDecorators that allows more customization:

apolloClient.withFlowDecorator {
    it.onStart {
      // log start
    }.onEach {
      // log response
    }.onCompletion {
      // log end
    }
  }

@AdamMTGreenberg let me know how that looks and if anything else is needed.

AdamMTGreenberg commented 3 years ago

@martinbonnin thanks for this - it looks awesome. I'll have to dig in, but offhand do you know if in HttpInfo the start/stop are triggered when the request is executed or scheduled?

martinbonnin commented 3 years ago

when the request is executed or scheduled

You mean what thread the start/stop is measured in? The threading is different on JVM vs Native (see https://github.com/apollographql/apollo-android/blob/dev-3.x/design-docs/Threading.md) but for JVM, the high level scheduling is like:

So it will not account for any Thread context switch.

AdamMTGreenberg commented 3 years ago

Thank you!

martinbonnin commented 2 years ago

Heads up that flowDecorators are gone with https://github.com/apollographql/apollo-android/pull/3538 as they were basically duplicating the Interceptor functionality. We'll add some doc about this. Actually, interceptors now control their Dispatcher so it will allow more fine grained monitoring if needed

martinbonnin commented 1 year ago

I'm going to close this issue. There are a bunch of extension points already:

@AdamMTGreenberg let us know if there's something you can't do with those and I'll reopen this issue.