Instrument network calls

[mtodd]

This PR works to add hooks for instrumentation/logging around network calls.

This specific implementation works well with ActiveSupport::Notifications but does not depend on it, only on an object that responds to instrument(event_name, payload, &block) (like the MockInstrumentationService for the tests).

This wraps the new private methods Net::LDAP::Connection#read and Net::LDAP::Connection#write.

Thoughts?

[mtodd]

Might need help with figuring out why jruby-1.9 is failing: https://travis-ci.org/ruby-ldap/ruby-net-ldap/jobs/33450895

[mtodd]

Guess it's been failing consistently: https://travis-ci.org/ruby-ldap/ruby-net-ldap/builds

[mtodd]

Thinking about this as a way to get information about the content length on read:

diff --git a/lib/net/ber/ber_parser.rb b/lib/net/ber/ber_parser.rb
index 682a599..47379b8 100644
--- a/lib/net/ber/ber_parser.rb
+++ b/lib/net/ber/ber_parser.rb
@@ -160,6 +160,7 @@ module Net::BER::BERParser
     if -1 == content_length
       raise Net::BER::BerError, "Indeterminite BER content length not implemented."
     else
+      yield id, content_length if block_given?
       data = read(content_length)
     end

diff --git a/lib/net/ldap.rb b/lib/net/ldap.rb
index 234a0a3..7c4c396 100644
--- a/lib/net/ldap.rb
+++ b/lib/net/ldap.rb
@@ -1268,14 +1268,17 @@ class Net::LDAP::Connection #:nodoc:
   end

   def read(syntax = Net::LDAP::AsnSyntax)
-    instrument "read.net_ldap_connection", :syntax => syntax do
-      @conn.read_ber(syntax)
+    instrument "read.net_ldap_connection", :conn => @conn, :syntax => syntax do |payload|
+      @conn.read_ber(syntax) do |id, content_length|
+        payload[:response_id]    = id
+        payload[:content_length] = content_length
+      end
     end
   end
   private :read

   def write(packet)
-    instrument "write.net_ldap_connection", :packet => packet do
+    instrument "write.net_ldap_connection", :conn => @conn, :packet => packet do
       @conn.write(packet)
     end
   end

[mtodd]

Specifically, the @conn.read_ber call with a block.

[mtodd]

@schaary would love some feedback on whether this will be considered for release or if I should instead maintain my own fork. Not sure what your contribution policies are, if they've changed since the last substantive Hacking.rdoc changes, if this repo is mostly in maintenance mode, etc. Any feedback appreciated.

[mtodd]

This could definitely be abstracted/extracted out.

[mtodd]

Still need to update docs and instrument other methods similar to search. I'm also happy to extract unrelated changes (such as using the Net::LDAP::PDU::RequestOrResponseConstant instead of equivalent integers) if that makes this easier to accept.

[halostatue]

This looks interesting—I'm not deep in the code enough to act on it myself, but this should probably be considered as I recall seeing a number of performance issues raised over time, and this should make it easier to find.

I also agree with your general commentary about repeated complex stanzas, @mtodd.

[mtodd]

I realized that there were a couple unrelated changes merged into this branch (because I was reusing this branch to do work in github/ruby-net-ldap) from #102 and #103. I'm happy to fix that, though if those two PRs get merged before this, it should reduce the diff here (this is the route I would recommend).

Any specific feedback that I can get working on here to get this into a state for merging? Any relevant documentation you feel is missing?