Skip to content

Avoid leaking tracing timestamp to breadcrumbs#1575

Merged
st0012 merged 3 commits intomasterfrom
avoid-leaking-tracing-timestamp-to-breadcrumbs
Sep 17, 2021
Merged

Avoid leaking tracing timestamp to breadcrumbs#1575
st0012 merged 3 commits intomasterfrom
avoid-leaking-tracing-timestamp-to-breadcrumbs

Conversation

@st0012
Copy link
Copy Markdown
Contributor

@st0012 st0012 commented Sep 17, 2021

Currently when using sentry-rails's active_support_logger breadcrumb logger with tracing enabled, all the breadcrumbs will have an additional start_timestamp attribute. That attribute is added for helping tracing subscribers calculate each span's duration and is not designed to be leaked outside.

So this PR adds 2 changes:

  • Because the attribute is added to all instrument events and is visible to all subscribers (including other libraries or users' subscribers), its name should have sentry_ prefix to avoid misuse.
  • InstrumentPayloadCleanupHelper should remove the sentry_start_timestamp key from breadcrumbs.

@st0012 st0012 added this to the 4.8.0 milestone Sep 17, 2021
@st0012 st0012 self-assigned this Sep 17, 2021
@st0012 st0012 force-pushed the avoid-leaking-tracing-timestamp-to-breadcrumbs branch from f66b336 to b0d3420 Compare September 17, 2021 14:56
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 17, 2021 •
edited by codecov bot
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.06%. Comparing base (fddb235) to head (9c553d4).
⚠️ Report is 724 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #1575   +/-   ##
=======================================
  Coverage   98.06%   98.06%           
=======================================
  Files         126      126           
  Lines        6923     6932    +9     
=======================================
+ Hits         6789     6798    +9     
  Misses        134      134

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@st0012 st0012 merged commit a6ce224 into master Sep 17, 2021
@st0012 st0012 deleted the avoid-leaking-tracing-timestamp-to-breadcrumbs branch September 17, 2021 15:28
@st0012 st0012 modified the milestones: 4.8.0, 4.7.3 Sep 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@st0012 st0012

Labels

sentry-rails

Projects

None yet

Milestone

4.7.3

Development

Successfully merging this pull request may close these issues.

2 participants

@st0012 @codecov-commenter