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.

Sign up for GitHub

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

http_text_format + w3c trace parent #55

Merged
merged 15 commits into from
Sep 12, 2019
Merged

http_text_format + w3c trace parent #55

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
e0ced19
http_text_format
ibawt Sep 9, 2019
94dbd6d
docs + implicit block
ibawt Sep 9, 2019
030cade
this should be a .to_s and I should have a test
ibawt Sep 10, 2019
2c8db28
move the encoding choice of the flag into the traceparent
ibawt Sep 10, 2019
b1fbec8
rubocopped
ibawt Sep 10, 2019
72bf7d7
add url
ibawt Sep 10, 2019
c9beea0
spelling
ibawt Sep 10, 2019
5c0fbf6
spacing
ibawt Sep 10, 2019
2002f30
add a bunch of documentation
ibawt Sep 10, 2019
5ae9cc2
use argument error
ibawt Sep 10, 2019
811480e
use the @return
ibawt Sep 10, 2019
f76e980
Merge branch 'master' into http-text-format
ibawt Sep 10, 2019
859d38a
move up the self methods into one block
ibawt Sep 11, 2019
c7fc1c4
Merge branch 'http-text-format' of https://github.com/Shopify/opentel…
ibawt Sep 11, 2019
a68c346
Merge branch 'master' into http-text-format
fbogsany Sep 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
add url
  • Loading branch information
@ibawt
ibawt committed Sep 10, 2019
commit 72bf7d7058a9158787096a425a3cf1beb846287b
1 change: 1 addition & 0 deletions api/lib/opentelemetry/distributed_context/propagation/trace_parent.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module OpenTelemetry
module DistributedContext
module Propagation
# A TraceParent is an implemenation of the W3C trace context specification
# https://www.w3.org/TR/trace-context/
# {SpanContext}
class TraceParent
Copy link
Member

Choose a reason for hiding this comment

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

The layout of this class is not what I am used to seeing. I'm not sure if we have a convention, or if we should have one, but typically I'm used to seeing something like:

class MyClass
  #constants
  MY_CONST = 1
  class << self
     #public class methods
     def public_class_method; end
     
     private
     
    #private class methods
     def private_class_method; end
  end
  
  #public instance methods

  def public_instance_method; end

  private
  
  #private instance methods
  def private_instance_method; end
end

Would there be any objections to adopting a similar format for OTel Ruby (and this file)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so you want the "static" methods up top? That doesn't bother me at all, I'll move it up.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, there are some that are in the class << self block and a few that are self. prefixed. If you wouldn't mind moving them into one class << self block at the top that'd be awesome!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated it, have a look.

Copy link
Member

Choose a reason for hiding this comment

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

Looks great!

InvalidFormatError = Class.new(Error)
Expand Down