chore: clearly state system.url-prefix value should be set

[aldy505]

There are more than one cases of people having their DSN or email links empty, most of it because system.url-prefix is not set on the config. Hope this will solves that (by providing a clearer message). But, I guess we should add this one to the docs site too, regarding "which config file you should be modifying before you run your sentry".

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.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.02%. Comparing base (036f6d4) to head (57ab77e). Report is 9 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3100      +/-   ##
==========================================
- Coverage   99.01%   98.02%   -0.99%     
==========================================
  Files           3        3              
  Lines         203      203              
==========================================
- Hits          201      199       -2     
- Misses          2        4       +2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[BYK]

This should be handled by the initial set up screen, that's why we didn't ask it to be set in the config.

Agree with docs update but disagree with this specific change. If there's an issue with the set-up flow we should address that instead (on get sentry/sentry side)

[aldy505]

This should be handled by the initial set up screen, that's why we didn't ask it to be set in the config.

Agree with docs update but disagree with this specific change. If there's an issue with the set-up flow we should address that instead (on get sentry/sentry side)

I'm quite pessimistic on having that changed on the sentry/sentry side, since this is a common issue that most people encountered during their first time self-hosting sentry.

[BYK]

I'm mostly trying to understand how this is not being set in the initial setup. Do we have any links to people asking about this so I can check?

[aldy505]

Do we have any links to people asking about this so I can check?

A few that I remembered:

• https://github.com/getsentry/self-hosted/issues/3050
• https://github.com/getsentry/self-hosted/issues/2621

[BYK]

Sorry for the late turnaround here. How about we try to do //github.com/getsentry/self-hosted/issues/2621#issuecomment-1842014431 and the subsequent comment instead? Hard-coding makes changing this a bit more tricky (although I agree that it should not be something that is changed regularly)

[aldy505]

Sorry for the late turnaround here. How about we try to do //github.com//issues/2621#issuecomment-1842014431 and the subsequent comment instead? Hard-coding makes changing this a bit more tricky (although I agree that it should not be something that is changed regularly)

I don't really get what you're trying to do here.. since I believe modifying through the web app is something that's not persisted on disk (to postgres or file). But again, I'm not so sure about that, hence I prefer it to be done via configuration file.

Don't get me wrong: I wanted the modified configurations to survive between updates, but I don't know whether it will correctly survive that.

[BYK]

since I believe modifying through the web app is something that's not persisted on disk (to postgres or file).

It should persist on Postgres, or somewhere persistent as that's how the options system works. I'll list the relevant code for reference:

1. https://github.com/getsentry/sentry/blob/accca8f98c4231dae9a484c8985da43e7a6c4645/src/sentry/options/defaults.py#L63
2. https://github.com/getsentry/sentry/blob/accca8f98c4231dae9a484c8985da43e7a6c4645/src/sentry/options/manager.py#L304
3. https://github.com/getsentry/sentry/blob/accca8f98c4231dae9a484c8985da43e7a6c4645/static/app/views/admin/adminSettings.spec.tsx#L15

Now in number 1 and 3, you can see that it will be disabled on the UI when it was set from the disk (options file). That said I cought something quite interesting: looks like we fetch the default value from SENTRY_SYSTEM_URL_PREFIX so I'd now encourage setting that (in .env maybe) and documenting it.

WDYT?

[aldy505]

since I believe modifying through the web app is something that's not persisted on disk (to postgres or file).

It should persist on Postgres, or somewhere persistent as that's how the options system works. I'll list the relevant code for reference:

1. https://github.com/getsentry/sentry/blob/accca8f98c4231dae9a484c8985da43e7a6c4645/src/sentry/options/defaults.py#L63
2. https://github.com/getsentry/sentry/blob/accca8f98c4231dae9a484c8985da43e7a6c4645/src/sentry/options/manager.py#L304
3. https://github.com/getsentry/sentry/blob/accca8f98c4231dae9a484c8985da43e7a6c4645/static/app/views/admin/adminSettings.spec.tsx#L15

Now in number 1 and 3, you can see that it will be disabled on the UI when it was set from the disk (options file). That said I cought something quite interesting: looks like we fetch the default value from SENTRY_SYSTEM_URL_PREFIX so I'd now encourage setting that (in .env maybe) and documenting it.

WDYT?

Putting on the .env file might work. Seeing that, do you still prefer setting it on the .env file for visibility, or we'll just leave the additional comment for the configuration file?

@hubertdeng123 @azaslavsky what's your thoughts about this?

[BYK]

Putting on the .env file might work. Seeing that, do you still prefer setting it on the .env file for visibility, or we'll just leave the additional comment for the configuration file?

I feel like .env is better because:

1. It is more visible as you suggest
2. It does not count as "on disk" so should allow changing from the UI

[hubertdeng123]

I too echo what @BYK has mentioned above. The option should be set on the initial set up screen. Personally I have not seen DSN appearing as empty as a reproducible issue and setting the system.url-prefix is sort of a hacky workaround. If we are to fix this I'd prefer to understand the root cause instead of covering this up even more