httpurlconnection create one span per io

[realark]

• Use the same operationName for all httpurlconnection calls
• use the connected field and helper hash map to determine if io has occurred
• Add a test for spring rest template
• Skipped instrumentation for a wrapper class which does not update its connected field

The way we're using connected is an implementation detail of HttpURLConnection. There's no way to cleanly do this at the api level (that I know of). Not great, but it works for all builtin implementations of HttpURLConnection.

Note that this will also not capture additional reads of the stream after the response stream is returned.

@gary-huang @tylerbenson Let me know what you think. Any concerns or anything you can think of to break this?

[gary-huang]

Looks good to me, just left a thought on potential problems, could be nothing.

[gary-huang]

Would this cause a side affect to the users by calling getResponseCode prematurely?

[tylerbenson]

I'm also concerned about this... getResponseCode() has some unfortunate side-effects.

[realark]

getResponseCode() causes the response to be parsed, caching the result in the responseCode field. Is the concern the additional work done to parse and cache the response code, or is there something else to consider?

[mar-kolya]

@realark: I think we may have been there before, e.g. 9ff09b9

[realark]

Good to know, thanks. I'll revert back to using the field. We'll backlog some additional work to better handle some of the doOutput=true corner cases.

[realark]

@gary-huang @tylerbenson Ran into some trouble in the case where connect() is called first. There doesn't seem to be an easy to way to capture capture the bulk of the i/o and do distributed tracing.

Solution for now:

• In connect() first cases, create two spans. One for connect() and one for i/o
• Disable httpurlconnection as a top-level span (the two-span case would cause confusion at the top-level).
• Use a weak-key map to store httpurlconnection state.

[tylerbenson]

Why is there two spans for a single request?

[realark]

This is the case where connect() is called before getInputStream(). We can change the instrumentation to only trace getInputStream(), but we won't be able to inject headers at that point.

[tylerbenson]

Should the two spans have different operation names (or some other way to distinguish them) in that case?

[realark]

Good idea. I used http.request.connect for those cases.

In the future I think we can handle this use-case without creating two spans but it will require some follow-on work.

[tylerbenson]