Add logging and instrumentation via features

[paul]

Adds logging and AS::Notifications-compatible instrumentation as separate features.

Building upon the features API introduced in #482, this adds a means to register new custom features:

HTTP::Options.register_feature(:my_feature, MyFeature)
HTTP.use(:my_feature).get("https://example.com/")

Also adds two new features, Logging and Instrumentation.

>> HTTP.use(logging: {logger: Logger.new(STDOUT)}).get("https://httpbin.org/get")
I, [2018-08-16T16:33:58.261195 #1538]  INFO -- : > GET https://httpbin.org/get
D, [2018-08-16T16:33:58.261256 #1538] DEBUG -- : Connection: close
Host: httpbin.org
User-Agent: http.rb/4.0.0.dev


I, [2018-08-16T16:33:58.487195 #1538]  INFO -- : < 200 OK
#<HTTP::Response/1.1 200 OK {"Connection"=>"close", "Server"=>"gunicorn/19.9.0", "Date"=>"Thu, 16 Aug 2018 22:33:58 GMT", "Content-Type"=>"application/json", "Content-Length"=>"201", "Access-Control-Allow-Origin"=>"*", "Access-Control-Allow-Credentials"=>"true", "Via"=>"1.1 vegur"}>
D, [2018-08-16T16:33:58.487440 #1538] DEBUG -- : Connection: close
Server: gunicorn/19.9.0
Date: Thu, 16 Aug 2018 22:33:58 GMT
Content-Type: application/json
Content-Length: 201
Access-Control-Allow-Origin: *
Access-Control-Allow-Credentials: true
Via: 1.1 vegur

{
  "args": {}, 
  "headers": {
    "Connection": "close", 
    "Host": "httpbin.org", 
    "User-Agent": "http.rb/4.0.0.dev"
  }, 
  "origin": "204.188.102.186", 
  "url": "https://httpbin.org/get"
}

>> HTTP.use(:instrumentation, instrumenter: ActiveSupport::Notifications, namespace: "my_http") # default namespace "http"
# => Emits a `"request.my_http"` event for the request/response

[tarcieri]

Nice!

[paul]

One of the jruby builds randomly failed, but it seems I don't have permission to restart it, and I don't want to commit an empty commit to poke it.

[tarcieri]

commit --amend and a force push will poke it, otherwise I can in a bit -- Tony Arcieri

[ixti]

IMO first use should have API:

def use(feature_id, **options); end

[paul]

@ixti Perhaps, but this PR didn't really touch that code. Want me to make that change as well?

[ixti]

@paul yes please

[ixti]

in fact i'm not sure :)) i just find HTTP.use(instrumentation: ActiveSupport::Notifications, namespace: "my_http") a bit confusing (seems like 2 features to me being asked to be used)

[ixti]

Let me think - will speak out tomorrow :D

[paul]

Well, I think it would instead be use(:instrumentation, instrumenter: AS::Notifications, namespace: "my_http"). That API doesn't allow multiple features to be used in one call (use(:auto_inflate, :auto_deflate), so it'd be an API-breaking change, unless we do something like:

def features=(*features, **options)
  @features = if features.size > 1 && !options.empty?
    # what here? assign options to every feature?
  elsif !options.empty?
    feature = features.first
    [feature.new(**options)]
  else
    features.map(&:new)
  end
end

[tarcieri]

What about:

use(:instrumentation)

use(instrumentation: { instrumenter: AS::Notifications, namespace: "my_http" })

use(:auto_inflate, :auto_deflate, instrumentation: { instrumenter: AS::Notifications, namespace: "my_http" })

use(:auto_inflate, instrumentation: { instrumenter: AS::Notifications, namespace: "my_http" }, :auto_deflate)

Essentially allow both Symbol and Hash as arguments, where Hash is expected to have one key which is a Symbol whose value is also a Hash (which is parsed into the name and the options hash)

[paul]

@tarcieri unless I'm mistaken, that's how it works now?

[tarcieri]

@paul haha okay, so why not just keep it that way?

[paul]

Oh, i see, I screwed up my example for it in the PR. No wonder I was confused.

[paul]

@ixti I had messed up the example I used in the description of this PR, so maybe that was your concern? This PR doesn't change the signature of .use at all, and I fixed the example.

Once this gets merged, I'll write a wiki page to document how to use the features feature.

[ixti]

Yeah. I guess I was confused.

Let's keep it as is.

[paul]

@ixti @tarcieri I think this is ready to be merged, then? This PR didn't actually change how #use worked.

Any chance of getting a release soon? The original feature stuff I did back in June isn't in the actual released gem yet.

[tarcieri]

[ixti]

@paul Yeah we have some backward incompatible changes right now in the master. I will check if we can extract somethign to release as 3.4.0 without introducing backward incompatible changes meanwhile. Just give me couple of weeks (on my vacation right now) and I will prepare and push the release.

[mateusluizfb]

There's any date to when this feature is gonna be available in the stable version?

[tarcieri]

@mateusluizfb if @ixti doesn't have time, I can do a release

[ixti]

Yeah. Sorry took me so long to get back in a shell. Will do release this weekend.

[ixti]

I have released current master as v4.0.0. Was hoping to sneak more breaking changes into major bump, but then realised that they will go to next major easily. :D

[tarcieri]

Thanks @ixti!

Was hoping to sneak more breaking changes into major bump, but then realised that they will go to next major easily. :D

The joys of semantic versioning!

[khataev]

Could you update the readme? Luckily found this PR, cause readme/wiki has no information about this feature

[paul]

@khataev I had updating the wiki on my TODO when I finished this PR, I guess it never happened. Sorry about that. I've added a page to the wiki, hopefully it helps: https://github.com/httprb/http/wiki/Logging-and-Instrumentation