From 62aebce70f866bb58dca25c3ce1a538ca24fdea2 Mon Sep 17 00:00:00 2001 From: Bart de Water <118401830+bdewater-thatch@users.noreply.github.com> Date: Tue, 21 Jul 2026 08:55:30 -0400 Subject: [PATCH] fix: Normalize UTF-8 attribute strings --- common/lib/opentelemetry/common/utilities.rb | 9 ++- .../opentelemetry/common/utilities_test.rb | 16 +++++ .../lib/opentelemetry/exporter/otlp/common.rb | 6 +- ...opentelemetry-exporter-otlp-common.gemspec | 1 + .../exporter/otlp/common/common_test.rb | 40 +++++++++++ .../exporter/otlp/logs/logs_exporter.rb | 5 +- .../exporter/otlp/logs_exporter_test.rb | 17 +++++ .../exporter/otlp/metrics/util.rb | 5 +- .../otlp/metrics/metrics_exporter_test.rb | 17 +++++ .../opentelemetry/exporter/otlp/exporter.rb | 5 +- .../exporter/otlp/exporter_test.rb | 28 ++++++++ logs_sdk/Gemfile | 1 + .../lib/opentelemetry/sdk/logs/log_record.rb | 2 +- .../opentelemetry/sdk/logs/log_record_test.rb | 33 +++++++++ .../instrument/asynchronous_instrument.rb | 3 +- .../instrument/synchronous_instrument.rb | 1 + .../sdk/metrics/instrument/counter_test.rb | 24 +++++++ sdk/lib/opentelemetry/sdk/internal.rb | 69 +++++++++++++++++++ .../opentelemetry/sdk/resources/resource.rb | 7 +- sdk/lib/opentelemetry/sdk/trace/span.rb | 16 +++-- sdk/lib/opentelemetry/sdk/trace/tracer.rb | 3 +- .../sdk/trace/tracer_provider.rb | 3 + .../sdk/resources/resource_test.rb | 19 +++++ sdk/test/opentelemetry/sdk/trace/span_test.rb | 36 ++++++++++ 24 files changed, 348 insertions(+), 18 deletions(-) diff --git a/common/lib/opentelemetry/common/utilities.rb b/common/lib/opentelemetry/common/utilities.rb index f6ed65c611..ab9cbc8095 100644 --- a/common/lib/opentelemetry/common/utilities.rb +++ b/common/lib/opentelemetry/common/utilities.rb @@ -54,9 +54,9 @@ def time_in_nanoseconds(timestamp = Time.now) # # @param [String] string The string to be utf8 encoded # @param [optional boolean] binary This option is for displaying binary data - # @param [optional String] placeholder The fallback string to be used if encoding fails + # @param [String, nil] placeholder The fallback value to be used if encoding fails # - # @return [String] + # @return [String, nil] def utf8_encode(string, binary: false, placeholder: STRING_PLACEHOLDER) string = string.to_s @@ -66,6 +66,11 @@ def utf8_encode(string, binary: false, placeholder: STRING_PLACEHOLDER) string.encode('UTF-8', 'binary', invalid: :replace, undef: :replace, replace: '') elsif string.encoding == ::Encoding::UTF_8 string + elsif string.encoding == ::Encoding::ASCII_8BIT + utf8_string = string.dup.force_encoding(::Encoding::UTF_8) + raise Encoding::InvalidByteSequenceError, 'binary string is not valid UTF-8' unless utf8_string.valid_encoding? + + utf8_string else string.encode(::Encoding::UTF_8) end diff --git a/common/test/opentelemetry/common/utilities_test.rb b/common/test/opentelemetry/common/utilities_test.rb index 827038fdf7..ac26460bc6 100644 --- a/common/test/opentelemetry/common/utilities_test.rb +++ b/common/test/opentelemetry/common/utilities_test.rb @@ -76,6 +76,22 @@ def shutdown(timeout: nil); end assert_equal('?', common_utils.utf8_encode(time_bomb, placeholder: '?')) end + it 'preserves valid UTF-8 bytes from a binary-encoded string' do + city = 'Montréal'.dup.force_encoding(::Encoding::ASCII_8BIT) + + encoded = common_utils.utf8_encode(city) + + assert_equal('Montréal', encoded) + assert_equal(::Encoding::UTF_8, encoded.encoding) + assert_equal(::Encoding::ASCII_8BIT, city.encoding) + end + + it 'does not validate an already UTF-8-tagged string' do + invalid = "\xC3".dup.force_encoding(::Encoding::UTF_8) + + assert_same(invalid, common_utils.utf8_encode(invalid, placeholder: '?')) + end + it 'with binary data' do byte_array = (+"keep what\xC2 is valid").force_encoding(::Encoding::ASCII_8BIT) diff --git a/exporter/otlp-common/lib/opentelemetry/exporter/otlp/common.rb b/exporter/otlp-common/lib/opentelemetry/exporter/otlp/common.rb index 07e63a46ad..2f907c6b93 100644 --- a/exporter/otlp-common/lib/opentelemetry/exporter/otlp/common.rb +++ b/exporter/otlp-common/lib/opentelemetry/exporter/otlp/common.rb @@ -5,6 +5,7 @@ # SPDX-License-Identifier: Apache-2.0 require 'opentelemetry' +require 'opentelemetry/common' require 'opentelemetry/exporter/otlp/common/version' require 'google/rpc/status_pb' @@ -130,9 +131,10 @@ def as_otlp_span_kind(kind) end def as_otlp_key_value(key, value) + key = OpenTelemetry::Common::Utilities.utf8_encode(key, placeholder: 'Encoding Error') Opentelemetry::Proto::Common::V1::KeyValue.new(key: key, value: as_otlp_any_value(value)) rescue Encoding::UndefinedConversionError => e - encoded_value = value.encode('UTF-8', invalid: :replace, undef: :replace, replace: '�') + encoded_value = value.to_s.encode('UTF-8', invalid: :replace, undef: :replace, replace: '�') OpenTelemetry.handle_error(exception: e, message: "encoding error for key #{key} and value #{encoded_value}") Opentelemetry::Proto::Common::V1::KeyValue.new(key: key, value: as_otlp_any_value('Encoding Error')) end @@ -141,7 +143,7 @@ def as_otlp_any_value(value) result = Opentelemetry::Proto::Common::V1::AnyValue.new case value when String - result.string_value = value + result.string_value = OpenTelemetry::Common::Utilities.utf8_encode(value, placeholder: value) when Integer result.int_value = value when Float diff --git a/exporter/otlp-common/opentelemetry-exporter-otlp-common.gemspec b/exporter/otlp-common/opentelemetry-exporter-otlp-common.gemspec index 864f1687b3..7962bae281 100644 --- a/exporter/otlp-common/opentelemetry-exporter-otlp-common.gemspec +++ b/exporter/otlp-common/opentelemetry-exporter-otlp-common.gemspec @@ -28,6 +28,7 @@ Gem::Specification.new do |spec| spec.add_dependency 'googleapis-common-protos-types', '~> 1.3' spec.add_dependency 'google-protobuf', '~> 3.19' spec.add_dependency 'opentelemetry-api', '~> 1.1' + spec.add_dependency 'opentelemetry-common', '~> 0.20' if spec.respond_to?(:metadata) spec.metadata['changelog_uri'] = "https://rubydoc.info/gems/#{spec.name}/#{spec.version}/file/CHANGELOG.md" diff --git a/exporter/otlp-common/test/opentelemetry/exporter/otlp/common/common_test.rb b/exporter/otlp-common/test/opentelemetry/exporter/otlp/common/common_test.rb index d91b0cd92b..5b32f587a4 100644 --- a/exporter/otlp-common/test/opentelemetry/exporter/otlp/common/common_test.rb +++ b/exporter/otlp-common/test/opentelemetry/exporter/otlp/common/common_test.rb @@ -61,6 +61,46 @@ _(result.resource_spans).must_be_empty end + it 'exports valid UTF-8 bytes from binary-encoded attribute strings' do + city = 'Montréal'.dup.force_encoding(::Encoding::ASCII_8BIT) + span_data = OpenTelemetry::TestHelpers.create_span_data( + total_recorded_attributes: 1, + attributes: { 'city' => city } + ) + + etsr = OpenTelemetry::Exporter::OTLP::Common.as_etsr([span_data]) + exported_span = etsr.resource_spans.first.scope_spans.first.spans.first + + _(exported_span.attributes.first.value.string_value).must_equal('Montréal') + end + + it 'safely exports attributes with invalid UTF-8 keys' do + invalid_key = "\xC2".dup.force_encoding(::Encoding::ASCII_8BIT) + span_data = OpenTelemetry::TestHelpers.create_span_data( + total_recorded_attributes: 1, + attributes: { invalid_key => 'value' } + ) + + etsr = OpenTelemetry::Exporter::OTLP::Common.as_etsr([span_data]) + exported_attribute = etsr.resource_spans.first.scope_spans.first.spans.first.attributes.first + + _(exported_attribute.key).must_equal('Encoding Error') + _(exported_attribute.value.string_value).must_equal('value') + end + + it 'safely exports arrays containing invalid UTF-8 strings' do + invalid_value = "\xC2".dup.force_encoding(::Encoding::ASCII_8BIT) + span_data = OpenTelemetry::TestHelpers.create_span_data( + total_recorded_attributes: 1, + attributes: { 'values' => [invalid_value] } + ) + + etsr = OpenTelemetry::Exporter::OTLP::Common.as_etsr([span_data]) + exported_value = etsr.resource_spans.first.scope_spans.first.spans.first.attributes.first.value + + _(exported_value.string_value).must_equal('Encoding Error') + end + it 'batches per resource and instrumentation scope' do # Test resource batching resource_one = OpenTelemetry::SDK::Resources::Resource.create('k1' => 'v1') diff --git a/exporter/otlp-logs/lib/opentelemetry/exporter/otlp/logs/logs_exporter.rb b/exporter/otlp-logs/lib/opentelemetry/exporter/otlp/logs/logs_exporter.rb index f9e9e670df..4f7da1727b 100644 --- a/exporter/otlp-logs/lib/opentelemetry/exporter/otlp/logs/logs_exporter.rb +++ b/exporter/otlp-logs/lib/opentelemetry/exporter/otlp/logs/logs_exporter.rb @@ -318,9 +318,10 @@ def as_otlp_log_record(log_record_data) end def as_otlp_key_value(key, value) + key = OpenTelemetry::Common::Utilities.utf8_encode(key, placeholder: 'Encoding Error') Opentelemetry::Proto::Common::V1::KeyValue.new(key: key, value: as_otlp_any_value(value)) rescue Encoding::UndefinedConversionError => e - encoded_value = value.encode('UTF-8', invalid: :replace, undef: :replace, replace: '�') + encoded_value = value.to_s.encode('UTF-8', invalid: :replace, undef: :replace, replace: '�') OpenTelemetry.handle_error(exception: e, message: "encoding error for key #{key} and value #{encoded_value}") Opentelemetry::Proto::Common::V1::KeyValue.new(key: key, value: as_otlp_any_value('Encoding Error')) end @@ -329,7 +330,7 @@ def as_otlp_any_value(value) # rubocop:disable Metrics/CyclomaticComplexity result = Opentelemetry::Proto::Common::V1::AnyValue.new case value when String - result.string_value = value + result.string_value = OpenTelemetry::Common::Utilities.utf8_encode(value, placeholder: value) when Integer result.int_value = value when Float diff --git a/exporter/otlp-logs/test/opentelemetry/exporter/otlp/logs_exporter_test.rb b/exporter/otlp-logs/test/opentelemetry/exporter/otlp/logs_exporter_test.rb index 4d37bef293..f5475f005a 100644 --- a/exporter/otlp-logs/test/opentelemetry/exporter/otlp/logs_exporter_test.rb +++ b/exporter/otlp-logs/test/opentelemetry/exporter/otlp/logs_exporter_test.rb @@ -649,6 +649,23 @@ OpenTelemetry.logger = logger end + it 'exports valid UTF-8 bytes from binary-encoded strings' do + city = 'Montréal'.dup.force_encoding(::Encoding::ASCII_8BIT) + + value = exporter.send(:as_otlp_any_value, city) + + _(value.string_value).must_equal('Montréal') + _(value.string_value.encoding).must_equal(::Encoding::UTF_8) + end + + it 'safely exports arrays containing invalid UTF-8 strings' do + invalid_value = "\xC2".dup.force_encoding(::Encoding::ASCII_8BIT) + + attribute = exporter.send(:as_otlp_key_value, 'values', [invalid_value]) + + _(attribute.value.string_value).must_equal('Encoding Error') + end + it 'logs rpc.Status on bad request' do log_stream = StringIO.new logger = OpenTelemetry.logger diff --git a/exporter/otlp-metrics/lib/opentelemetry/exporter/otlp/metrics/util.rb b/exporter/otlp-metrics/lib/opentelemetry/exporter/otlp/metrics/util.rb index 406a1afa10..c2a2b9583f 100644 --- a/exporter/otlp-metrics/lib/opentelemetry/exporter/otlp/metrics/util.rb +++ b/exporter/otlp-metrics/lib/opentelemetry/exporter/otlp/metrics/util.rb @@ -31,9 +31,10 @@ def around_request end def as_otlp_key_value(key, value) + key = OpenTelemetry::Common::Utilities.utf8_encode(key, placeholder: 'Encoding Error') Opentelemetry::Proto::Common::V1::KeyValue.new(key: key, value: as_otlp_any_value(value)) rescue Encoding::UndefinedConversionError => e - encoded_value = value.encode('UTF-8', invalid: :replace, undef: :replace, replace: '�') + encoded_value = value.to_s.encode('UTF-8', invalid: :replace, undef: :replace, replace: '�') OpenTelemetry.handle_error(exception: e, message: "encoding error for key #{key} and value #{encoded_value}") Opentelemetry::Proto::Common::V1::KeyValue.new(key: key, value: as_otlp_any_value('Encoding Error')) end @@ -42,7 +43,7 @@ def as_otlp_any_value(value) result = Opentelemetry::Proto::Common::V1::AnyValue.new case value when String - result.string_value = value + result.string_value = OpenTelemetry::Common::Utilities.utf8_encode(value, placeholder: value) when Integer result.int_value = value when Float diff --git a/exporter/otlp-metrics/test/opentelemetry/exporter/otlp/metrics/metrics_exporter_test.rb b/exporter/otlp-metrics/test/opentelemetry/exporter/otlp/metrics/metrics_exporter_test.rb index 1c57517bd1..9349270272 100644 --- a/exporter/otlp-metrics/test/opentelemetry/exporter/otlp/metrics/metrics_exporter_test.rb +++ b/exporter/otlp-metrics/test/opentelemetry/exporter/otlp/metrics/metrics_exporter_test.rb @@ -587,6 +587,23 @@ OpenTelemetry.logger = logger end + it 'exports valid UTF-8 bytes from binary-encoded attribute strings' do + city = 'Montréal'.dup.force_encoding(::Encoding::ASCII_8BIT) + + value = exporter.send(:as_otlp_any_value, city) + + _(value.string_value).must_equal('Montréal') + _(value.string_value.encoding).must_equal(::Encoding::UTF_8) + end + + it 'safely exports arrays containing invalid UTF-8 strings' do + invalid_value = "\xC2".dup.force_encoding(::Encoding::ASCII_8BIT) + + attribute = exporter.send(:as_otlp_key_value, 'values', [invalid_value]) + + _(attribute.value.string_value).must_equal('Encoding Error') + end + it 'is able to encode NumberDataPoint with Integer or Float value' do stub_request(:post, 'http://localhost:4318/v1/metrics').to_return(status: 200) diff --git a/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb b/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb index 68f6ceae18..990e8479ae 100644 --- a/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb +++ b/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb @@ -391,9 +391,10 @@ def as_otlp_span_kind(kind) end def as_otlp_key_value(key, value) + key = OpenTelemetry::Common::Utilities.utf8_encode(key, placeholder: 'Encoding Error') Opentelemetry::Proto::Common::V1::KeyValue.new(key: key, value: as_otlp_any_value(value)) rescue Encoding::UndefinedConversionError => e - encoded_value = value.encode('UTF-8', invalid: :replace, undef: :replace, replace: '�') + encoded_value = value.to_s.encode('UTF-8', invalid: :replace, undef: :replace, replace: '�') OpenTelemetry.handle_error(exception: e, message: "encoding error for key #{key} and value #{encoded_value}") Opentelemetry::Proto::Common::V1::KeyValue.new(key: key, value: as_otlp_any_value('Encoding Error')) end @@ -402,7 +403,7 @@ def as_otlp_any_value(value) result = Opentelemetry::Proto::Common::V1::AnyValue.new case value when String - result.string_value = value + result.string_value = OpenTelemetry::Common::Utilities.utf8_encode(value, placeholder: value) when Integer result.int_value = value when Float diff --git a/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb b/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb index e5a1dc5e0a..ce29a598bf 100644 --- a/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb +++ b/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb @@ -627,6 +627,34 @@ OpenTelemetry.logger = logger end + it 'exports valid UTF-8 bytes from binary-encoded attribute strings' do + city = 'Montréal'.dup.force_encoding(::Encoding::ASCII_8BIT) + span_data = OpenTelemetry::TestHelpers.create_span_data( + total_recorded_attributes: 1, + attributes: { 'city' => city } + ) + + encoded_data = exporter.send(:encode, [span_data]) + decoded = Opentelemetry::Proto::Collector::Trace::V1::ExportTraceServiceRequest.decode(encoded_data) + exported_span = decoded.resource_spans.first.scope_spans.first.spans.first + + _(exported_span.attributes.first.value.string_value).must_equal('Montréal') + end + + it 'safely exports arrays containing invalid UTF-8 strings' do + invalid_value = "\xC2".dup.force_encoding(::Encoding::ASCII_8BIT) + span_data = OpenTelemetry::TestHelpers.create_span_data( + total_recorded_attributes: 1, + attributes: { 'values' => [invalid_value] } + ) + + encoded_data = exporter.send(:encode, [span_data]) + decoded = Opentelemetry::Proto::Collector::Trace::V1::ExportTraceServiceRequest.decode(encoded_data) + exported_value = decoded.resource_spans.first.scope_spans.first.spans.first.attributes.first.value + + _(exported_value.string_value).must_equal('Encoding Error') + end + it 'logs rpc.Status on bad request' do log_stream = StringIO.new logger = OpenTelemetry.logger diff --git a/logs_sdk/Gemfile b/logs_sdk/Gemfile index edba27129d..56af4d1620 100644 --- a/logs_sdk/Gemfile +++ b/logs_sdk/Gemfile @@ -20,6 +20,7 @@ group :test, :development do gem 'yard', '~> 0.9.0' gem 'yard-doctest', '~> 0.1.17' gem 'opentelemetry-api', path: '../api', require: false + gem 'opentelemetry-common', path: '../common', require: false gem 'opentelemetry-exporter-otlp-logs', path: '../exporter/otlp-logs', require: false gem 'opentelemetry-logs-api', path: '../logs_api', require: false gem 'opentelemetry-sdk', path: '../sdk', require: false diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/log_record.rb b/logs_sdk/lib/opentelemetry/sdk/logs/log_record.rb index d2d6f0f544..964e6c5369 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/log_record.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/log_record.rb @@ -81,7 +81,7 @@ def initialize( @severity_text = severity_text @severity_number = severity_number @body = body - @attributes = attributes&.to_h # We need a mutable copy of attributes + @attributes = Internal.normalize_attribute_encodings(body, 'log record', attributes&.to_h) @event_name = event_name @trace_id = trace_id @span_id = span_id diff --git a/logs_sdk/test/opentelemetry/sdk/logs/log_record_test.rb b/logs_sdk/test/opentelemetry/sdk/logs/log_record_test.rb index dff217b520..787717bd99 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/log_record_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/log_record_test.rb @@ -136,6 +136,39 @@ end end + it 'normalizes valid UTF-8 bytes from a binary-encoded attribute string' do + city = 'Montréal'.dup.force_encoding(::Encoding::ASCII_8BIT) + + log_record = Logs::LogRecord.new(attributes: { 'city' => city }) + value = log_record.attributes['city'] + + assert_equal('Montréal', value) + assert_equal(::Encoding::UTF_8, value.encoding) + assert_equal(::Encoding::ASCII_8BIT, city.encoding) + end + + it 'normalizes valid UTF-8 bytes in attribute keys' do + key = 'Montréal'.dup.force_encoding(::Encoding::ASCII_8BIT) + + log_record = Logs::LogRecord.new(attributes: { key => 'city' }) + normalized_key = log_record.attributes.keys.first + + assert_equal('Montréal', normalized_key) + assert_equal(::Encoding::UTF_8, normalized_key.encoding) + assert_equal(::Encoding::ASCII_8BIT, key.encoding) + end + + it 'drops binary-encoded attribute strings that are not valid UTF-8' do + invalid = "\xC3".dup.force_encoding(::Encoding::ASCII_8BIT) + + OpenTelemetry::TestHelpers.with_test_logger do |log_stream| + log_record = Logs::LogRecord.new(attributes: { 'invalid' => invalid }) + + assert_empty(log_record.attributes) + assert_match(/invalid UTF-8 encoding.*invalid.*Dropping attribute/, log_stream.string) + end + end + it 'uses the default limits if none provided' do log_record = Logs::LogRecord.new default = Logs::LogRecordLimits::DEFAULT diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/asynchronous_instrument.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/asynchronous_instrument.rb index 19c212ecc8..5a16da67b1 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/asynchronous_instrument.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/asynchronous_instrument.rb @@ -82,7 +82,7 @@ def timeout(timeout) end def add_attributes(attributes) - @attributes.merge!(attributes) if attributes.instance_of?(Hash) + @attributes.merge!(Internal.normalize_attributes(@name, 'metric', attributes)) if attributes.instance_of?(Hash) end private @@ -90,6 +90,7 @@ def add_attributes(attributes) # update the observed value (after calling observe) # invoke callback will execute callback and export metric_data that is observed def update(timeout, attributes) + attributes = Internal.normalize_attributes(@name, 'metric', attributes) @metric_streams.each { |ms| ms.invoke_callback(timeout, attributes) } end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/synchronous_instrument.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/synchronous_instrument.rb index 0280021947..d772c3860a 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/synchronous_instrument.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/synchronous_instrument.rb @@ -44,6 +44,7 @@ def register_with_new_metric_store(metric_store, aggregation: default_aggregatio private def update(value, attributes) + attributes = Internal.normalize_attributes(@name, 'metric', attributes) @metric_streams.each { |ms| ms.update(value, attributes) } end end diff --git a/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/counter_test.rb b/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/counter_test.rb index 0cac97f4b3..e78ea26fd9 100644 --- a/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/counter_test.rb +++ b/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/counter_test.rb @@ -30,4 +30,28 @@ _(last_snapshot[0].data_points[0].attributes).must_equal('foo' => 'bar') _(last_snapshot[0].aggregation_temporality).must_equal(:cumulative) end + + it 'normalizes valid UTF-8 bytes in attributes' do + city = 'Montréal'.dup.force_encoding(::Encoding::ASCII_8BIT) + + counter.add(1, attributes: { 'city' => city }) + metric_exporter.pull + value = metric_exporter.metric_snapshots[0].data_points[0].attributes['city'] + + _(value).must_equal('Montréal') + _(value.encoding).must_equal(::Encoding::UTF_8) + _(city.encoding).must_equal(::Encoding::ASCII_8BIT) + end + + it 'drops attributes that are not valid UTF-8' do + invalid = "\xC3".dup.force_encoding(::Encoding::ASCII_8BIT) + + OpenTelemetry::TestHelpers.with_test_logger do |log_stream| + counter.add(1, attributes: { 'invalid' => invalid }) + metric_exporter.pull + + _(metric_exporter.metric_snapshots[0].data_points[0].attributes).must_be_empty + _(log_stream.string).must_match(/invalid UTF-8 encoding.*invalid.*Dropping attribute/) + end + end end diff --git a/sdk/lib/opentelemetry/sdk/internal.rb b/sdk/lib/opentelemetry/sdk/internal.rb index 81f488e056..2ead41f6b3 100644 --- a/sdk/lib/opentelemetry/sdk/internal.rb +++ b/sdk/lib/opentelemetry/sdk/internal.rb @@ -48,14 +48,83 @@ def valid_value?(value) valid_simple_value?(value) || valid_array_value?(value) end + # Returns an attribute value with non-UTF-8 strings normalized when they + # can be converted without replacement. UTF-8-tagged strings are unchanged. + # + # @param [String, Boolean, Numeric, Array] value + # @return [String, Boolean, Numeric, Array, nil] + def normalize_attribute_value(value) + case value + when String + OpenTelemetry::Common::Utilities.utf8_encode(value, placeholder: nil) + when Array + normalized = value.map { |element| normalize_attribute_value(element) } + normalized unless normalized.any?(&:nil?) + else + value + end + end + + # Normalizes string encodings without validating attribute value types. + # + # @param [Object] owner The telemetry object that owns the attributes + # @param [String] kind The telemetry object's kind for diagnostic messages + # @param [Hash, nil] attrs The attributes to normalize + # @return [Hash, nil] + def normalize_attribute_encodings(owner, kind, attrs) + return if attrs.nil? + + attrs.each_with_object({}) do |(key, value), normalized| + normalized_key = normalize_attribute_value(key) + normalized_value = normalize_attribute_value(value) + if normalized_key.nil? + OpenTelemetry.handle_error(message: "invalid UTF-8 encoding for #{kind} attribute key on #{kind} '#{owner}'. Dropping attribute.") + elsif normalized_value.nil? + OpenTelemetry.handle_error(message: "invalid UTF-8 encoding for #{kind} attribute '#{key}' on #{kind} '#{owner}'. Dropping attribute.") + else + normalized[normalized_key] = normalized_value + end + end + end + + # Validates attributes and normalizes their string encodings. + # + # @param [Object] owner The telemetry object that owns the attributes + # @param [String] kind The telemetry object's kind for diagnostic messages + # @param [Hash, nil] attrs The attributes to validate and normalize + # @return [Hash, nil] + def normalize_attributes(owner, kind, attrs) + return if attrs.nil? + + attrs.each_with_object({}) do |(key, value), normalized| + if !valid_key?(key) + OpenTelemetry.handle_error(message: "invalid #{kind} attribute key type #{key.class} on #{kind} '#{owner}'") + elsif (normalized_key = normalize_attribute_value(key)).nil? + OpenTelemetry.handle_error(message: "invalid UTF-8 encoding for #{kind} attribute key on #{kind} '#{owner}'. Dropping attribute.") + elsif !valid_value?(value) + OpenTelemetry.handle_error(message: "invalid #{kind} attribute value type #{value.class} for key '#{key}' on #{kind} '#{owner}'") + elsif (normalized_value = normalize_attribute_value(value)).nil? + OpenTelemetry.handle_error(message: "invalid UTF-8 encoding for #{kind} attribute '#{key}' on #{kind} '#{owner}'. Dropping attribute.") + else + normalized[normalized_key] = normalized_value + end + end + end + def valid_attributes?(owner, kind, attrs) attrs.nil? || attrs.each do |k, v| if !valid_key?(k) OpenTelemetry.handle_error(message: "invalid #{kind} attribute key type #{k.class} on span '#{owner}'") return false + elsif normalize_attribute_value(k).nil? + OpenTelemetry.handle_error(message: "invalid UTF-8 encoding for #{kind} attribute key on span '#{owner}'. Dropping attribute.") + return false elsif !valid_value?(v) OpenTelemetry.handle_error(message: "invalid #{kind} attribute value type #{v.class} for key '#{k}' on span '#{owner}'") return false + elsif normalize_attribute_value(v).nil? + OpenTelemetry.handle_error(message: "invalid UTF-8 encoding for #{kind} attribute '#{k}' on span '#{owner}'. Dropping attribute.") + return false end end diff --git a/sdk/lib/opentelemetry/sdk/resources/resource.rb b/sdk/lib/opentelemetry/sdk/resources/resource.rb index 53e55c5354..968dffee0b 100644 --- a/sdk/lib/opentelemetry/sdk/resources/resource.rb +++ b/sdk/lib/opentelemetry/sdk/resources/resource.rb @@ -24,7 +24,12 @@ def create(attributes = {}) raise ArgumentError, 'attribute keys must be strings' unless k.is_a?(String) raise ArgumentError, 'attribute values must be (array of) strings, integers, floats, or booleans' unless Internal.valid_value?(v) - memo[-k] = v.freeze + normalized_key = Internal.normalize_attribute_value(k) + normalized_value = Internal.normalize_attribute_value(v) + raise ArgumentError, 'attribute keys must contain valid UTF-8' if normalized_key.nil? + raise ArgumentError, 'attribute string values must contain valid UTF-8' if normalized_value.nil? + + memo[-normalized_key] = normalized_value.freeze end.freeze new(frozen_attributes) diff --git a/sdk/lib/opentelemetry/sdk/trace/span.rb b/sdk/lib/opentelemetry/sdk/trace/span.rb index 010fa21afe..4466a6b7e8 100644 --- a/sdk/lib/opentelemetry/sdk/trace/span.rb +++ b/sdk/lib/opentelemetry/sdk/trace/span.rb @@ -82,7 +82,7 @@ def set_attribute(key, value) OpenTelemetry.logger.warn('Calling set_attribute on an ended Span.') else @attributes ||= {} - @attributes[key] = value + @attributes.merge!(Internal.normalize_attributes(name, 'span', { key => value })) trim_span_attributes(@attributes) @total_recorded_attributes += 1 end @@ -110,7 +110,7 @@ def add_attributes(attributes) OpenTelemetry.logger.warn('Calling add_attributes on an ended Span.') else @attributes ||= {} - @attributes.merge!(attributes) + @attributes.merge!(Internal.normalize_attributes(name, 'span', attributes)) trim_span_attributes(@attributes) @total_recorded_attributes += attributes.size end @@ -142,7 +142,11 @@ def add_link(link) OpenTelemetry.logger.warn('Calling add_link on an ended Span.') else @links ||= [] - @links = trim_links(@links << link, @span_limits.link_count_limit, @span_limits.link_attribute_count_limit) + normalized_link = OpenTelemetry::Trace::Link.new( + link.span_context, + Internal.normalize_attributes(name, 'link', link.attributes) + ) + @links = trim_links(@links << normalized_link, @span_limits.link_count_limit, @span_limits.link_attribute_count_limit) @total_recorded_links += 1 end end @@ -168,6 +172,7 @@ def add_link(link) # # @return [self] returns itself def add_event(name, attributes: nil, timestamp: nil) + attributes = Internal.normalize_attributes(self.name, 'event', attributes) event = Event.new(name, truncate_attribute_values(attributes, @span_limits.event_attribute_length_limit), relative_timestamp(timestamp)) @mutex.synchronize do @@ -334,9 +339,12 @@ def initialize(context, parent_context, parent_span, name, kind, parent_span_id, @total_recorded_events = 0 @total_recorded_links = links&.size || 0 @total_recorded_attributes = attributes&.size || 0 - @attributes = attributes + @attributes = Internal.normalize_attributes(name, 'span', attributes) trim_span_attributes(@attributes) @events = nil + links = links&.map do |link| + OpenTelemetry::Trace::Link.new(link.span_context, Internal.normalize_attributes(name, 'link', link.attributes)) + end @links = trim_links(links, span_limits.link_count_limit, span_limits.link_attribute_count_limit) # Times are hard. Whenever an explicit timestamp is provided diff --git a/sdk/lib/opentelemetry/sdk/trace/tracer.rb b/sdk/lib/opentelemetry/sdk/trace/tracer.rb index e8fb72eb15..fcc7aa8008 100644 --- a/sdk/lib/opentelemetry/sdk/trace/tracer.rb +++ b/sdk/lib/opentelemetry/sdk/trace/tracer.rb @@ -21,7 +21,8 @@ class Tracer < OpenTelemetry::Trace::Tracer # # @return [Tracer] def initialize(name, version, tracer_provider, attributes: nil) - @instrumentation_scope = InstrumentationScope.new(name, version, attributes || {}.freeze) + attributes = Internal.normalize_attributes(name, 'instrumentation scope', attributes) || {} + @instrumentation_scope = InstrumentationScope.new(name, version, attributes.freeze) @tracer_provider = tracer_provider end diff --git a/sdk/lib/opentelemetry/sdk/trace/tracer_provider.rb b/sdk/lib/opentelemetry/sdk/trace/tracer_provider.rb index b64cc2a707..232fbef171 100644 --- a/sdk/lib/opentelemetry/sdk/trace/tracer_provider.rb +++ b/sdk/lib/opentelemetry/sdk/trace/tracer_provider.rb @@ -156,12 +156,15 @@ def internal_start_span(name, kind, attributes, links, start_timestamp, parent_c return OpenTelemetry::Trace.non_recording_span(OpenTelemetry::Trace::SpanContext.new(trace_id: trace_id, span_id: span_id)) end + # Samplers observe the caller's attributes and links unchanged. Normalize only after sampling, + # when the values become recorded SDK state. result = @sampler.should_sample?(trace_id: trace_id, parent_context: parent_context, links: links, name: name, kind: kind, attributes: attributes) span_id = @id_generator.generate_span_id if result.recording? && !@stopped trace_flags = result.sampled? ? OpenTelemetry::Trace::TraceFlags::SAMPLED : OpenTelemetry::Trace::TraceFlags::DEFAULT context = OpenTelemetry::Trace::SpanContext.new(trace_id: trace_id, span_id: span_id, trace_flags: trace_flags, tracestate: result.tracestate) attributes = attributes&.merge(result.attributes) || result.attributes.dup + attributes = Internal.normalize_attributes(name, 'span', attributes) Span.new( context, parent_context, diff --git a/sdk/test/opentelemetry/sdk/resources/resource_test.rb b/sdk/test/opentelemetry/sdk/resources/resource_test.rb index fdbd91d4ea..93d55b009a 100644 --- a/sdk/test/opentelemetry/sdk/resources/resource_test.rb +++ b/sdk/test/opentelemetry/sdk/resources/resource_test.rb @@ -16,6 +16,25 @@ end describe '.create' do + it 'normalizes valid UTF-8 bytes from a binary-encoded string' do + city = 'Montréal'.dup.force_encoding(::Encoding::ASCII_8BIT) + + resource = Resource.create('city' => city) + value = resource.attribute_enumerator.to_h['city'] + + _(value).must_equal('Montréal') + _(value.encoding).must_equal(::Encoding::UTF_8) + _(city.encoding).must_equal(::Encoding::ASCII_8BIT) + end + + it 'rejects binary-encoded strings that are not valid UTF-8' do + invalid = "\xC3".dup.force_encoding(::Encoding::ASCII_8BIT) + + error = _ { Resource.create('invalid' => invalid) }.must_raise(ArgumentError) + + _(error.message).must_match(/valid UTF-8/) + end + it 'can be initialized with attributes' do expected_attributes = { 'k1' => 'v1', 'k2' => 'v2' } resource = Resource.create(expected_attributes) diff --git a/sdk/test/opentelemetry/sdk/trace/span_test.rb b/sdk/test/opentelemetry/sdk/trace/span_test.rb index f5c97c43f9..39e1813ce6 100644 --- a/sdk/test/opentelemetry/sdk/trace/span_test.rb +++ b/sdk/test/opentelemetry/sdk/trace/span_test.rb @@ -59,6 +59,42 @@ _(span.attributes).must_equal('foo' => 'bar') end + it 'normalizes valid UTF-8 bytes from a binary-encoded string' do + city = 'Montréal'.dup.force_encoding(::Encoding::ASCII_8BIT) + + span.set_attribute('city', city) + + value = span.attributes['city'] + _(value).must_equal('Montréal') + _(value.encoding).must_equal(::Encoding::UTF_8) + _(city.encoding).must_equal(::Encoding::ASCII_8BIT) + end + + it 'normalizes valid UTF-8 bytes in attribute keys and arrays' do + key = 'Montréal'.dup.force_encoding(::Encoding::ASCII_8BIT) + values = ['Québec'.dup.force_encoding(::Encoding::ASCII_8BIT)] + + span.set_attribute(key, values) + + normalized_key = span.attributes.keys.first + normalized_value = span.attributes.values.first.first + _(normalized_key.encoding).must_equal(::Encoding::UTF_8) + _(normalized_value.encoding).must_equal(::Encoding::UTF_8) + _(key.encoding).must_equal(::Encoding::ASCII_8BIT) + _(values.first.encoding).must_equal(::Encoding::ASCII_8BIT) + end + + it 'drops binary-encoded strings that are not valid UTF-8' do + invalid = "\xC3".dup.force_encoding(::Encoding::ASCII_8BIT) + + OpenTelemetry::TestHelpers.with_test_logger do |log_stream| + span.set_attribute('invalid', invalid) + + _(span.attributes).must_be_empty + _(log_stream.string).must_match(/invalid UTF-8 encoding.*invalid.*Dropping attribute/) + end + end + it 'trims the newest attribute' do span.set_attribute('old', 'oldbar') span.set_attribute('foo', 'bar')