Skip to content

Commit

Permalink
feat: support InstrumentationScope, and update OTLP proto to 0.18.0 (#…
Browse files Browse the repository at this point in the history
…1345)

* feat: support InstrumentationScope, and update OTLP proto to 0.18.0

This commit accomplishes two things simultaneously - adding support for
InstrumentationScope everywhere, and also updating the OTLP proto to v0.18.0.

The PR that introduced InstrumentationScope is [here](open-telemetry/opentelemetry-specification#2276)

My best understanding (which is implemented in this PR) is that:
- Our OTLP export requests should group by InstrumentationScope instead
  of InstrumentationLibrary
- We must be able to support accessing the InstrumentationLibrary from
  anywhere a ReadableSpan is available, and that it should represent the
  `name` and `version` fields from the InstrumentationScope.
- When creating a tracer, we create and store an InstrumentationScope
  rather than an InstrumentationLibrary.
- Non-OTLP exporters must export both `otlp.scope.{name,version}` AND
  `otlp.library.{name,version}` tags for backwards compatibility.

Some notes that may be interesting:
- I chose to keep the original definition of `InstrumentationLibrary`
  around for now.
- I chose to have `Span#instrumentation_library` and
  `SpanData#instrumentation_library` create and memoize an `InstrumentationLibrary`
  on-demand when someone asks for it (and marked the method as
  deprecated in the YARD docs).
- I chose *not* to reference that deprecated helper when modifying the
  zipkin and jaeger exporters, for performance reasons.

* Update sdk/test/opentelemetry/sdk/trace/span_test.rb

Co-authored-by: Francis Bogsanyi <francis.bogsanyi@shopify.com>

* Update sdk/lib/opentelemetry/sdk/trace/span_data.rb

Co-authored-by: Francis Bogsanyi <francis.bogsanyi@shopify.com>

* Update sdk/lib/opentelemetry/sdk/trace/span.rb

Co-authored-by: Francis Bogsanyi <francis.bogsanyi@shopify.com>

* fixup: use alias_method

* fixup: add deprecation notice to InstrumentationLibrary

* fixup: argh, use alias in class scope

* Update sdk/lib/opentelemetry/sdk/trace/span.rb

Co-authored-by: Francis Bogsanyi <francis.bogsanyi@shopify.com>

* Update sdk/lib/opentelemetry/sdk/trace/span.rb

Co-authored-by: Francis Bogsanyi <francis.bogsanyi@shopify.com>

* Update sdk/lib/opentelemetry/sdk/trace/span_data.rb

Co-authored-by: Francis Bogsanyi <francis.bogsanyi@shopify.com>

* fixup: update Rakefile to regenerate protos correctly

It now checks out a specific tag, for starters.

* bundle exec rake update_protobuf

Co-authored-by: Francis Bogsanyi <francis.bogsanyi@shopify.com>
  • Loading branch information
ahayworth and fbogsany authored Aug 22, 2022
1 parent 5b0cee9 commit 03e36ce
Show file tree
Hide file tree
Showing 53 changed files with 325 additions and 359 deletions.
18 changes: 13 additions & 5 deletions exporter/jaeger/lib/opentelemetry/exporter/jaeger/encoder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def encoded_span(span_data) # rubocop:disable Metrics/MethodLength
tags = encoded_tags(span_data.attributes) +
encoded_status(span_data.status) +
encoded_kind(span_data.kind) +
encoded_instrumentation_library(span_data.instrumentation_library)
encoded_instrumentation_scope(span_data.instrumentation_scope)

dropped_attributes_count = span_data.total_recorded_attributes - span_data.attributes&.size.to_i
dropped_events_count = span_data.total_recorded_events - span_data.events&.size.to_i
Expand Down Expand Up @@ -141,12 +141,20 @@ def encoded_status(status)
)
end

def encoded_instrumentation_library(instrumentation_library)
return EMPTY_ARRAY unless instrumentation_library
def encoded_instrumentation_scope(instrumentation_scope)
return EMPTY_ARRAY unless instrumentation_scope

tags = []
tags << encoded_tag('otel.library.name', instrumentation_library.name) if instrumentation_library.name
tags << encoded_tag('otel.library.version', instrumentation_library.version) if instrumentation_library.version
if instrumentation_scope.name
tags << encoded_tag('otel.scope.name', instrumentation_scope.name)
tags << encoded_tag('otel.library.name', instrumentation_scope.name)
end

if instrumentation_scope.version
tags << encoded_tag('otel.scope.version', instrumentation_scope.version)
tags << encoded_tag('otel.library.version', instrumentation_scope.version)
end

tags
end

Expand Down
39 changes: 24 additions & 15 deletions exporter/jaeger/test/opentelemetry/exporter/jaeger/encoder_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,38 +119,47 @@
)
end

describe 'instrumentation library' do
it 'encodes library and version when set' do
lib = OpenTelemetry::SDK::InstrumentationLibrary.new('mylib', '0.1.0')
span_data = create_span_data(instrumentation_library: lib)
describe 'instrumentation scope' do
it 'encodes name and version when set, with backwards-compat tags' do
lib = OpenTelemetry::SDK::InstrumentationScope.new('mylib', '0.1.0')
span_data = create_span_data(instrumentation_scope: lib)
encoded_span = Encoder.encoded_span(span_data)

_(encoded_span.tags.size).must_equal(2)
_(encoded_span.tags.size).must_equal(4)

name_tag, version_tag = encoded_span.tags
name_tag, old_name_tag, version_tag, old_version_tag = encoded_span.tags

_(name_tag.key).must_equal('otel.library.name')
_(name_tag.key).must_equal('otel.scope.name')
_(name_tag.vStr).must_equal('mylib')

_(version_tag.key).must_equal('otel.library.version')
_(old_name_tag.key).must_equal('otel.library.name')
_(old_name_tag.vStr).must_equal('mylib')

_(version_tag.key).must_equal('otel.scope.version')
_(version_tag.vStr).must_equal('0.1.0')

_(old_version_tag.key).must_equal('otel.library.version')
_(old_version_tag.vStr).must_equal('0.1.0')
end

it 'skips nil values' do
lib = OpenTelemetry::SDK::InstrumentationLibrary.new('mylib')
span_data = create_span_data(instrumentation_library: lib)
lib = OpenTelemetry::SDK::InstrumentationScope.new('mylib')
span_data = create_span_data(instrumentation_scope: lib)
encoded_span = Encoder.encoded_span(span_data)

_(encoded_span.tags.size).must_equal(1)
_(encoded_span.tags.size).must_equal(2)

name_tag, = encoded_span.tags
name_tag, old_name_tag = encoded_span.tags

_(name_tag.key).must_equal('otel.library.name')
_(name_tag.key).must_equal('otel.scope.name')
_(name_tag.vStr).must_equal('mylib')

_(old_name_tag.key).must_equal('otel.library.name')
_(old_name_tag.vStr).must_equal('mylib')
end
end

def create_span_data(status: nil, kind: nil, attributes: nil, total_recorded_attributes: 0, events: nil, total_recorded_events: 0, links: nil, total_recorded_links: 0, instrumentation_library: nil, trace_id: OpenTelemetry::Trace.generate_trace_id, trace_flags: OpenTelemetry::Trace::TraceFlags::DEFAULT, tracestate: nil)
def create_span_data(status: nil, kind: nil, attributes: nil, total_recorded_attributes: 0, events: nil, total_recorded_events: 0, links: nil, total_recorded_links: 0, instrumentation_scope: nil, trace_id: OpenTelemetry::Trace.generate_trace_id, trace_flags: OpenTelemetry::Trace::TraceFlags::DEFAULT, tracestate: nil)
OpenTelemetry::SDK::Trace::SpanData.new(
'',
kind,
Expand All @@ -165,7 +174,7 @@ def create_span_data(status: nil, kind: nil, attributes: nil, total_recorded_att
links,
events,
nil,
instrumentation_library,
instrumentation_scope,
OpenTelemetry::Trace.generate_span_id,
trace_id,
trace_flags,
Expand Down
4 changes: 2 additions & 2 deletions exporter/jaeger/test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@

def create_span_data(name: '', kind: nil, status: nil, parent_span_id: OpenTelemetry::Trace::INVALID_SPAN_ID,
total_recorded_attributes: 0, total_recorded_events: 0, total_recorded_links: 0, start_timestamp: OpenTelemetry::TestHelpers.exportable_timestamp,
end_timestamp: OpenTelemetry::TestHelpers.exportable_timestamp, attributes: nil, links: nil, events: nil, resource: nil, instrumentation_library: nil,
end_timestamp: OpenTelemetry::TestHelpers.exportable_timestamp, attributes: nil, links: nil, events: nil, resource: nil, instrumentation_scope: nil,
span_id: OpenTelemetry::Trace.generate_span_id, trace_id: OpenTelemetry::Trace.generate_trace_id,
trace_flags: OpenTelemetry::Trace::TraceFlags::DEFAULT, tracestate: nil)
OpenTelemetry::SDK::Trace::SpanData.new(name, kind, status, parent_span_id, total_recorded_attributes,
total_recorded_events, total_recorded_links, start_timestamp, end_timestamp,
attributes, links, events, resource, instrumentation_library, span_id, trace_id, trace_flags, tracestate)
attributes, links, events, resource, instrumentation_scope, span_id, trace_id, trace_flags, tracestate)
end
10 changes: 8 additions & 2 deletions exporter/otlp-common/Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,21 @@ else
task default: %i[test rubocop yard]
end

PROTO_VERSION = 'v0.18.0'
PROTOBUF_FILES = [
'collector/logs/v1/logs_service.proto',
'collector/metrics/v1/metrics_service.proto',
'collector/trace/v1/trace_service.proto',
'common/v1/common.proto',
'logs/v1/logs.proto',
'metrics/v1/metrics.proto',
'resource/v1/resource.proto',
'trace/v1/trace.proto',
'collector/trace/v1/trace_service.proto'
'trace/v1/trace_config.proto'
].freeze

task :update_protobuf do
system('git clone /~https://github.com/open-telemetry/opentelemetry-proto')
system("git clone -b #{PROTO_VERSION} /~https://github.com/open-telemetry/opentelemetry-proto")
PROTOBUF_FILES.each do |file|
system("protoc --ruby_out=lib/ --proto_path=opentelemetry-proto opentelemetry/proto/#{file}")
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ def as_etsr(span_data) # rubocop:disable Metrics/MethodLength
resource: Opentelemetry::Proto::Resource::V1::Resource.new(
attributes: resource.attribute_enumerator.map { |key, value| as_otlp_key_value(key, value) }
),
instrumentation_library_spans: span_datas
.group_by(&:instrumentation_library)
scope_spans: span_datas
.group_by(&:instrumentation_scope)
.map do |il, sds|
Opentelemetry::Proto::Trace::V1::InstrumentationLibrarySpans.new(
instrumentation_library: Opentelemetry::Proto::Common::V1::InstrumentationLibrary.new(
Opentelemetry::Proto::Trace::V1::ScopeSpans.new(
scope: Opentelemetry::Proto::Common::V1::InstrumentationScope.new(
name: il.name,
version: il.version
),
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 11 additions & 3 deletions exporter/otlp-common/lib/opentelemetry/proto/logs/v1/logs_pb.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

This file was deleted.

Loading

0 comments on commit 03e36ce

Please sign in to comment.