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

fix! implement new Span.Status spec #411

Merged
merged 8 commits into from
Sep 22, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
72 changes: 7 additions & 65 deletions api/lib/opentelemetry/trace/status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class Status

# Initialize a Status.
#
# @param [Integer] canonical_code One of the standard gRPC codes: https://github.com/grpc/grpc/blob/master/doc/statuscodes.md
# @param [Integer] canonical_code One of the canonical status codes below
# @param [String] description
def initialize(canonical_code, description: '')
@canonical_code = canonical_code
Expand All @@ -37,78 +37,20 @@ def initialize(canonical_code, description: '')
#
# @return [Boolean]
def ok?
@canonical_code == OK
@canonical_code != ERROR
end

# The following represents the canonical set of status codes of a
# finished {Span}, following the standard gRPC codes:
# https://github.com/grpc/grpc/blob/master/doc/statuscodes.md
# finished {Span}

# The operation completed successfully.
OK = 0

# The operation was cancelled (typically by the caller).
CANCELLED = 1
# The default status.
UNSET = 1

# An unknown error.
UNKNOWN_ERROR = 2

# Client specified an invalid argument. Note that this differs from
# {FAILED_PRECONDITION}. {INVALID_ARGUMENT} indicates arguments that are
# problematic regardless of the state of the system.
INVALID_ARGUMENT = 3

# Deadline expired before operation could complete. For operations that
# change the state of the system, this error may be returned even if the
# operation has completed successfully.
DEADLINE_EXCEEDED = 4

# Some requested entity (e.g., file or directory) was not found.
NOT_FOUND = 5

# Some entity that we attempted to create (e.g., file or directory)
# already exists.
ALREADY_EXISTS = 6

# The caller does not have permission to execute the specified operation.
# {PERMISSION_DENIED} must not be used if the caller cannot be identified
# (use {UNAUTHENTICATED} instead for those errors).
PERMISSION_DENIED = 7

# Some resource has been exhausted, perhaps a per-user quota, or perhaps
# the entire file system is out of space.
RESOURCE_EXHAUSTED = 8

# Operation was rejected because the system is not in a state required
# for the operation's execution.
FAILED_PRECONDITION = 9

# The operation was aborted, typically due to a concurrency issue like
# sequencer check failures, transaction aborts, etc.
ABORTED = 10

# Operation was attempted past the valid range. E.g., seeking or reading
# past end of file. Unlike {INVALID_ARGUMENT}, this error indicates a
# problem that may be fixed if the system state changes.
OUT_OF_RANGE = 11

# Operation is not implemented or not supported/enabled in this service.
UNIMPLEMENTED = 12

# Internal errors. Means some invariants expected by underlying system
# has been broken.
INTERNAL_ERROR = 13

# The service is currently unavailable. This is a most likely a transient
# condition and may be corrected by retrying with a backoff.
UNAVAILABLE = 14

# Unrecoverable data loss or corruption.
DATA_LOSS = 15

# The request does not have valid authentication credentials for the
# operation.
UNAUTHENTICATED = 16
# An error.
ERROR = 2
end
end
end
2 changes: 1 addition & 1 deletion api/lib/opentelemetry/trace/tracer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def in_span(name, attributes: nil, links: nil, start_timestamp: nil, kind: nil,
with_span(span) { |s, c| yield s, c }
rescue Exception => e # rubocop:disable Lint/RescueException
span&.record_exception(e)
span&.status = Status.new(Status::UNKNOWN_ERROR,
span&.status = Status.new(Status::ERROR,
description: "Unhandled exception of type: #{e.class}")
raise e
ensure
Expand Down
27 changes: 4 additions & 23 deletions api/lib/opentelemetry/trace/util/http_to_status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,36 +9,17 @@ module Trace
module Util
# Convenience methods, not necessarily required by the API specification.
module HttpToStatus
# Implemented according to
# https://github.com/open-telemetry/opentelemetry-specification/issues/306
# https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-http.md#status
# Maps numeric HTTP status codes to Trace::Status. This module is a mixin for Trace::Status
# and is not intended for standalone use.
#
# @param code Numeric HTTP status
# @return Status
def http_to_status(code) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength
def http_to_status(code)
case code.to_i
when 100..399
new(const_get(:OK))
when 401
new(const_get(:UNAUTHENTICATED))
when 403
new(const_get(:PERMISSION_DENIED))
when 404
new(const_get(:NOT_FOUND))
when 429
new(const_get(:RESOURCE_EXHAUSTED))
when 400..499
new(const_get(:INVALID_ARGUMENT))
when 501
new(const_get(:UNIMPLEMENTED))
when 503
new(const_get(:UNAVAILABLE))
when 504
new(const_get(:DEADLINE_EXCEEDED))
when 500..599
new(const_get(:INTERNAL_ERROR))
else
new(const_get(:UNKNOWN_ERROR))
new(const_get(:ERROR))
end
end
end
Expand Down
19 changes: 5 additions & 14 deletions api/test/opentelemetry/trace/status_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,13 @@ def assert_http_to_status(http_code, trace_status_code)
end

it 'maps http 4xx codes' do
assert_http_to_status(400, trace_status::INVALID_ARGUMENT)
assert_http_to_status(401, trace_status::UNAUTHENTICATED)
assert_http_to_status(403, trace_status::PERMISSION_DENIED)
assert_http_to_status(404, trace_status::NOT_FOUND)
assert_http_to_status(409, trace_status::INVALID_ARGUMENT)
assert_http_to_status(429, trace_status::RESOURCE_EXHAUSTED)
assert_http_to_status(499, trace_status::INVALID_ARGUMENT)
assert_http_to_status(400, trace_status::ERROR)
assert_http_to_status(499, trace_status::ERROR)
end

it 'maps http 5xx codes' do
assert_http_to_status(500, trace_status::INTERNAL_ERROR)
assert_http_to_status(501, trace_status::UNIMPLEMENTED)
assert_http_to_status(502, trace_status::INTERNAL_ERROR)
assert_http_to_status(503, trace_status::UNAVAILABLE)
assert_http_to_status(504, trace_status::DEADLINE_EXCEEDED)
assert_http_to_status(599, trace_status::INTERNAL_ERROR)
assert_http_to_status(500, trace_status::ERROR)
assert_http_to_status(599, trace_status::ERROR)
end
end

Expand Down Expand Up @@ -88,7 +79,7 @@ def assert_http_to_status(http_code, trace_status_code)
end

it 'reflects canonical_code when not OK' do
canonical_codes = OpenTelemetry::Trace::Status.constants - %i[OK]
canonical_codes = OpenTelemetry::Trace::Status.constants - %i[OK UNSET]
canonical_codes.each do |canonical_code|
code = OpenTelemetry::Trace::Status.const_get(canonical_code)
status = OpenTelemetry::Trace::Status.new(code)
Expand Down
3 changes: 2 additions & 1 deletion exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,9 @@ def as_otlp_span(span_data) # rubocop:disable Metrics/AbcSize, Metrics/MethodLen
end,
dropped_links_count: span_data.total_recorded_links - span_data.links&.size.to_i,
status: span_data.status&.yield_self do |status|
# TODO: fix this based on spec update.
Opentelemetry::Proto::Trace::V1::Status.new(
code: status.canonical_code,
code: status.canonical_code == OpenTelemetry::Trace::Status::ERROR ? Opentelemetry::Proto::Trace::V1::Status::StatusCode::UnknownError : Opentelemetry::Proto::Trace::V1::Status::StatusCode::Ok,
message: status.description
)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
span['i'] = 2
span['s'] = 'val'
span['a'] = [3, 4]
span.status = OpenTelemetry::Trace::Status.new(OpenTelemetry::Trace::Status::UNKNOWN_ERROR)
span.status = OpenTelemetry::Trace::Status.new(OpenTelemetry::Trace::Status::ERROR)
client = with_ids(trace_id, client_span_id) { tracer.start_span('client', with_parent: span, kind: :client, start_timestamp: start_timestamp + 2).finish(end_timestamp: end_timestamp) }
with_ids(trace_id, server_span_id) { other_tracer.start_span('server', with_parent: client, kind: :server, start_timestamp: start_timestamp + 3).finish(end_timestamp: end_timestamp) }
span.add_event('event', attributes: { 'attr' => 42 }, timestamp: start_timestamp + 4)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def complete # rubocop:disable Metrics/MethodLength
return_code = response_options[:return_code]
message = return_code ? ::Ethon::Curl.easy_strerror(return_code) : 'unknown reason'
@otel_span.status = OpenTelemetry::Trace::Status.new(
OpenTelemetry::Trace::Status::UNKNOWN_ERROR,
OpenTelemetry::Trace::Status::ERROR,
description: "Request has failed: #{message}"
)
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def stub_response(options)
_(span.attributes['http.status_code']).must_be_nil
_(span.attributes['http.url']).must_equal 'http://example.com/test'
_(span.status.canonical_code).must_equal(
OpenTelemetry::Trace::Status::UNKNOWN_ERROR
OpenTelemetry::Trace::Status::ERROR
)
_(span.status.description).must_equal(
'Request has failed: Timeout was reached'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def handle_response(datum) # rubocop:disable Metrics/AbcSize, Metrics/MethodLeng

if datum.key?(:error)
span.status = OpenTelemetry::Trace::Status.new(
OpenTelemetry::Trace::Status::UNKNOWN_ERROR,
OpenTelemetry::Trace::Status::ERROR,
description: "Request has failed: #{datum[:error]}"
)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
_(span.attributes['http.host']).must_equal 'example.com'
_(span.attributes['http.target']).must_equal '/timeout'
_(span.status.canonical_code).must_equal(
OpenTelemetry::Trace::Status::UNKNOWN_ERROR
OpenTelemetry::Trace::Status::ERROR
)
_(span.status.description).must_equal(
'Request has failed: Excon::Error::Timeout'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
_(span.attributes['net.peer.port']).must_equal port.to_s

_(span.status.canonical_code).must_equal(
OpenTelemetry::Trace::Status::UNKNOWN_ERROR
OpenTelemetry::Trace::Status::ERROR
)
_(span.events.first.name).must_equal 'exception'
_(span.events.first.attributes['exception.type']).must_equal 'Mysql2::Error'
Expand Down Expand Up @@ -120,7 +120,7 @@
_(span.attributes['net.peer.port']).must_equal port.to_s

_(span.status.canonical_code).must_equal(
OpenTelemetry::Trace::Status::UNKNOWN_ERROR
OpenTelemetry::Trace::Status::ERROR
)
_(span.events.first.name).must_equal 'exception'
_(span.events.first.attributes['exception.type']).must_equal 'Mysql2::Error'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
_(span.attributes['peer.hostname']).must_equal 'example.com'
_(span.attributes['peer.port']).must_equal 443
_(span.status.canonical_code).must_equal(
OpenTelemetry::Trace::Status::UNKNOWN_ERROR
OpenTelemetry::Trace::Status::ERROR
)
_(span.status.description).must_equal(
'Unhandled exception of type: Net::OpenTimeout'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@
assert_raises SimulatedError do
Rack::MockRequest.new(rack_builder).get('/', env)
end
_(first_span.status.canonical_code).must_equal OpenTelemetry::Trace::Status::UNKNOWN_ERROR
_(first_span.status.canonical_code).must_equal OpenTelemetry::Trace::Status::ERROR
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
_(span.attributes['net.peer.name']).must_equal '127.0.0.1'
_(span.attributes['net.peer.port']).must_equal 6379
_(span.status.canonical_code).must_equal(
OpenTelemetry::Trace::Status::UNKNOWN_ERROR
OpenTelemetry::Trace::Status::ERROR
)
_(span.status.description).must_equal(
'Unhandled exception of type: Redis::CommandError'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@
it 'does not create unhandled exceptions for missing routes' do
get '/one/missing_example/not_present'

_(exporter.finished_spans.first.status.canonical_code).must_equal 5
_(exporter.finished_spans.first.status.canonical_code).must_equal OpenTelemetry::Trace::Status::ERROR
_(exporter.finished_spans.first.attributes).must_equal(
'http.method' => 'GET',
'http.url' => '/missing_example/not_present',
Expand Down
2 changes: 1 addition & 1 deletion sdk/test/opentelemetry/sdk/trace/tracer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@

_(span.events[0].name).must_equal('exception')
_(span.events[0].attributes['exception.message']).must_equal('this is fine')
_(span.status.canonical_code).must_equal(OpenTelemetry::Trace::Status::UNKNOWN_ERROR)
_(span.status.canonical_code).must_equal(OpenTelemetry::Trace::Status::ERROR)
_(span.status.description).must_equal('Unhandled exception of type: RuntimeError')
end
end
Expand Down