-
-
Notifications
You must be signed in to change notification settings - Fork 487
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
Add span and trace origins #2319
Conversation
9fb1e3c
to
5c988f8
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2319 +/- ##
==========================================
+ Coverage 98.66% 98.68% +0.01%
==========================================
Files 205 205
Lines 13422 13486 +64
==========================================
+ Hits 13243 13308 +65
+ Misses 179 178 -1
|
rails CI is broken due to |
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.
LGTM
@@ -8,7 +8,8 @@ class Plugin < ::Delayed::Plugin | |||
# need to symbolize strings as keyword arguments in Ruby 2.4~2.6 | |||
DELAYED_JOB_CONTEXT_KEY = :"Delayed-Job" | |||
ACTIVE_JOB_CONTEXT_KEY = :"Active-Job" | |||
OP_NAME = "queue.delayed_job".freeze | |||
OP_NAME = "queue.delayed_job" |
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.
Was .freeze
removed from OP_NAME
on purpose?
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.
some files have this on top so it applies to all strings by default
https://docs.ruby-lang.org/en/3.0/syntax/comments_rdoc.html#label-frozen_string_literal+Directive
I'll clean this up separately, just syntax nicety and it's not consistent in some files.
@@ -8,7 +8,8 @@ class Plugin < ::Delayed::Plugin | |||
# need to symbolize strings as keyword arguments in Ruby 2.4~2.6 | |||
DELAYED_JOB_CONTEXT_KEY = :"Delayed-Job" | |||
ACTIVE_JOB_CONTEXT_KEY = :"Active-Job" | |||
OP_NAME = "queue.delayed_job".freeze | |||
OP_NAME = "queue.delayed_job" | |||
SPAN_ORIGIN = "auto.queue.delayed_job" |
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.
Not all of the span origins have the .freeze
call, should they?
:origin
attribute to span and use in trace context and span hashcloses #2068