Skip to content

chore: Remove DSN from samples#4268

Merged
jamescrosswell merged 4 commits intomainfrom
remove-dsn-from-samples
Jun 10, 2025
Merged

chore: Remove DSN from samples#4268
jamescrosswell merged 4 commits intomainfrom
remove-dsn-from-samples

Conversation

@jamescrosswell
Copy link
Copy Markdown
Collaborator

@jamescrosswell jamescrosswell commented Jun 9, 2025

Replaces #4238

Summary

To avoid SDK users accidentally using the DSN used internally to maintain the Sentry SDK, the DSN has been removed from all of the sample projects. SDK maintainers can either set this as an environment variable or, as with other SDK users, set it manually in the samples/SamplesShared.cs file (although changes to that file should never be checked in).

Problem

In the past, I would check out a branch, manually copy/paste our DSN into one of the sample projects, work on the code and then try to remember not to check in that file/change... unless the changes actually concerned the sample projects themselves, in which case I'd have to remove the DSN before making the commit. For a single commit, that's not too much effort. However when you're switching branches 10 times a day and doing that every day of every week, it starts to get pretty repetitive and error prone. Over time then, we just ended up leaving the DSNs in the sample projects... and a week or so ago one of our SDK users must have accedentally left our sample DSN in one of their apps which is now in staging or production or something.

Approach

The idea is that two main groups of people might be forking this repo:

  1. SDK users
  2. Maintainers and contributors

SDK Users

For SDK users, we want to make it as easy as possible for them to run any of our sample apps. One of the most common scenarios would be for someone unfamiliar with Sentry to try to run one of the samples without making any changes... which won't work because, at the very least, they need to specify a DSN.

We could wait until the application runs and then log an error... and that's what would happen if you created a new .NET application, added the Sentry package and tried to initialise Sentry without any DSN.

In the Samples for this repo though we can catch the problem at compile time, rather than waiting until runtime... for this, there's a new /samples/SamplesShared.cs file that, by default, results in a compilation error... letting any potential SDK users or new contributors know that they need to provide a DSN before running the sample.

Maintainers and contributors

For people who are contributing to the SDK regularly, I figured we could just set the DSN in an environment variable and have all of the samples automatically read it from that... if a DSN isn't specified explicitly via the options (or if it is but it's an empty DSN) this would be the default behaviour anyway.

About the only slight glitch with that approach is that Android, iOS and MacCatalyst apps won't pick up environment variables... and AssemblyAttributes also don't work for these applications since Assembly.GetEntryAssembly() returns null for these apps.

As a workaround, this PR adds a GenerateSharedDsnConstant build target that writes the SENTRY_DSN environment variable out as a constant to a generated file that gets included in all of the sample apps... and this DSN can then be referenced directly from any of the mobile samples.

Notes re tests

Some of the tests never actually worked if the SENTRY_DSN environment variable was set. Those tests are now skipped when this variable is set (which should/will never be the case on CI).

Also the code that would result in an error compiling the samples when the DSN is neither specified as an environment variable nor in code is conditionally compiled only when we are not building the solution on CI... on CI the samples will compile fine with no DSN configured. They would bomb if you actually tried to run them, but that should never be an issue.

#skip-changelog

jamescrosswell and others added 2 commits June 9, 2025 20:43
Replaces #4238

To avoid SDK users accidentally using the DSN used internally to maintain the Sentry SDK, the DSN has been removed from all of the sample projects. SDK maintainers can either set this as an environment variable or, as with other SDK users, set it manually in the `samples/SamplesShared.cs` file (although changes to that file should never be checked in).

#skip-changelog
#if !SENTRY_DSN_DEFINED_IN_ENV
// You must specify a DSN. On mobile platforms, this should be done in code here.
// See https://docs.sentry.io/product/sentry-basics/dsn-explainer/
options.Dsn = SamplesShared.Dsn;
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.

This will just be overwritten by the one below. I don't follow, why do we do this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not overwritten no... this code gets compiled only #if !SENTRY_DSN_DEFINED_IN_ENV - so when the SENTRY_DSN environment variable isn't set. This will typically be the case for SDK users who have forked the repo.

If the environment variable is set then we write it into EnvironmentVariables.Dsn where it's available to mobile samples (like this one)... which is where we read it from below. This is probably what will happen when SDK maintainers are running the samples.

Comment thread samples/Sentry.Samples.Google.Cloud.Functions/Function.cs Outdated
jamescrosswell and others added 2 commits June 10, 2025 09:52
@jamescrosswell jamescrosswell marked this pull request as ready for review June 10, 2025 08:45
@jamescrosswell jamescrosswell merged commit 5f6e7b4 into main Jun 10, 2025
36 checks passed
@jamescrosswell jamescrosswell deleted the remove-dsn-from-samples branch June 10, 2025 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants