Skip to content

Tracing without performance#2084

Merged
sl0thentr0py merged 13 commits intomasterfrom
neel/twp
Sep 1, 2023
Merged

Tracing without performance#2084
sl0thentr0py merged 13 commits intomasterfrom
neel/twp

Conversation

@sl0thentr0py
Copy link
Copy Markdown
Member

@sl0thentr0py sl0thentr0py commented Aug 2, 2023 •
edited
Loading

This PR is 1/2 to enable Tracing without Performance, i.e. make sure all our events are connected even if they are not Transactions.

This enables use cases such as Errors / Transactions / Replays etc all being connected across services and not just Transactions.

Summary of changes

  • new PropagationContext class that generates trace/span ids and baggage irrespective of whether there are transactions/spans active or not
  • this lives on the Scope
  • three new top level methods that first check the span and fallback on the scope's propagation context
    • Sentry.get_traceparent
    • Sentry.get_baggage
    • Sentry.get_trace_propagation_headers
  • move dynamic_sampling_context to Event from TransactionEvent since all events will now have this info
  • use the new top level helpers in net/http patch to set propagation headers

closes #2056
also see #2089

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 2, 2023 •
edited
Loading

Codecov Report

Patch coverage is 64.46% of modified lines.

Files Changed Coverage
sentry-ruby/lib/sentry/client.rb ø
sentry-ruby/lib/sentry/rack/capture_exceptions.rb 0.00%
sentry-ruby/lib/sentry/transaction_event.rb ø
sentry-ruby/lib/sentry/hub.rb 21.05%
sentry-ruby/lib/sentry/net/http.rb 28.57%
sentry-ruby/lib/sentry-ruby.rb 33.33%
sentry-ruby/lib/sentry/propagation_context.rb 77.35%
sentry-ruby/lib/sentry/scope.rb 77.77%
sentry-rails/lib/sentry/rails/action_cable.rb 100.00%
...entry-rails/lib/sentry/rails/capture_exceptions.rb 100.00%
... and 5 more

📢 Thoughts on this report? Let us know!.

@sl0thentr0py sl0thentr0py force-pushed the neel/twp branch 6 times, most recently from 9ffcc1f to 23a4d06 Compare August 16, 2023 12:17
@sl0thentr0py sl0thentr0py mentioned this pull request Aug 16, 2023
Merged
@sl0thentr0py sl0thentr0py requested a review from st0012 August 16, 2023 12:44
@sl0thentr0py
Copy link
Copy Markdown
Member Author

sl0thentr0py commented Aug 16, 2023

@st0012 still gonna write some specs and test but feel free to start reviewing if you have time and let me know if you have questions on why we are doing this!

@sl0thentr0py sl0thentr0py mentioned this pull request Aug 17, 2023
Closed
st0012
st0012 reviewed Aug 20, 2023
View reviewed changes
szokeasaurusrex
szokeasaurusrex reviewed Aug 29, 2023
View reviewed changes
@sl0thentr0py sl0thentr0py mentioned this pull request Aug 29, 2023
Open
@sl0thentr0py sl0thentr0py force-pushed the neel/twp branch 3 times, most recently from 83541fd to c6d3bfe Compare August 29, 2023 13:22
@sl0thentr0py sl0thentr0py marked this pull request as ready for review August 29, 2023 13:40
cleptric
cleptric approved these changes Aug 31, 2023
View reviewed changes
github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 31, 2023
View reviewed changes
sentry-ruby/lib/sentry/propagation_context.rb
# @param sentry_trace [String] the sentry-trace header value from the previous transaction.
# @return [Array, nil]
def self.extract_sentry_trace(sentry_trace)
match = SENTRY_TRACE_REGEXP.match(sentry_trace)

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data

This [regular expression](1) that depends on a [library input](2) may run slow on strings with many repetitions of '\t'. This [regular expression](1) that depends on a [library input](3) may run slow on strings with many repetitions of '\t'. This [regular expression](1) that depends on a [library input](4) may run slow on strings with many repetitions of '\t'.
@sl0thentr0py sl0thentr0py merged commit 9641957 into master Sep 1, 2023
@sl0thentr0py sl0thentr0py deleted the neel/twp branch September 1, 2023 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@szokeasaurusrex szokeasaurusrex szokeasaurusrex left review comments

@cleptric cleptric cleptric approved these changes

@st0012 st0012 Awaiting requested review from st0012

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Tracing without Performance

5 participants

@sl0thentr0py @st0012 @cleptric @szokeasaurusrex @github-advanced-security