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.

Sign up for GitHub

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

Jump to bottom

ref(js): Rewrite outdated "Transaction Name" documentation #12091

Merged
merged 8 commits into from
Dec 19, 2024
Merged

ref(js): Rewrite outdated "Transaction Name" documentation #12091

merged 8 commits into from
Dec 19, 2024

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Dec 11, 2024 •
edited
Loading

DESCRIBE YOUR PR

⚠️ to be merged after #11824 (which this PR is based on)

This PR rewrites the "Enriching Events"->"Transaction Name" page which is was fairly outdated and misleading for JavaScript.

Most importantly, it now mentions that scope.setTransactionName only applies to error events and has nothing to do with spans and root span names. The page now disambiguates the two concepts and lists ways how to update both names individually.

As an experiment, I added a "Further Information" section at the bottom of the page which tries to explain the historic context of why these two names are convoluted and appear ambiguous. I'm happy to remove this but this question and the confusion around the two names is coming up repeatedly on GitHub, in Discord as well as internally. While reviewing, feel free to let me know your thoughts if we should keep the section or reword it.

I decided to rewrite this page while triaging an older issue in the SDK repo where the two names were mixed up multiple times.

closes getsentry/sentry-javascript#13391

IS YOUR CHANGE URGENT?

Help us prioritize incoming PRs by letting us know when the change needs to go live.

  • Urgent deadline (GA date, etc.):
  • Other deadline:
  • None: Not urgent, can wait up to 1 week+

SLA

  • Teamwork makes the dream work, so please add a reviewer to your PRs.
  • Please give the docs team up to 1 week to review your PR unless you've added an urgent due date to it.
    Thanks in advance for your help!

PRE-MERGE CHECKLIST

Make sure you've checked the following before merging your changes:

  • Checked Vercel preview for correctness, including links
  • PR was reviewed and approved by any necessary SMEs (subject matter experts)
  • PR was reviewed and approved by a member of the Sentry docs team

LEGAL BOILERPLATE

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

EXTRA RESOURCES

@vercel Vercel
Copy link

vercel bot commented Dec 11, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sentry-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 19, 2024 9:11am
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
changelog ⬜️ Ignored (Inspect) Visit Preview Dec 19, 2024 9:11am
develop-docs ⬜️ Ignored (Inspect) Visit Preview Dec 19, 2024 9:11am

@Lms24 Lms24 changed the base branch from master to lms/feat-updateSpanName December 11, 2024 10:30
@Lms24 Lms24 requested a review from a team as a code owner December 11, 2024 10:30
@Lms24 Lms24 force-pushed the lms/feat-updateSpanName branch from 99ec67e to e5d3f66 Compare December 11, 2024 10:31
@Lms24 Lms24 requested review from mydea and s1gr1d December 11, 2024 10:36
@codecov Codecov
Copy link

codecov bot commented Dec 11, 2024 •
edited
Loading

Bundle Report

Changes will decrease total bundle size by 270 bytes (-0.0%) ⬇️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
sentry-docs-server-cjs 10.36MB 264 bytes (-0.0%) ⬇️
sentry-docs-client-array-push 9.28MB 6 bytes (-0.0%) ⬇️

@Lms24 Lms24 mentioned this pull request Dec 11, 2024
Closed
3 tasks
@Lms24 Lms24 self-assigned this Dec 11, 2024
@Lms24 Lms24 mentioned this pull request Dec 11, 2024
Closed
lizokm
lizokm approved these changes Dec 12, 2024
View reviewed changes
Copy link
Contributor

@lizokm lizokm left a comment

Choose a reason for hiding this comment

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

Thanks for adding this! Left a few comments.

s1gr1d
s1gr1d approved these changes Dec 13, 2024
View reviewed changes
Copy link
Member

@s1gr1d s1gr1d left a comment

Choose a reason for hiding this comment

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

Very good write-up 🙌

Base automatically changed from lms/feat-updateSpanName to master December 19, 2024 08:43
@Lms24 Lms24 force-pushed the lms/ref-js-rewrite-transaction-name-page branch from e046798 to fc3fe31 Compare December 19, 2024 08:56
@Lms24 Lms24 enabled auto-merge (squash) December 19, 2024 08:57
@Lms24 Lms24 merged commit 99016ee into master Dec 19, 2024
11 checks passed
@Lms24 Lms24 deleted the lms/ref-js-rewrite-transaction-name-page branch December 19, 2024 09:11
@github-actions github-actions bot locked and limited conversation to collaborators Jan 4, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lizokm lizokm lizokm approved these changes

@s1gr1d s1gr1d s1gr1d approved these changes

@mydea mydea Awaiting requested review from mydea

Assignees

@Lms24 Lms24

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Child span/transaction name replaced by Lambda function name
3 participants
@Lms24 @lizokm @s1gr1d