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

Consolidate HTTP span data conventions with OpenTelemetry #2093

Merged
merged 1 commit into from Sep 4, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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
Expand Down
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions sentry-rails/spec/sentry/rails/tracing_spec.rb
Expand Up @@ -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")
Expand All @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions sentry-ruby/lib/sentry/net/http.rb
Expand Up @@ -38,10 +38,10 @@

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)

Check warning on line 44 in sentry-ruby/lib/sentry/net/http.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/net/http.rb#L41-L44

Added lines #L41 - L44 were not covered by tests
end
end
end
Expand Down
11 changes: 10 additions & 1 deletion sentry-ruby/lib/sentry/span.rb
Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions sentry-ruby/spec/sentry/net/http_spec.rb
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
})
Expand Down Expand Up @@ -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",
})
Expand All @@ -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",
})
Expand Down
2 changes: 1 addition & 1 deletion sentry-ruby/spec/sentry/span_spec.rb
Expand Up @@ -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
Expand Down