Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect aws-sdk-core as well as the monolithic aws-sdk gem #752

Merged
merged 2 commits into from
May 10, 2019
Merged

Detect aws-sdk-core as well as the monolithic aws-sdk gem #752

merged 2 commits into from
May 10, 2019

Conversation

tobypinder
Copy link
Contributor

See #707

This PR introduces @alksl's workaround from the issue above to detect both monolithic and modular versions of the aws-sdk* gem family. This is important as AWS themselves encourage the more modular approach going forward.

Versions between the "monolithic" and "modular" gems are kept consistent (see aws-sdk-core and aws-sdk) so there shouldn't be any problems with collision.

@tobypinder
Copy link
Contributor Author

Not sure what's causing the remaining CI failure - it appears to be unrelated.

rspec './spec/ddtrace/contrib/grpc/integration_spec.rb[1:2:1:1]' # gRPC integration test request reply behaves like associates child spans with the parent 
rspec './spec/ddtrace/contrib/grpc/integration_spec.rb[1:4:1:1]' # gRPC integration test server stream behaves like associates child spans with the parent 
rspec ./spec/ddtrace/contrib/grpc/integration_spec.rb:34 # gRPC integration test multiple client configurations uses the correct configuration information

@delner
Copy link
Contributor

delner commented May 9, 2019

Thanks for the contribution @tobypinder! Don't worry about the gRPC test, that flakes out sometimes. I reran it for you.

Just for clarification, our instrumentation as its written will work fine with both the monolith gem or individual modules, as long as they include "core"? Would it make sense to just derive this version from "core" always? (Assuming its part of the monolith?)

@delner delner added bug Involves a bug community Was opened by a community member integrations Involves tracing integrations labels May 9, 2019
@tobypinder
Copy link
Contributor Author

@delner glad to hear it's unrelated.

My understanding is that the current monolithic gem is just calling the other gems via dependencies:

  • aws-sdk
    • aws-sdk-resources

And that each "resource" (or api type) has a dependency on aws-sdk-core

I think moving to just aws-sdk-core would work, but I think versions of the gem prior to version 2 would then break, as aws "split" the subgems from the monolith beginning with v2. aws-sdk v2 was released September 25, 2014 but I imagine there are still people using it and didn't want to introduce any breaking changes.

@delner
Copy link
Contributor

delner commented May 10, 2019

@tobypinder I think you're right... the latest version of their 1.x branch was released in 2017 and still has well over a million downloads on it, so there's a risk of that breaking compatibility. Given that, I think this is fine as is.

Copy link
Contributor

@delner delner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for the contribution! 👍

@delner delner merged commit 411ce3a into DataDog:master May 10, 2019
@delner delner added this to the 0.23.2 milestone May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug community Was opened by a community member integrations Involves tracing integrations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants