Skip to content

Conversation

@Lms24
Copy link
Member

@Lms24 Lms24 commented Dec 3, 2024

DESCRIBE YOUR PR

Adds a section about the importance of having SDKs aligned to the same version in MFE setups plus an optional workaround for when this is not feasible. I opted to only link to the workaround posted in GH for the moment because we're still awaiting confirmation that this works and it requires some deep SDK behaviour intervention from users. We might revisit this and provide a better abstraction for it on demand.

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

EXTRA RESOURCES

@vercel
Copy link

vercel bot commented Dec 3, 2024

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 13, 2024 11:52am
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
changelog ⬜️ Ignored (Inspect) Visit Preview Dec 13, 2024 11:52am
develop-docs ⬜️ Ignored (Inspect) Visit Preview Dec 13, 2024 11:52am

@Lms24 Lms24 self-assigned this Dec 3, 2024
@Lms24 Lms24 requested review from mydea and smeubank December 3, 2024 12:04
@codecov
Copy link

codecov bot commented Dec 3, 2024

Bundle Report

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

Detailed changes
Bundle name Size Change
sentry-docs-server-cjs 10.34MB 9 bytes (-0.0%) ⬇️
sentry-docs-client-array-push 9.27MB 6 bytes (-0.0%) ⬇️

@lizokm lizokm self-requested a review December 3, 2024 18:37
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 so much for updating this! I added another PR with additional updates addressing the user feedback I got: #12040

## SDK Version Alignment
Starting with version 8, if you have multiple Sentry JavaScript SDKs on the same page, they will [only interact with each other](https://github.com/getsentry/sentry-javascript/pull/12206) if they're all using the same version. This prevents unwanted cross-SDK interactions, for example, from browser extensions or 3rd party scripts.

Choose a reason for hiding this comment

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

Starting with version 8

Technically, it was 8.6.0, right? (Or 8.7.0... the release notes suggest that 8.6 didn't get released correctly) https://github.com/getsentry/sentry-javascript/releases/tag/8.7.0

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, we can be more specific. I changed it to 8.7.0.

## SDK Version Alignment
Starting with version 8, if you have multiple Sentry JavaScript SDKs on the same page, they will [only interact with each other](https://github.com/getsentry/sentry-javascript/pull/12206) if they're all using the same version. This prevents unwanted cross-SDK interactions, for example, from browser extensions or 3rd party scripts.

Choose a reason for hiding this comment

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

This prevents unwanted cross-SDK interactions, for example, from browser extensions or 3rd party scripts.

Is this the reason? I was under the impression it was to ensure there were no incompatibilities between the global singleton (created by one version) and the running SDK (on a different version).

The versioned carrier isn't a robust way to prevent browser extensions or third party scripts, because of course, they could coincidentally be on the same version.

Copy link
Member Author

@Lms24 Lms24 Dec 13, 2024

Choose a reason for hiding this comment

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

I reworded this section a bit and focussed on incompatibility problems. However, one of the top reasons why such incompatibility errors occured, and why we introduced the versioned carrier object, was the clash between extensions and host app Sentry SDKs.

And yes, you're right: The version key does not guarantee isolation between extensions and host apps. We also started blocking Senty.init calls in browser extension environments as we recommend extension users to follow a more isolated Sentry client setup.

Starting with version 8, if you have multiple Sentry JavaScript SDKs on the same page, they will [only interact with each other](https://github.com/getsentry/sentry-javascript/pull/12206) if they're all using the same version. This prevents unwanted cross-SDK interactions, for example, from browser extensions or 3rd party scripts.
However, for micro frontends, where you _want_ SDK interaction between multiple SDKs, you'll need to ensure that all SDKs are using the same version.

Choose a reason for hiding this comment

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

Perhaps something like:

However, the micro frontend integration depends upon multiple configurations interacting with the same Sentry instance. This means that all microfrontends and the host must use the same exact version of Sentry.

Copy link
Member Author

Choose a reason for hiding this comment

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

However, the micro frontend integration depends upon multiple configurations interacting with the same Sentry instance.

I don't think it strictly depends on same versions. Some MFE<>Sentry users just use Sentry in the host app and not in the MFEs at all. For them, the "multiple SDKs" part doesn't apply. However, I'll reword this because it's a bit misleading, I agree.

However, for micro frontends, where you _want_ SDK interaction between multiple SDKs, you'll need to ensure that all SDKs are using the same version.
Version alignment is important when you initialize the SDK in a host or skeleton application, but use Sentry SDK calls in micro frontend child applications. For example, if your individual micro frontends call `Sentry.captureException`.

Choose a reason for hiding this comment

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

Might be worth adding something like "Module federation is one way to achieve version alignment."

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm by far no module federation expert but to me it module federation is one of many ways how to architect an MFE setup. How would module federation guarantee version alignment?

Choose a reason for hiding this comment

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

The aspect of Module Federation that I was thinking about was the dependency sharing. Perhaps it could be said more generically.

But the idea is: if the @sentry/browser dependency is shared (amongst the host and MFEs), then it will necessarily be the same version. (Of course, if the MFE doesn't make use of that dependency and chooses to use its own version, that's another story).

@Lms24 Lms24 requested a review from a team as a code owner December 13, 2024 11:25
@Lms24
Copy link
Member Author

Lms24 commented Dec 13, 2024

I'm gonna merge this in because we actually need to re-deploy our docs to fix another issue. This is the simplest way. @davemyersworld we can follow up on potential suggestions in a new PR.

@Lms24 Lms24 merged commit b4f043d into master Dec 13, 2024
11 checks passed
@Lms24 Lms24 deleted the lms/ref-mfe-version-alignment branch December 13, 2024 14:26
@github-actions github-actions bot locked and limited conversation to collaborators Dec 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants