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.

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

Remove support for lazy event creation #329

Merged
merged 8 commits into from
Aug 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ tracer.in_span('foo') do |span|
# set an attribute
span.set_attribute('platform', 'osx')
# add an event
span.add_event(name: 'event in bar')
span.add_event('event in bar')
# create bar as child of foo
tracer.in_span('bar') do |child_span|
# inspect the span
Expand Down
2 changes: 1 addition & 1 deletion api/benchmarks/span_bench.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
end

x.report 'add_event' do
span.add_event(name: 'test event', attributes: attributes)
span.add_event('test event', attributes: attributes)
end

x.compare!
Expand Down
1 change: 0 additions & 1 deletion api/lib/opentelemetry/trace.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ def self.generate_span_id
end
end

require 'opentelemetry/trace/event'
require 'opentelemetry/trace/link'
require 'opentelemetry/trace/trace_flags'
require 'opentelemetry/trace/span_context'
Expand Down
46 changes: 0 additions & 46 deletions api/lib/opentelemetry/trace/event.rb

This file was deleted.

20 changes: 6 additions & 14 deletions api/lib/opentelemetry/trace/span.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,34 +58,26 @@ def set_attribute(key, value)
end
alias []= set_attribute

# Add an Event to a {Span}. This can be accomplished eagerly or lazily.
# Lazy evaluation is useful when the event attributes are expensive to
# build and where the cost can be avoided for an unsampled {Span}.
# Add an event to a {Span}.
#
# Eager example:
# Example:
#
# span.add_event(name: 'event', attributes: {'eager' => true})
#
# Lazy example:
#
# span.add_event { tracer.create_event(name: 'event', attributes: {'eager' => false}) }
# span.add_event('event', attributes: {'eager' => true})
#
# Note that the OpenTelemetry project
# {https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md
# documents} certain "standard event names and keys" which have
# prescribed semantic meanings.
#
# @param [optional String] name Optional name of the event. This is
# required if a block is not given.
# @param [String] name Name of the event.
# @param [optional Hash{String => String, Numeric, Boolean, Array<String, Numeric, Boolean>}]
# attributes One or more key:value pairs, where the keys must be
# strings and the values may be (array of) string, boolean or numeric
# type. This argument should only be used when passing in a name.
# type.
# @param [optional Time] timestamp Optional timestamp for the event.
# This argument should only be used when passing in a name.
#
# @return [self] returns itself
def add_event(name: nil, attributes: nil, timestamp: nil)
def add_event(name, attributes: nil, timestamp: nil)
self
end

Expand Down
12 changes: 4 additions & 8 deletions api/test/opentelemetry/trace/span_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,19 @@

describe '#add_event' do
it 'returns self' do
_(span.add_event(name: 'event-name')).must_equal(span)
_(span.add_event('event-name')).must_equal(span)
end

it 'accepts a name and attributes' do
_(span.add_event(name: 'event-name', attributes: { 'foo' => 'bar' })).must_equal(span)
_(span.add_event('event-name', attributes: { 'foo' => 'bar' })).must_equal(span)
end

it 'accepts array-valued attributes' do
_(span.add_event(name: 'event-name', attributes: { 'foo' => [1, 2, 3] })).must_equal(span)
_(span.add_event('event-name', attributes: { 'foo' => [1, 2, 3] })).must_equal(span)
end

it 'accepts a timestamp' do
_(span.add_event(name: 'event-name', timestamp: Time.now)).must_equal(span)
end

it 'accepts an event formatter' do
_(span.add_event { Object.new }).must_equal(span)
_(span.add_event('event-name', timestamp: Time.now)).must_equal(span)
end
end

Expand Down
2 changes: 1 addition & 1 deletion exporters/jaeger/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ tracer.in_span('foo') do |span|
# set an attribute
span.set_attribute('platform', 'osx')
# add an event
span.add_event(name: 'event in bar')
span.add_event('event in bar')
# create bar as child of foo
tracer.in_span('bar') do |child_span|
# inspect the span
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
it 'encodes attributes in events and the span' do
attributes = { 'akey' => 'avalue' }
events = [
OpenTelemetry::Trace::Event.new(
OpenTelemetry::SDK::Trace::Event.new(
name: 'event', attributes: { 'ekey' => 'evalue' }
)
]
Expand All @@ -40,7 +40,7 @@
it 'encodes array attribute values in events and the span as JSON strings' do
attributes = { 'akey' => ['avalue'] }
events = [
OpenTelemetry::Trace::Event.new(
OpenTelemetry::SDK::Trace::Event.new(
name: 'event', attributes: { 'ekey' => ['evalue'] }
)
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def call(_worker_class, job, _queue, _redis_pool)
kind: :producer
) do |span|
OpenTelemetry.propagation.text.inject(job)
span.add_event(name: 'created_at', timestamp: job['created_at'])
span.add_event('created_at', timestamp: job['created_at'])
yield
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def call(_worker, msg, _queue)
with_parent_context: parent_context,
kind: :consumer
) do |span|
span.add_event(name: 'created_at', timestamp: msg['created_at'])
span.add_event(name: 'enqueued_at', timestamp: msg['enqueued_at'])
span.add_event('created_at', timestamp: msg['created_at'])
span.add_event('enqueued_at', timestamp: msg['enqueued_at'])
yield
end
end
Expand Down
2 changes: 1 addition & 1 deletion sdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ tracer.in_span('foo') do |span|
# set an attribute
span.set_attribute('platform', 'osx')
# add an event
span.add_event(name: 'event in bar')
span.add_event('event in bar')
# create bar as child of foo
tracer.in_span('bar') do |child_span|
# inspect the span
Expand Down
1 change: 1 addition & 0 deletions sdk/lib/opentelemetry/sdk/trace.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ module Trace

require 'opentelemetry/sdk/trace/samplers'
require 'opentelemetry/sdk/trace/config'
require 'opentelemetry/sdk/trace/event'
require 'opentelemetry/sdk/trace/export'
require 'opentelemetry/sdk/trace/multi_span_processor'
require 'opentelemetry/sdk/trace/noop_span_processor'
Expand Down
4 changes: 2 additions & 2 deletions sdk/lib/opentelemetry/sdk/trace/config/trace_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ class TraceConfig
# The global default max number of attributes per {Span}.
attr_reader :max_attributes_count

# The global default max number of {OpenTelemetry::Trace::Event}s per {Span}.
# The global default max number of {OpenTelemetry::SDK::Trace::Event}s per {Span}.
attr_reader :max_events_count

# The global default max number of {OpenTelemetry::Trace::Link} entries per {Span}.
attr_reader :max_links_count

# The global default max number of attributes per {OpenTelemetry::Trace::Event}.
# The global default max number of attributes per {OpenTelemetry::SDK::Trace::Event}.
attr_reader :max_attributes_per_event

# The global default max number of attributes per {OpenTelemetry::Trace::Link}.
Expand Down
48 changes: 48 additions & 0 deletions sdk/lib/opentelemetry/sdk/trace/event.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# frozen_string_literal: true

# Copyright 2019 OpenTelemetry Authors
#
# SPDX-License-Identifier: Apache-2.0

module OpenTelemetry
module SDK
module Trace
# A text annotation with a set of attributes and a timestamp.
class Event
EMPTY_ATTRIBUTES = {}.freeze

private_constant :EMPTY_ATTRIBUTES

# Returns the name of this event
#
# @return [String]
attr_reader :name

# Returns the frozen attributes for this event
#
# @return [Hash{String => String, Numeric, Boolean, Array<String, Numeric, Boolean>}]
attr_reader :attributes

# Returns the timestamp for this event
#
# @return [Time]
attr_reader :timestamp

# Returns a new immutable {Event}.
#
# @param [String] name The name of this event
# @param [optional Hash{String => String, Numeric, Boolean, Array<String, Numeric, Boolean>}]
# attributes A hash of attributes for this event. Attributes will be
# frozen during Event initialization.
# @param [optional Time] timestamp The timestamp for this event.
# Defaults to Time.now.
# @return [Event]
def initialize(name:, attributes: nil, timestamp: nil)
@name = name
@attributes = attributes.freeze || EMPTY_ATTRIBUTES
@timestamp = timestamp || Time.now
end
end
end
end
end
27 changes: 9 additions & 18 deletions sdk/lib/opentelemetry/sdk/trace/span.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,36 +77,27 @@ def set_attribute(key, value)
self
end

# Add an Event to a {Span}. This can be accomplished eagerly or lazily.
# Lazy evaluation is useful when the event attributes are expensive to
# build and where the cost can be avoided for an unsampled {Span}.
# Add an Event to a {Span}.
#
# Eager example:
# Example:
#
# span.add_event(name: 'event', attributes: {'eager' => true})
#
# Lazy example:
#
# span.add_event { OpenTelemetry::Trace::Event.new(name: 'event', attributes: {'eager' => false}) }
# span.add_event('event', attributes: {'eager' => true})
#
# Note that the OpenTelemetry project
# {https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md
# documents} certain "standard event names and keys" which have
# prescribed semantic meanings.
#
# @param [optional String] name Optional name of the event. This is
# required if a block is not given.
# @param [String] name Name of the event.
# @param [optional Hash{String => String, Numeric, Boolean, Array<String, Numeric, Boolean>}] attributes
# One or more key:value pairs, where the keys must be strings and the
# values may be string, boolean or numeric type. This argument should
# only be used when passing in a name.
# values may be string, boolean or numeric type.
# @param [optional Time] timestamp Optional timestamp for the event.
# This argument should only be used when passing in a name.
#
# @return [self] returns itself
def add_event(name: nil, attributes: nil, timestamp: nil)
def add_event(name, attributes: nil, timestamp: nil)
super
event = block_given? ? yield : OpenTelemetry::Trace::Event.new(name: name, attributes: attributes, timestamp: timestamp || Time.now)
event = Event.new(name: name, attributes: attributes, timestamp: timestamp || Time.now)

@mutex.synchronize do
if @ended
Expand All @@ -127,7 +118,7 @@ def add_event(name: nil, attributes: nil, timestamp: nil)
#
# @return [void]
def record_error(error)
add_event(name: 'error',
add_event('error',
attributes: {
'error.type' => error.class.to_s,
'error.message' => error.message,
Expand Down Expand Up @@ -325,7 +316,7 @@ def append_event(events, event) # rubocop:disable Metrics/AbcSize, Metrics/Cyclo
attrs.keep_if { |key, value| Internal.valid_key?(key) && Internal.valid_value?(value) }
excess = attrs.size - max_attributes_per_event
excess.times { attrs.shift } if excess.positive?
event = OpenTelemetry::Trace::Event.new(name: event.name, attributes: attrs, timestamp: event.timestamp)
event = Event.new(name: event.name, attributes: attrs, timestamp: event.timestamp)
end
events << event
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

require 'test_helper'

describe OpenTelemetry::Trace::Event do
Event = OpenTelemetry::Trace::Event
describe OpenTelemetry::SDK::Trace::Event do
Event = OpenTelemetry::SDK::Trace::Event
describe '.new' do
it 'accepts a name' do
event = Event.new(name: 'message')
Expand Down
Loading