Skip to content

feat(spans): Produce items from process-segments #91714

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 16 commits into from
May 21, 2025

Conversation

untitaker
Copy link
Member

@untitaker untitaker commented May 15, 2025

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 15, 2025
Copy link

codecov bot commented May 15, 2025

Codecov Report

Attention: Patch coverage is 92.39130% with 7 lines in your changes missing coverage. Please review.

⚠️ Parser warning

The parser emitted a warning. Please review your JUnit XML file:

Warning while parsing testcase attributes: Limit of string is 1000 chars, for name, we got 2083 at 1:157340 in /home/runner/work/sentry/sentry/.artifacts/pytest.junit.xml
Files with missing lines Patch % Lines
...sentry/spans/consumers/process_segments/convert.py 86.27% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #91714      +/-   ##
==========================================
+ Coverage   83.79%   87.68%   +3.89%     
==========================================
  Files       10361    10343      -18     
  Lines      587529   586778     -751     
  Branches    22595    22469     -126     
==========================================
+ Hits       492298   514523   +22225     
+ Misses      94810    71816   -22994     
- Partials      421      439      +18     

* master: (58 commits)
  link: cleanup link (#91687)
  ref: create project_id index for organizationonboardingtask (#91918)
  storybook: smaller last edited (#91875)
  issues: fix chonk stacktrace alignment (#91891)
  alert: drop custom alert (#91892)
  insights: fix bar height (#91895)
  ref(span-buffer): Move max-memory-percentage to right CLI (#91924)
  ref(js): Factor button functionality (#91763)
  tests(resolve_groups): Clean up the tests (#91779)
  ref(span-buffer): Add backpressure (#91707)
  fix(nextjs-insights): project id is not passed to explore link (#91920)
  fix(crons): Floor seconds / microsecond on recorded dateClock (#91890)
  fix(uptime): Fix bug with the uptime_checks dataset in the events endpoint (#91824)
  ref: add state-only migration to reflect existing indexes in prod (#91901)
  ref: remove unnecssary metaclass (#91906)
  fix(stats): use data category title name (#91913)
  feat(issues): Add success messages to some actions (#91899)
  test(taskworker): Lower exec time (#91907)
  chore(aci): manually add spans for delayed workflow processing (#91908)
  chore(aci): remove uses of WorkflowFireHistory rollout columns (#91904)
  ...
@jan-auer jan-auer marked this pull request as ready for review May 21, 2025 12:21
@jan-auer jan-auer requested a review from a team as a code owner May 21, 2025 12:21
Copy link
Member Author

@untitaker untitaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved

def _timestamp(value: float) -> Timestamp:
return Timestamp(
seconds=int(value),
nanos=round((value % 1) * 1_000_000) * 1000,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We map timestamps with full available precision. EAP will truncate those, but we do not do this here to reduce coupling.

"description": "sentry.raw_description",
"duration_ms": "sentry.duration_ms",
"is_segment": "sentry.is_segment",
"exclusive_time_ms": "sentry.exclusive_time_ms",
Copy link
Member

@jan-auer jan-auer May 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exclusive_time_ms was not part of the Relay implementation, but eap_items_span converted this field. @phacops do we need this or should we actually map exclusive_time?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the product adapted to reading sentry.exclusive_time_ms (

"span.self_time": "exclusive_time_ms",
) so I'd leave it like this.

"received": "sentry.received",
"origin": "sentry.origin",
"kind": "sentry.kind",
"hash": "sentry.hash",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field is written in enrichment. We use this for performance issue detection and used to persist it.

@jan-auer jan-auer changed the title wip: Produce items from process-segments feat(spans): Produce items from process-segments May 21, 2025
@jan-auer jan-auer merged commit eccffc2 into master May 21, 2025
60 checks passed
@jan-auer jan-auer deleted the ref/process-segments-eap-items branch May 21, 2025 14:49
Copy link

sentry-io bot commented May 26, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ ChildProcessTerminated: 17 sentry.spans.consumers.process_segments.factory... View Issue
  • ‼️ DecodeError: Error parsing message sentry.spans.consumers.process_segments.convert... View Issue

Did you find this useful? React with a 👍 or 👎

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants