Allow Tesla middleware to be configured

[GrantJamesPowell]

To provide support for custom middleware, allow the ExForce.Client.Tesla.build_client/2 method to take an additional param in it's opts argument defining middleware to run.

The choice was made to allow users to provide their own instrumentation

The middleware runs before the compression/json encoding and after the API version headers

[sourcelevel-bot[bot]]

Hello, @GrantJamesPowell! This is your first Pull Request that will be reviewed by SourceLevel, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

[sourcelevel-bot[bot]]

SourceLevel has finished reviewing this Pull Request and has found:

• 1 possible new issue (including those that may have been commented here).

See more details about this review.

[coveralls]

Coverage remained the same at 100.0% when pulling 7ffa9c4a577e0617c7bca33544cff34da4253d24 on SalesLoft:sl__configurable-tesla-middleware into db7792f57be45f576cd90dc08830a99853257a54 on chulkilee:master.

[chulkilee]

@GrantJamesPowell thanks for the PR!

I love instrumentation! However, I'm not convinced to choose specific position for any custom middleware hm.

Another approach would be: we add direct integration for Telemetry since Telemetry is becoming a standard instrumentation mechanism in Elixir world, and Telsa has built-in middleware for this - Tesla.Middleware.Telemetry. For example, the function may take telemetry_metadata, and add the Telemetry middleware when it's present.

[GrantJamesPowell]

@chulkilee, I agree a dedicated telemetry integration could provide value. That might be something our team would be willing contribute if you're interested.

As far as custom middle ware, as we iterated further we found several use cases within our SF sync for custom middleware, our auth flow/rate limiting/reporting is pretty nuanced and we're using custom middle ware to handle it. We're currently using this fork in production.

If you're interested in support for custom middleware in the upstream, we're more than happy to rework this patch to bring it more inline with the vision.

Thanks again for open sourcing this, we appreciate it!

[chulkilee]

@GrantJamesPowell thanks a lot for giving more details of your use case. I'm not using this library anymore :wink: but I'm happy to make reasonable changes as needed!

Since ExForce client is a just tesla client, you may use the new Tesla features to get the middleware - https://github.com/teamon/tesla/pull/373

See this fully working example:

%Tesla.Client{
  adapter: {Tesla.Adapter.Hackney, :call, [[]]},
  fun: nil,
  post: [],
  pre: [
    {ExForce.Client.Tesla.Middleware, :call, [{"http://example.com", "42.0"}]},
    {Tesla.Middleware.Compression, :call, [[format: "gzip"]]},
    {Tesla.Middleware.JSON, :call, [[engine: Jason]]},
    {Tesla.Middleware.Headers, :call, [[{"authorization", "Bearer mytoken"}]]}
  ]
} =
  client =
  ExForce.build_client(%{instance_url: "http://example.com", access_token: "mytoken"},
    adapter: Tesla.Adapter.Hackney
  )

[{ExForce.Client.Tesla.Middleware, _} = h | t] = Tesla.Client.middleware(client)
adapter = Tesla.Client.adapter(client)

%Tesla.Client{
  adapter: {Tesla.Adapter.Hackney, :call, [[]]},
  fun: nil,
  post: [],
  pre: [
    {ExForce.Client.Tesla.Middleware, :call, [{"http://example.com", "42.0"}]},
    {MyMiddleware, :call, [[]]},
    {Tesla.Middleware.Compression, :call, [[format: "gzip"]]},
    {Tesla.Middleware.JSON, :call, [[engine: Jason]]},
    {Tesla.Middleware.Headers, :call, [[{"authorization", "Bearer mytoken"}]]}
  ]
} = Tesla.client([h | [MyMiddleware | t]], adapter)

Could you test it out? If it works, then probably we can add it to documentation to ExForce.Client.Tesla as a tip, instead of changes in this PR.

[GrantJamesPowell]

@chulkilee, great points/tip. Made a card for our team to try this out.

I agree that the suggestion above, to edit the built client, is a much better idea.

Will close this PR when we move off of the fork

[ggiill]

I'm getting a timeout somewhere in Enforce.OAuth.get_token or ExForce.versions, and, since there is no ability to configure the middleware in ExForce.Client.Tesla.build_oauth_client, I can't trace where that timeout is coming from.

+1 to allowing global configuration of Tesla middleware in all ExForce clients. (Or, at least adding Tesla.Middleware.Telemetry by default.)