-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Remove span_type
method call on SpanOperation
#4776
Remove span_type
method call on SpanOperation
#4776
Conversation
@@ -158,8 +155,7 @@ def resolve_type_lazy(object:, type:, query:) | |||
|
|||
def resolve_type_span(span_key, object, type, query) | |||
platform_key = @platform_key_cache[DataDogTrace].platform_resolve_type_key_cache[type] | |||
@tracer.trace(platform_key, service: @service_name) do |span| | |||
span.span_type = 'custom' | |||
@tracer.trace(platform_key, service: @service_name, type: 'custom') do |span| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to make sure I understand:
What will dd-trace-rb do with this type: 'custom'
argument? Will it ignore it, or 💥 ?
If this code only works with dd-trace-rb 2+, what do you think about adding a version check of some kind in def initialize
above? If that's possible, it might be a nicer development experience than troubleshooting the argument error (if span: ...
does cause an error, that is).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will dd-trace-rb do with this type: 'custom' argument?
The data model of Span
/SpanOperation
contains a type
field, which can filled by multiple ways.
- Through
Tracing#trace(...)
, there are bothtype
/span_type
as optional keyword arguments. https://github.com/DataDog/dd-trace-rb/blob/master/lib/datadog/tracing/tracer.rb#L131-L132
Both options yields the same outcome.
[2] pry(main)> span = Datadog::Tracing.trace('my_span', span_type: 'custom');
[3] pry(main)> span.type
=> "custom"
[4] pry(main)> span = Datadog::Tracing.trace('my_span', type: 'custom');
[5] pry(main)> span.type
=> "custom"
- Through attribute setter from the
SpanOperation
, https://github.com/DataDog/dd-trace-rb/blob/master/lib/datadog/tracing/span_operation.rb#L502
Since,span_type=
is just an alias fortype=
, the following code are the same.
[3] pry(main)> Datadog::Tracing.trace('my_span') do |span|
[3] pry(main)* span.span_type = 'custom'
[3] pry(main)* puts span.type
[3] pry(main)* end
custom
[3] pry(main)> Datadog::Tracing.trace('my_span') do |span|
[3] pry(main)* span.type = 'custom'
[3] pry(main)* puts span.type
[3] pry(main)* end
custom
If this code only works with dd-trace-rb 2+
This is not entirely correct. Because the changes basically migrate the deprecated usage to a supported usage, it works on all dd-trace-rb
versions.
dd-trace-rb
1.x supports bothspan_type=
andtype=
dd-trace-rb
2.x only supportstype=
The existing graphql-ruby
versions without such changes won't work with dd-trace-rb
2+. That is why graphql-ruby
users need to upgrade with this change to use dd-trace-rb
2+.
Closes #4762
Why
Datadog tracer is removing a deprecated method
span_type
fordd-trace-rb
2+ . This is the PR.What
graphql-ruby
fromspan_type
totype
.dd-trace-rb
service
option and its defaulttracer
optionAdditional Notes
The test is setup with DataDog/dd-trace-rb#3376
🚨 This change is backward compatible (All graphql-ruby versions works with
dd-trace-rb
1+ ). However,graphql-ruby
users need to upgrade with this change to usedd-trace-rb
2+.