From fd17ccfbdaa60bd429e81dbee73b449a9f86bb3e Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 17 Aug 2023 15:17:53 +0200 Subject: [PATCH] Consolidate Sentry::Span::DataConventions with OpenTelemetry --- CHANGELOG.md | 1 + .../tracing/action_controller_subscriber_spec.rb | 2 +- sentry-rails/spec/sentry/rails/tracing_spec.rb | 4 ++-- sentry-ruby/lib/sentry/net/http.rb | 8 ++++---- sentry-ruby/lib/sentry/span.rb | 11 ++++++++++- sentry-ruby/spec/sentry/net/http_spec.rb | 8 ++++---- sentry-ruby/spec/sentry/span_spec.rb | 2 +- 7 files changed, 23 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3e4cce68..d9c37d7e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Make `:value` in `SingleExceptionInterface` writable, so that it can be modified in `before_send` under `event.exception.values[n].value` [#2072](https://github.com/getsentry/sentry-ruby/pull/2072) - Add `sampled` field to `dynamic_sampling_context` [#2092](https://github.com/getsentry/sentry-ruby/pull/2092) +- Consolidate HTTP span data conventions with OpenTelemetry with `Sentry::Span::DataConventions` [#2093](https://github.com/getsentry/sentry-ruby/pull/2093) - Add new `config.trace_propagation_targets` option to set targets for which headers are propagated in outgoing HTTP requests [#2079](https://github.com/getsentry/sentry-ruby/pull/2079) ```rb diff --git a/sentry-rails/spec/sentry/rails/tracing/action_controller_subscriber_spec.rb b/sentry-rails/spec/sentry/rails/tracing/action_controller_subscriber_spec.rb index 2a099ba6c..58852df52 100644 --- a/sentry-rails/spec/sentry/rails/tracing/action_controller_subscriber_spec.rb +++ b/sentry-rails/spec/sentry/rails/tracing/action_controller_subscriber_spec.rb @@ -38,7 +38,7 @@ expect(span[:op]).to eq("view.process_action.action_controller") expect(span[:description]).to eq("HelloController#world") expect(span[:trace_id]).to eq(transaction.dig(:contexts, :trace, :trace_id)) - expect(span[:data].keys).to match_array(["status_code", :format, :method, :path, :params]) + expect(span[:data].keys).to match_array(["http.response.status_code", :format, :method, :path, :params]) end end diff --git a/sentry-rails/spec/sentry/rails/tracing_spec.rb b/sentry-rails/spec/sentry/rails/tracing_spec.rb index d362fcf54..3085b688e 100644 --- a/sentry-rails/spec/sentry/rails/tracing_spec.rb +++ b/sentry-rails/spec/sentry/rails/tracing_spec.rb @@ -40,7 +40,7 @@ expect(first_span[:description]).to eq("PostsController#index") expect(first_span[:parent_span_id]).to eq(parent_span_id) expect(first_span[:status]).to eq("internal_error") - expect(first_span[:data].keys).to match_array(["status_code", :format, :method, :path, :params]) + expect(first_span[:data].keys).to match_array(["http.response.status_code", :format, :method, :path, :params]) second_span = transaction[:spans][1] expect(second_span[:op]).to eq("db.sql.active_record") @@ -67,7 +67,7 @@ expect(transaction[:spans].count).to eq(3) first_span = transaction[:spans][0] - expect(first_span[:data].keys).to match_array(["status_code", :format, :method, :path, :params]) + expect(first_span[:data].keys).to match_array(["http.response.status_code", :format, :method, :path, :params]) expect(first_span[:op]).to eq("view.process_action.action_controller") expect(first_span[:description]).to eq("PostsController#show") expect(first_span[:parent_span_id]).to eq(parent_span_id) diff --git a/sentry-ruby/lib/sentry/net/http.rb b/sentry-ruby/lib/sentry/net/http.rb index 989d8f393..1ffb18ffb 100644 --- a/sentry-ruby/lib/sentry/net/http.rb +++ b/sentry-ruby/lib/sentry/net/http.rb @@ -38,10 +38,10 @@ def request(req, body = nil, &block) if sentry_span sentry_span.set_description("#{request_info[:method]} #{request_info[:url]}") - sentry_span.set_data('url', request_info[:url]) - sentry_span.set_data('http.method', request_info[:method]) - sentry_span.set_data('http.query', request_info[:query]) if request_info[:query] - sentry_span.set_data('status', res.code.to_i) + sentry_span.set_data(Span::DataConventions::URL, request_info[:url]) + sentry_span.set_data(Span::DataConventions::HTTP_METHOD, request_info[:method]) + sentry_span.set_data(Span::DataConventions::HTTP_QUERY, request_info[:query]) if request_info[:query] + sentry_span.set_data(Span::DataConventions::HTTP_STATUS_CODE, res.code.to_i) end end end diff --git a/sentry-ruby/lib/sentry/span.rb b/sentry-ruby/lib/sentry/span.rb index 8fda79fd8..bc9b28f75 100644 --- a/sentry-ruby/lib/sentry/span.rb +++ b/sentry-ruby/lib/sentry/span.rb @@ -4,6 +4,15 @@ module Sentry class Span + + # We will try to be consistent with OpenTelemetry on this front going forward. + module DataConventions + URL = "url" + HTTP_STATUS_CODE = "http.response.status_code" + HTTP_QUERY = "http.query" + HTTP_METHOD = "http.method" + end + STATUS_MAP = { 400 => "invalid_argument", 401 => "unauthenticated", @@ -208,7 +217,7 @@ def set_timestamp(timestamp) # @param status_code [String] example: "500". def set_http_status(status_code) status_code = status_code.to_i - set_data("status_code", status_code) + set_data(DataConventions::HTTP_STATUS_CODE, status_code) status = if status_code >= 200 && status_code < 299 diff --git a/sentry-ruby/spec/sentry/net/http_spec.rb b/sentry-ruby/spec/sentry/net/http_spec.rb index eaa79e955..2e7a424d8 100644 --- a/sentry-ruby/spec/sentry/net/http_spec.rb +++ b/sentry-ruby/spec/sentry/net/http_spec.rb @@ -43,7 +43,7 @@ expect(request_span.start_timestamp).not_to eq(request_span.timestamp) expect(request_span.description).to eq("GET http://example.com/path") expect(request_span.data).to eq({ - "status" => 200, + "http.response.status_code" => 200, "url" => "http://example.com/path", "http.method" => "GET", "http.query" => "foo=bar" @@ -74,7 +74,7 @@ expect(request_span.start_timestamp).not_to eq(request_span.timestamp) expect(request_span.description).to eq("GET http://example.com/path") expect(request_span.data).to eq({ - "status" => 200, + "http.response.status_code" => 200, "url" => "http://example.com/path", "http.method" => "GET", }) @@ -318,7 +318,7 @@ def verify_spans(transaction) expect(request_span.start_timestamp).not_to eq(request_span.timestamp) expect(request_span.description).to eq("GET http://example.com/path") expect(request_span.data).to eq({ - "status" => 200, + "http.response.status_code" => 200, "url" => "http://example.com/path", "http.method" => "GET", }) @@ -330,7 +330,7 @@ def verify_spans(transaction) expect(request_span.start_timestamp).not_to eq(request_span.timestamp) expect(request_span.description).to eq("GET http://example.com/path") expect(request_span.data).to eq({ - "status" => 404, + "http.response.status_code" => 404, "url" => "http://example.com/path", "http.method" => "GET", }) diff --git a/sentry-ruby/spec/sentry/span_spec.rb b/sentry-ruby/spec/sentry/span_spec.rb index 5518f23c5..712efea87 100644 --- a/sentry-ruby/spec/sentry/span_spec.rb +++ b/sentry-ruby/spec/sentry/span_spec.rb @@ -265,7 +265,7 @@ it "adds status_code (#{status_code}) to data and sets correct status (#{status})" do subject.set_http_status(status_code) - expect(subject.data["status_code"]).to eq(status_code) + expect(subject.data["http.response.status_code"]).to eq(status_code) expect(subject.status).to eq(status) end end