Decent approach to time OkHttpClient events for metrics?

Viewed 386

I'd like to gather connection and request timing metrics for an OkHttpClient instance that calls a particular service. I'm wondering if this approach is correct, and whether my interpretation of the event types makes sense?

Timer callTimer = <new codahale timer>;
Timer connectTimer = <new codahale timer>;
Timer secureConnectTimer = <new codahale timer>;
Timer requestTimer = <new codahale timer>;

# this gets registered with my client
new EventListener() {
      // see https://square.github.io/okhttp/events/#eventlistener for info on the ordering of these events
      private final Map<Call, Timer.Context> secureConnectTimerContexts = Maps.newConcurrentMap();
      private final Map<Call, Timer.Context> connectTimerContexts = Maps.newConcurrentMap();
      private final Map<Call, Timer.Context> callTimerContexts = Maps.newConcurrentMap();
      private final Map<Call, Timer.Context> requestTimerContexts = Maps.newConcurrentMap();

      @Override
      public void secureConnectStart(Call call) {
        secureConnectTimerContexts.put(call, secureConnectTimer.time());
      }

      @Override
      public void secureConnectEnd(Call call, @Nullable Handshake handshake) {
        Timer.Context context = secureConnectTimerContexts.remove(call);
        if (Objects.nonNull(context)) {
          context.stop();
        }
      }

      @Override
      public void connectStart(Call call, InetSocketAddress inetSocketAddress, Proxy proxy) {
        connectTimerContexts.put(call, connectTimer.time());
      }

      @Override
      public void connectEnd(Call call, InetSocketAddress inetSocketAddress, Proxy proxy, @Nullable Protocol protocol) {
        Timer.Context context = connectTimerContexts.remove(call);
        if (Objects.nonNull(context)) {
          context.stop();
        }
      }

      @Override
      public void connectionAcquired(Call call, Connection connection) {
        requestTimerContexts.put(call, requestTimer.time());
      }

      @Override
      public void connectionReleased(Call call, Connection connection) {
        Timer.Context context = requestTimerContexts.remove(call);
        if (context != null) {
          context.stop();
        }
      }

      @Override
      public void connectFailed(Call call, InetSocketAddress inetSocketAddress, Proxy proxy,
                                @Nullable Protocol protocol, IOException ioe) {
        Timer.Context context = connectTimerContexts.remove(call);
        if (Objects.nonNull(context)) {
          context.stop();
        }
      }

      @Override
      public void callStart(Call call) {
        callTimerContexts.put(call, callTimer.time());
      }

      @Override
      public void callEnd(Call call) {
        callFinishedForMetrics(call);
      }

      @Override
      public void callFailed(Call call, IOException ioe) {
        callFinishedForMetrics(call);
      }

      private void callFinishedForMetrics(Call call) {
        Timer.Context callTimerContext = callTimerContexts.remove(call);
        if (callTimerContext != null) {
          callTimerContext.stop();
        }

        requestTimerContexts.remove(call);
        secureConnectTimerContexts.remove(call);
        connectTimerContexts.remove(call);
      }
    }
1 Answers

You can use EventListener.Factory to create a unique listener instance for each Call. That way you don't need all the maps; the Timer.Context objects can just be instance fields of the call-bound EventListener.

Related