Metrics SDK synchronous instruments (#1108)

* WIP: Metrics SDK implementation

* Extend the existing OpenTelemetry SDK

* fix: remove duplicate mutex instantiation

* feat: add api metrics measurement

* fix: add metric install sdk tests

* fix: comments

Co-authored-by: Francis Bogsanyi <[email protected]>

* fix: add meter provider implementation and tests

* fix: ruby style

* Implement SDK instrument creation

* fix: handling of immutable descriptions

* wip: safety commit

* chore: rubocops

* fix: add missing require

* Synchronous instruments

* chore: remove exporter work from PR

* Metric Stream refactor

* Remove metrics test helper

* feedback fixups

* Remove console exporter

* Add integration test assertions

* fix: broken test assert

* chore: appease the cop

* chore: appease the cop

* fix: francis feedback

Co-authored-by: Francis Bogsanyi <[email protected]>

Author: robertlaurin

metrics_api/lib/opentelemetry-metrics-api.rb

@@ -17,6 +17,8 @@
 #
 # The OpenTelemetry module provides global accessors for telemetry objects.
 module OpenTelemetry
+  @meter_provider = Internal::ProxyMeterProvider.new
+
   # Register the global meter provider.
   #
   # @param [MeterProvider] provider A meter provider to register as the

metrics_api/lib/opentelemetry/internal/proxy_meter.rb

@@ -29,7 +29,7 @@ def delegate=(meter)
         @mutex.synchronize do
           if @delegate.nil?
             @delegate = meter
-            @registry.each_value { |instrument| instrument.upgrade_with(meter) }
+            @instrument_registry.each_value { |instrument| instrument.upgrade_with(meter) }
           else
             OpenTelemetry.logger.warn 'Attempt to reset delegate in ProxyMeter ignored.'
           end

metrics_api/lib/opentelemetry/metrics.rb

@@ -11,5 +11,6 @@ module Metrics
 end
 
 require 'opentelemetry/metrics/instrument'
+require 'opentelemetry/metrics/measurement'
 require 'opentelemetry/metrics/meter'
 require 'opentelemetry/metrics/meter_provider'

metrics_api/lib/opentelemetry/metrics/measurement.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+# Copyright The OpenTelemetry Authors
+#
+# SPDX-License-Identifier: Apache-2.0
+
+module OpenTelemetry
+  module Metrics
+    Measurement = Struct.new(:value, :attributes)
+  end
+end

metrics_api/lib/opentelemetry/metrics/meter.rb

@@ -26,29 +26,29 @@ class Meter
 
       def initialize
         @mutex = Mutex.new
-        @registry = {}
+        @instrument_registry = {}
       end
 
       def create_counter(name, unit: nil, description: nil)
         create_instrument(:counter, name, unit, description, nil) { COUNTER }
       end
 
-      def create_observable_counter(name, unit: nil, description: nil, callback:)
-        create_instrument(:observable_counter, name, unit, description, callback) { OBSERVABLE_COUNTER }
-      end
-
       def create_histogram(name, unit: nil, description: nil)
         create_instrument(:histogram, name, unit, description, nil) { HISTOGRAM }
       end
 
-      def create_observable_gauge(name, unit: nil, description: nil, callback:)
-        create_instrument(:observable_gauge, name, unit, description, callback) { OBSERVABLE_GAUGE }
-      end
-
       def create_up_down_counter(name, unit: nil, description: nil)
         create_instrument(:up_down_counter, name, unit, description, nil) { UP_DOWN_COUNTER }
       end
 
+      def create_observable_counter(name, unit: nil, description: nil, callback:)
+        create_instrument(:observable_counter, name, unit, description, callback) { OBSERVABLE_COUNTER }
+      end
+
+      def create_observable_gauge(name, unit: nil, description: nil, callback:)
+        create_instrument(:observable_gauge, name, unit, description, callback) { OBSERVABLE_GAUGE }
+      end
+
       def create_observable_up_down_counter(name, unit: nil, description: nil, callback:)
         create_instrument(:observable_up_down_counter, name, unit, description, callback) { OBSERVABLE_UP_DOWN_COUNTER }
       end
@@ -60,12 +60,12 @@ def create_instrument(kind, name, unit, description, callback)
         raise InstrumentNameError if name.empty?
         raise InstrumentNameError unless NAME_REGEX.match?(name)
         raise InstrumentUnitError if unit && (!unit.ascii_only? || unit.size > 63)
-        raise InstrumentDescriptionError if description && (description.size > 1023 || !utf8mb3_encoding?(description))
+        raise InstrumentDescriptionError if description && (description.size > 1023 || !utf8mb3_encoding?(description.dup))
 
         @mutex.synchronize do
-          raise DuplicateInstrumentError if @registry.include? name
+          OpenTelemetry.logger.warn("duplicate instrument registration occurred for instrument #{name}") if @instrument_registry.include? name
 
-          @registry[name] = yield
+          @instrument_registry[name] = yield
         end
       end
 

metrics_api/opentelemetry-metrics-api.gemspec

@@ -31,6 +31,7 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'bundler', '>= 1.17'
   spec.add_development_dependency 'faraday', '~> 0.13'
   spec.add_development_dependency 'minitest', '~> 5.0'
+  spec.add_development_dependency 'opentelemetry-test-helpers'
   spec.add_development_dependency 'rake', '~> 12.0'
   spec.add_development_dependency 'rubocop', '~> 0.73.0'
   spec.add_development_dependency 'simplecov', '~> 0.17'

metrics_api/test/opentelemetry/metrics/meter_test.rb

@@ -16,10 +16,12 @@
   let(:meter) { meter_provider.meter('test-meter') }
 
   describe 'creating an instrument' do
-    it 'instrument name must be unique' do
-      meter.create_counter('a_counter')
-      _(-> { meter.create_counter('a_counter') }).must_raise(DUPLICATE_INSTRUMENT_ERROR)
-      _(-> { meter.create_histogram('a_counter') }).must_raise(DUPLICATE_INSTRUMENT_ERROR)
+    it 'duplicate instrument registration logs a warning' do
+      OpenTelemetry::TestHelpers.with_test_logger do |log_stream|
+        meter.create_counter('a_counter')
+        meter.create_counter('a_counter')
+        _(log_stream.string).must_match(/duplicate instrument registration occurred for instrument a_counter/)
+      end
     end
 
     it 'instrument name must not be nil' do

metrics_api/test/test_helper.rb

@@ -8,6 +8,7 @@
 # # SimpleCov.start
 # # SimpleCov.minimum_coverage 85
 
+require 'opentelemetry-test-helpers'
 require 'opentelemetry-metrics-api'
 require 'minitest/autorun'
 require 'pry'

metrics_sdk/.rubocop.yml

@@ -0,0 +1,22 @@
+AllCops:
+  TargetRubyVersion: "2.6.0"
+
+Lint/UnusedMethodArgument:
+  Enabled: false
+Metrics/AbcSize:
+  Max: 30
+Metrics/LineLength:
+  Enabled: false
+Metrics/MethodLength:
+  Max: 50
+Metrics/PerceivedComplexity:
+  Max: 30
+Metrics/CyclomaticComplexity:
+  Max: 20
+Metrics/ParameterLists:
+  Enabled: false
+Naming/FileName:
+  Exclude:
+    - "lib/opentelemetry-metrics-sdk.rb"
+Style/ModuleFunction:
+  Enabled: false

metrics_sdk/.yardopts

@@ -0,0 +1,9 @@
+--no-private
+--title=OpenTelemetry Metrics SDK
+--markup=markdown
+--main=README.md
+./lib/opentelemetry/**/*.rb
+./lib/opentelemetry.rb
+-
+README.md
+CHANGELOG.md