Sequel Support

[ufoot]

Following the work initiated by #163 this PR:

• includes Adds support for Sequel ruby gem. #163 (which is a very nice starting points, has tests & everything)
• adds support for execute_ddl, execute_dui and execute_insert (without that last one, we'd get no clue on INSERT calls for instance)
• completes the test suite

[randy-girard]

I wasn't sure if you would want to differentiate between mysql and mysql2 adapters. If you do, can probably remove this line, otherwise this should group them together.

[randy-girard]

This regexp should be updated and probably extract out into the class:

REGEXP ||= Regexp.new(
            Regexp.union([
              /'(?:[^']|'')*?(?:\\'.*|'(?!'))/,
              /"(?:[^"]|"")*?(?:\\".*|"(?!"))/,
              /\b-?(?:[0-9]+\.)?[0-9]+([eE][+-]?[0-9]+)?\b/,
              /\b(?:true|false|null)\b/i,
              /0x[0-9a-fA-F]+/,
              /(?:#|--).*?(?=\r|\n|$)/i,
              /\/\*(?:[^\/]|\/[^*])*?(?:\*\/|\/\*.*)/
            ])
          )

Replace this following line to sql.to_s.gsub(REGEXP, '?'). This should help speed it up some as well as remove extraneous ?.

[randy-girard]

Added a few comments. Otherwise looks good!

[thiago-sydow]

Hello! Is there something missing/blocked in this PR? I was going to start using this gem but just now realized the lack of support for Sequel

[palazzem]

Hello @thiago-sydow ! thanks for reaching us! At the moment we're planning to ship a new release with a lot of changes. Unfortunately Sequel support will not be added in this release but in the next one. I can ping you again once the review (and the refactoring with the new config system) is in our weekly cycle.

Sorry for the delay!

[thiago-sydow]

Hi @palazzem thank you for replying! I understand, and I would love to get notified when Sequel is worked/merged.

I checked the new config system to be released and seem far better then the current one, nice job!

[palazzem]

@thiago-sydow thank you very much! Will ping everyone in this thread once we move this PR review from the backlog to the current cycle of work.

[delner]

Updated this pull request to 0.13-dev:

• Squashed previous commits
• Fixed merge conflicts
• Changed to not use Monkey
• Converted tests from Minitest to RSpec

Still need to:

• Update use of Pin in favor of Datadog.configure
• Update documentation
• Change how it quantizes SQL, and restructure the span
• Review how we patch Sequel (is there a better way than re-writes?)

[delner]

In addition to the above, I also:

• Extracted #normalize_vendor from ActiveRecord into Datadog::Utils::Database so this integration could use it without depending on the ActiveRecord integration.
• Changed the default from sequel to whatever the adapter name is. If you set a service name via #use or directly against the database object, that one is used instead.

[palazzem]

Hey @thiago-sydow ! we've finished the rebase and all the work around Sequel! Actually the 0.12 is in release candidate but Sequel will be released in the 0.13 (usually we do a minor release every 2-3 weeks). Before that time, we'll ship probably a beta release so all these new integrations can be tested beforehand.

Sounds good?

[thiago-sydow]

Hi @palazzem thank you for notifying me, yes that sounds great!

[delner]

Also, @palin just letting you know this one is merged!

[twe4ked]

I'm having trouble getting this to work. I've tried a few different versions of the gems. I'm trying to match what I've installed locally using the appraisals gem. I get get the ddtrace specs to pass but I can't get the example to run.

Example gist: https://gist.github.com/twe4ked/590db5a686104770a53a78b657eacc3d

Thanks.

Edit: The Database#initialize method never seems to get called.

[delner]

@twe4ked Hmmm, if initialize isn't being called, then maybe it isn't patching. I'll take a look at this, too. Thanks for the Gist.

[twe4ked]

No worries. It seems to prepend the InstanceMethods ok. I've also confirmed when I run the ddtrace specs that the initialize method is called so that definitely seems like the right path.

[delner]

Cool, let me know if you find anything else.

[delner]

Oh I think I know why: your Sequel::Database is created before Datadog.configure runs. If you configure before creating the database, it should work.

This is because it patches #initialize, which adds a Pin configuration to the Database object. Though #initialize does get patched, it only happens after the database was created, thus the database has no tracer configuration.

I suppose we could probably add some kind of default pin to the Sequel::Database constant, so databases will at least get some default tracing if they were instantiated too early, as opposed to nothing.

@twe4ked I'll open a bug fix PR for this. Thanks for reporting!

[twe4ked]

That's it! Thanks for your help.

[palin]

Amazing! Thanks!

[delner]

@twe4ked Pull request for the bug fix open here: #408. Will hopefully merge that to 0.13-dev soon.

[twe4ked]

Any update on getting a beta release out soon? I'm guessing it might be a while since 0.12.0 is still in RC phase. Cheers.

[palazzem]

Hello @twe4ked! yes we're going to ship the 0.12.0 this week, so that the first 0.13.beta1 can be out next one. Thanks for your patience!