Add HTTPTransport setting to gremlin-go driver

[milesbxf]

We'd like to be able to instrument the HTTP transport layer in gremlin-go, monitor connections and requests etc. Rather than add a specific metrics implementation that won't be useful to everyone, I'm proposing we make it possible for users to supply their own http.RoundTripper so that they can hook into requests/responses and add custom logic. I think this is very Go idiomatic and opens up lots of other interesting use cases like more control over retries/client-side rate limits/logging/timeouts/testing.

To make this possible, I've added an HTTPTransport http.RoundTripper field to ClientSettings and DriverRemoteConnectionSettings. When set, the driver uses it directly instead of creating its own transport. When nil, we fall back to the existing default behaviour.

Open question: we expose a few HTTP transport dials directly on the DriverRemoteConnectionSettings (e.g. MaxIdleConnections/IdleConnectionTimeout). If a user supplies their own http.RoundTripper but also configures these then what should happen? Currently we're just ignoring the specific configs and expecting that the user's RoundTripper has set sensible values, but I could see this being confusing. One possible alternative is a decorator pattern (something like TransportDecorator func(http.RoundTripper) http.RoundTripper) that allows the user to wrap a base http.RoundTripper that we supply with the pool configuration - that has some other complexities though.

[Cole-Greer]

Hi @milesbxf,
Thanks for submitting this PR. Before getting into the changes, I wanted to let you know that we are currently in code freeze in preparation for a set of releases (3.7.6, 3.8.1, and 4.0.0-beta.2) next week, and are not currently considering new submissions for these releases. Your changes here will be considered for the following release cycle.

I did want to ask about which version you intend to target with your changes. If you're unfamiliar with our branching strategy, 3.7-dev accumulates changes for the 3.7.x release line, 3.8-dev for 3.8.x, and master is currently accumulating changes for 4.0.0. If you want to target multiple release lines, we ask you target the oldest branch, and we will merge the changes up to later releases for you after your PR is merged.

Gremlin-go looks very different under the hood in TinkerPop 4 (master) compared to 3.x.x, as TinkerPop 4 uses HTTP as a transport, while TinkerPop 3 uses WebSockets. Is your intention here to contribute instrumentation solely to gremlin-go 4, or did you want to add it to an existing 3.7.x/3.8.x release line which you are currently using? If you are looking for instrumentation in TinkerPop 3, I would ask that you open an additional PR targetting either 3.7-dev or 3.8-dev with a websockets implementation, in addition to this HTTP implementation for TinkerPop 4.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.49%. Comparing base (cfd6889) to head (7886e8f).
⚠️ Report is 917 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3360      +/-   ##
============================================
- Coverage     77.87%   75.49%   -2.38%     
+ Complexity    13578    13307     -271     
============================================
  Files          1015     1023       +8     
  Lines         59308    60260     +952     
  Branches       6835     7075     +240     
============================================
- Hits          46184    45493     -691     
- Misses        10817    12080    +1263     
- Partials       2307     2687     +380     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.