fix: update context to match spec

api/lib/opentelemetry/context.rb

@@ -29,23 +29,44 @@ def current
         Thread.current[KEY] ||= ROOT
       end
 
-      # Sets the current context
-      #
-      # @param [Context] ctx The context to be made active
+      # @api private
       def current=(ctx)
         Thread.current[KEY] = ctx
       end
 
+      # Returns the previous context so that it can be restored
+      #
+      # @param [Context] context The new context
+      # @return [Context] prev The previous context
+      def attach(context)
+        return current.parent if context == current
+
+        prev = current
+        self.current = context
+        context.parent = prev
+      end
+
+      # Restores the current context to the context supplied or the parent context
+      # if no context is provided
+      #
+      # @param [Context] previous_context The previous context to restore
+      def detach(previous_context = nil)
+        OpenTelemetry.logger.warn 'Calls to detach should match corresponding calls to attach' if current.parent != previous_context
+
+        previous_context ||= current.parent || ROOT
+        self.current = previous_context
+      end
+
       # Executes a block with ctx as the current context. It restores
       # the previous context upon exiting.
       #
       # @param [Context] ctx The context to be made active
       # @yield [context] Yields context to the block
       def with_current(ctx)
-        prev = ctx.attach
+        prev = attach(ctx)
         yield ctx
       ensure
-        ctx.detach(prev)
+        detach(prev)
       end
 
       # Execute a block in a new context with key set to value. Restores the
@@ -58,10 +79,10 @@ def with_current(ctx)
       #   the block
       def with_value(key, value)
         ctx = current.set_value(key, value)
-        prev = ctx.attach
+        prev = attach(ctx)
         yield ctx, value
       ensure
-        ctx.detach(prev)
+        detach(prev)
       end
 
       # Execute a block in a new context where its values are merged with the
@@ -75,31 +96,26 @@ def with_value(key, value)
       #   to the block
       def with_values(values)
         ctx = current.set_values(values)
-        prev = ctx.attach
+        prev = attach(ctx)
         yield ctx, values
       ensure
-        ctx.detach(prev)
-      end
-
-      # Returns the value associated with key in the current context
-      #
-      # @param [String] key The lookup key
-      def value(key)
-        current.value(key)
+        detach(prev)
       end
 
       def clear
         self.current = ROOT
       end
 
       def empty
-        new(nil, EMPTY_ENTRIES)
+        new(EMPTY_ENTRIES)
       end
     end
 
-    def initialize(parent, entries)
-      @parent = parent
+    attr_accessor :parent
+
+    def initialize(entries)
       @entries = entries.freeze
+      @parent = parent
     end
 
     # Returns the corresponding value (or nil) for key
@@ -120,7 +136,7 @@ def value(key)
     def set_value(key, value)
       new_entries = @entries.dup
       new_entries[key] = value
-      Context.new(self, new_entries)
+      Context.new(new_entries)
     end
 
     # Returns a new Context with the current context's entries merged with the
@@ -131,22 +147,7 @@ def set_value(key, value)
     # @param [Object] value Object to be stored under key
     # @return [Context]
     def set_values(values) # rubocop:disable Naming/AccessorMethodName:
-      Context.new(self, @entries.merge(values))
-    end
-
-    # @api private
-    def attach
-      prev = self.class.current
-      self.class.current = self
-      prev
-    end
-
-    # @api private
-    def detach(ctx_to_attach = nil)
-      OpenTelemetry.logger.warn 'Calls to detach should match corresponding calls to attach' if self.class.current != self
-
-      ctx_to_attach ||= @parent || ROOT
-      ctx_to_attach.attach
+      Context.new(@entries.merge(values))
     end
 
     ROOT = empty.freeze

api/test/opentelemetry/context_test.rb

@@ -11,21 +11,122 @@
 describe OpenTelemetry::Context do
   Context = OpenTelemetry::Context
 
-  after do
-    Context.clear
-  end
+  after { Context.clear }
 
   let(:foo_key) { Context.create_key('foo') }
   let(:bar_key) { Context.create_key('bar') }
   let(:baz_key) { Context.create_key('baz') }
   let(:new_context) { Context.empty.set_value(foo_key, 'bar') }
 
+  describe '.create_key' do
+    it 'returns a Context::Key' do
+      key = Context.create_key('testing')
+      _(key).must_be_instance_of(Context::Key)
+      _(key.name).must_equal('testing')
+    end
+  end
+
   describe '.current' do
     it 'defaults to the root context' do
       _(Context.current).must_equal(Context::ROOT)
     end
   end
 
+  describe '.attach' do
+    it 'returns a reference to the previous context' do
+      previous_context = Context.attach(new_context)
+      _(previous_context).must_equal(Context::ROOT)
+    end
+
+    it 'sets the current context' do
+      c1 = new_context
+      Context.attach(c1)
+      _(Context.current).must_equal(c1)
+      _(Context.current[foo_key]).must_equal('bar')
+
+      c2 = Context.current.set_value(foo_key, 'c2')
+      Context.attach(c2)
+      _(Context.current).must_equal(c2)
+      _(Context.current[foo_key]).must_equal('c2')
+
+      c3 = Context.current.set_value(foo_key, 'c3')
+      Context.attach(c3)
+      _(Context.current).must_equal(c3)
+      _(Context.current[foo_key]).must_equal('c3')
+    end
+
+    it 'attaching the current context does not create a circular reference' do
+      Context.attach(new_context)
+      Context.attach(Context.current)
+      Context.detach
+      _(Context.current).must_equal(Context::ROOT)
+    end
+  end
+
+  describe '.detach' do
+    before do
+      @log_stream = StringIO.new
+      @_logger = OpenTelemetry.logger
+      OpenTelemetry.logger = ::Logger.new(@log_stream)
+    end
+
+    after do
+      OpenTelemetry.logger = @_logger
+    end
+
+    it 'restores the context' do
+      prev = Context.attach(new_context)
+      _(Context.current).must_equal(new_context)
+
+      Context.detach(prev)
+      _(Context.current).must_equal(Context::ROOT)
+
+      _(@log_stream.string).must_be_empty
+    end
+
+    it 'warns mismatched detach calls' do
+      c1 = new_context
+      Context.attach(c1)
+
+      c2 = Context.current.set_value(foo_key, 'c2')
+      c1_token = Context.attach(c2)
+
+      c3 = Context.current.set_value(foo_key, 'c3')
+      Context.attach(c3)
+
+      Context.detach(c1_token)
+
+      _(@log_stream.string).must_match(/Calls to detach should match corresponding calls to attach/)
+    end
+
+    it 'detaches to the parent if no context is provided' do
+      c1 = new_context
+      Context.attach(c1)
+
+      c2 = Context.current.set_value(foo_key, 'c2')
+      Context.attach(c2)
+
+      c3 = Context.current.set_value(foo_key, 'c3')
+      Context.attach(c3)
+
+      _(Context.current).must_equal(c3)
+
+      Context.detach
+      _(Context.current).must_equal(c2)
+
+      Context.detach
+      _(Context.current).must_equal(c1)
+
+      Context.detach
+      _(Context.current).must_equal(Context::ROOT)
+    end
+
+    it 'detaching at the root leaves the root as the current context' do
+      Context.detach
+      _(Context.current).must_equal(Context::ROOT)
+    end
+  end
+
   describe '.with_current' do
     it 'handles nested contexts' do
       c1 = new_context
@@ -85,13 +186,6 @@
     end
   end
 
-  describe '#value' do
-    it 'returns corresponding value for key' do
-      ctx = new_context
-      _(ctx.value(foo_key)).must_equal('bar')
-    end
-  end
-
   describe '.with_values' do
     it 'executes block within new context' do
       orig_ctx = Context.current
@@ -119,6 +213,33 @@
     end
   end
 
+  describe '.clear' do
+    it 'clears the context' do
+      Context.attach(new_context)
+      _(Context.current).must_equal(new_context)
+
+      Context.clear
+
+      _(Context.current).must_equal(Context::ROOT)
+    end
+  end
+
+  describe '#value' do
+    it 'returns corresponding value for key' do
+      ctx = new_context
+      _(ctx.value(foo_key)).must_equal('bar')
+    end
+  end
+
+  describe '#set_value' do
+    it 'returns new context with entry' do
+      c1 = Context.current
+      c2 = c1.set_value(foo_key, 'bar')
+      _(c1.value(foo_key)).must_be_nil
+      _(c2.value(foo_key)).must_equal('bar')
+    end
+  end
+
   describe '#set_values' do
     it 'assigns multiple values' do
       ctx = new_context
@@ -136,15 +257,6 @@
     end
   end
 
-  describe '#update' do
-    it 'returns new context with entry' do
-      c1 = Context.current
-      c2 = c1.set_value(foo_key, 'bar')
-      _(c1.value(foo_key)).must_be_nil
-      _(c2.value(foo_key)).must_equal('bar')
-    end
-  end
-
   describe 'threading' do
     it 'unwinds the stack on each thread' do
       ctx = new_context