Question related to upgrade

[TonyCTHsu]

👋 Hi, I am currently working on dd-trace-rb at Datadog. I have a question related to upgrading.

Currently, I am working on dd-trace-rb 2.0 upgrade, which contains breaking changes of our public API.

This PR removes the deprecated method that is used at here in graphql-ruby. If I change to span.type = 'custom' in graphql-ruby, it is still possible that it breaks application since there is no version/dependency management between dd-trace-rb and graphql-ruby.

I was wondering how would you recommend handling this situation?

[rmosolgo]

Hey, thanks for asking. I guess my suggestion would be to assume that an application only runs one version of the datadog library, so you could check once to detect which method is supported:

# `span_type` was removed in dd-trace-rb 2.0, so check for that: 
@span_type_method ||= span.respond_to?(:type=) ? :type= : :span_type= 
span.public_send(@span_type_method, "custom") 

What do you think of that approach?

[TonyCTHsu]

👋 @rmosolgo , Thanks for getting back to me.

What do you think of that approach?

Hmm... I think such approach might not work properly with dd-trace-rb 2+ with any graphql-ruby version before changes, since those graphql-ruby versions are still using the removed method (span_type) from dd-trace-rb.

---	graphql-ruby (current)	graphql-ruby (future)
dd-trace-rb 1+	✅	✅
dd-trace-rb 2+	❌	✅

This problem is tricky because there is no version/dependency management between the two gems. I supposed the version detection has to be sitting in dd-trace-rb in order to understand whether which public API graphql-ruby is using.

[rmosolgo]

Yep, that sounds like the nature of a breaking change to me: previously-written code won't work. The other options I see are:

• Retain the old methods to keep compatibility
• Add a new tracer to dd-trace-rb 2+ with documentation to use that one with the new version. (For comparison, Skylight and OpenTelemetry maintain their own tracers. It doesn't solve every compatibility version, because some GraphQL-Ruby changes require tracer changes, but it's easier in some ways)
• Proceed as suggested above, document that dd-trace-rb 2+ requires GraphQL-Ruby 2.2.4+

What sounds good to you? Are there other options I've overlooked?

[TonyCTHsu]

👍 Thanks for the suggestions.

Skylight and OpenTelemetry maintain their own tracers

I will bring the discussion back to our team, but for now I have opened a pull request for the changes in graphql-ruby.

The changes are fairly straightforward and additional test setup can be found at DataDog/dd-trace-rb#3376

Furthermore, I understand that graphql-ruby might introduce breaking changes in minor version release, and our stats shown that a lot of our users are staying on 1.12.x , 1.13.x, 2.0.x, 2.1.x, 2.2.x. Would it be possible that those changes can be backport to some of those versions? This would help users upgrade dd-trace-rb without having to deal with the breaking changes from graphql-ruby.

[rmosolgo]

I'm game to publish updates to those old graphql-ruby versions. Would you mind opening the backport PRs? You could open PRs against these branches:

• 1.12.x
• 1.13.x
• 2.0.x
• 2.1.x

And I'll merge them, update the changelog, and publish them to Rubygems 👍

[TonyCTHsu]

👋 @rmosolgo , Thanks! I have opened several PRs to backport (draft PR).

However, some of them are failing tests not related to the changes, should I be concerned about them?

I am currently discussing with our team for what other stuff, we would like to backport.

I will keep you posted when it is ready. 😄

[TonyCTHsu]

👋 @rmosolgo I believe it is ready. After speaking to the team. We would like to backport to 1.13.x, 2.0.x, and 2.1.x

Branch	Pull Request
master	#4776
2.1.x	#4786
2.0.x	#4789
1.13.x	#4788

I took this chance and add a couple of additional changes, including

1. Replace dd-trace-rb constants reference with simple strings
2. Deprecate redundant options
3. Change service default to nil

[rmosolgo]

Great, I'll review those soon and get those releases out!