Skip to content

feat(onboarding): Add profiling to platform node - #55530

Merged
ArthurKnaus merged 5 commits into
masterfrom
aknaus/feat/onboarding/add-profiling-platform-node
Aug 31, 2023
Merged

feat(onboarding): Add profiling to platform node#55530
ArthurKnaus merged 5 commits into
masterfrom
aknaus/feat/onboarding/add-profiling-platform-node

Conversation

@ArthurKnaus

Copy link
Copy Markdown
Member

Add profiling product selection to platform node.

Closes #55441

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Aug 31, 2023

@ArthurKnaus ArthurKnaus Aug 31, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I extracted the common logic for the node platforms into this file as it would be a lot of logic duplication when I continue to add the product selector to the others.

@ArthurKnaus
ArthurKnaus requested a review from a team August 31, 2023 09:28
@ArthurKnaus
ArthurKnaus marked this pull request as ready for review August 31, 2023 09:29
@ArthurKnaus
ArthurKnaus requested a review from a team August 31, 2023 09:29
Comment on lines +96 to +101
params.push(
`// Set sampling rate for profiling - this is relative to tracesSampleRate`
);
params.push(
`profilesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production!`
);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we use single-line inserts instead of backticks with multiline content?
Is it better for indentation formatting?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, this way we leave the indentation up to the single platform files.
Imho, indentation should not be a concern of those utils as they are not aware of the full code snippet context.

@ArthurKnaus

Copy link
Copy Markdown
Member Author

@Lms24 since we talked about the content:
Screenshot 2023-08-31 at 11 39 15

@ArthurKnaus
ArthurKnaus requested a review from Lms24 August 31, 2023 09:39

@Lms24 Lms24 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for applying my feedback around CJS/ESM. Love the dynamic getting started docs!
Had one minor comment. :shipit:

Comment thread static/app/utils/gettingStartedDocs/node.ts Outdated
ArthurKnaus and others added 2 commits August 31, 2023 11:51
Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
@ArthurKnaus
ArthurKnaus merged commit 4d864b2 into master Aug 31, 2023
@ArthurKnaus
ArthurKnaus deleted the aknaus/feat/onboarding/add-profiling-platform-node branch August 31, 2023 10:51
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add profiling to node

3 participants