[integration] add support for AWS

[p-lambert]

Overview

Adds the AWS integration that traces all interactions (e.g. API calls) with AWS services (S3, ElastiCache etc.). To use it simply:

require 'aws-sdk'
require 'ddtrace'

Datadog::Monkey.patch_module(:aws)  # explicitly patch it

Aws::S3::Client.new.list_buckets  # this call is traced

[palazzem]

Minor failing because of:

  1) Failure:
MonkeyTest#test_patch_module [/home/ubuntu/dd-trace-rb/test/monkey_test.rb:26]:
Expected: false
  Actual: nil

We have tests to ensure the default of Monkey.patch_all, so probably you need to add this integration to the list.

[palazzem]

Thanks for the contribution! a small change and a question about the span.name. Then we should be good here!

[palazzem]

we should check pin.enabled?:

dd-trace-rb/lib/ddtrace/pin.rb

Lines 32 to 35 in 54ed5b7

	def enabled?
	return @tracer.enabled if @tracer
	false
	end

[palazzem]

what kind of names do we have here? can we check that in our tests also so it's explicit what we're expecting?

[p-lambert]

Basically I tried to mimmic the Python's client behavior here. The name will be something like service.action. There is a test case for that already:

dd-trace-rb/test/contrib/aws/parsed_context_test.rb

Lines 12 to 14 in 54ed5b7

	def test_resource
	assert_equal('s3.list_buckets', @context.resource)
	end

[p-lambert]

Whoops, just realized we don't have a explicit test for the name! Will add that!

[palazzem]

👍

[palazzem]

assuming that each application uses only one initialized AWS client, setting the Pin in the module ::Aws is fine.

[p-lambert]

Actually each service builds its own client dynamically, the first time the service constant (e.g. S3) is referenced. If you think having a separated pin would be desirable, I could work on that, but I'll probably have to do some nasty hack around the inherited hook of the base class or something like that. That would make the contribution a bit more brittle, because we wouldn't be sticking to their plugin API. Whatever you think it's best!

[palazzem]

I think that for a user AWS is the service. I prefer a simple approach and then extend it if it's needed. Especially if we should do a nasty hack, so good to me that way 👍

[palazzem]

That's great! thank you so much for this integration!

[palazzem]

@p-lambert cool! the code is fine, now we're only missing the documentation and the integration is ready. Feel free to ping me again to review the docs. Thank you!

(forget to ask for docs)

[palazzem]

@p-lambert let's initialize the client and tracer only in the setup

[palazzem]

@p-lambert always test our pin defaults, so we know if we break defaults while changing how the integration works

[palazzem]

@p-lambert it's enough to have all the checks in one test since it should reproduce a real use case (i.e. a call to list_buckets or a more complex scenario that reproduces/protects from a regression).

[palazzem]

@p-lambert don't use our constants in tests; http.method is what we're expecting, so if someone changes the constant value (breaking a lot of stuff), this test must break.

[palazzem]

@p-lambert always grab the spans list and check how many spans are there. If you search only what you need, you will miss:

• double instrumentation (2 spans instead of one)
• conflicts in instrumentation (one integration triggers wrongly another one)
• wrong parenting if you're expecting 2+ spans