armeria icon indicating copy to clipboard operation
armeria copied to clipboard

Add timing information for acquiring an existing connection

Open jrhee17 opened this issue 2 years ago • 1 comments

Motivation:

It has been pointed out that pendingAcquisitionDuration denotes only the duration during which a connection is newly acquired. I propose that we add an existingAcquisitionDuration to clearly differentiate between

  1. Waiting for a connection that is pending in completing
  2. Waiting for a connection that already exists

Modifications:

  • Add ClientConnectionTimings#existingAcquisitionStartTimeMicros, ClientConnectionTimings#existingAcquisitionStartTimeMillis, ClientConnectionTimings#existingAcquisitionDurationNanos
  • Add existing.acquisition.duration to RequestMetricSupport
    • Note that although the original issue suggested we migrate all metrics related to request timing, I don't think this has to be necessarily done in this PR
  • Rename brave spans annotations "connection-reuse.start", "connection-reuse.end" -> "connection-pending.start", "connection-pending.end"
  • Add brave span annotations "connection-existing.start", "connection-existing.end"

Result:

  • Closes #4886
  • Describe the consequences that a user will face after this PR is merged.

Screenshot 2023-06-27 at 3 42 46 PM

jrhee17 avatar Jun 07 '23 15:06 jrhee17

@jrhee17 Please let us know once it's ready!

trustin avatar Jul 05 '23 08:07 trustin

Looking back into this, I'm not sure if there is really much value in adding a meter for this particular scenario. Let me close this PR and revisit later

jrhee17 avatar Apr 11 '24 07:04 jrhee17