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.

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

Feature: Allow to add configuration per store (Multiple DSN...) #136

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

bruno-blackbird
Copy link

@bruno-blackbird bruno-blackbird commented Apr 21, 2024

Summary

This pull request aims to add the ability to use distinct configuration per Magento Stores or Websites. We use it for a multi-website Magento where one Website is legacy and based on Luma and the Other one is based on Hyva theme.

As two different teams are working on the distinct websites, and to avoid merging issues between them, I needed to be able to customize the "dsn" and other configurations per website.

I also added the ability in the configuration to have the checkbox “can restore” on all config fields. So I can version a default configuration for each website on a config.xml file without blocking the ability to edit them through the configuration if needed.

An example of config.xml file :

<default>
        <sentry>
            <general>
                <enable_script_tag>1</enable_script_tag>
                <script_tag_placement>before.body.end</script_tag_placement>
                <enable_session_replay>1</enable_session_replay>
                <replay_session_sample_rate>0.0</replay_session_sample_rate>
                <replay_error_sample_rate>1.0</replay_error_sample_rate>
                <replay_block_media>0</replay_block_media>
                <replay_mask_text>0</replay_mask_text>
                <use_logrocket>0</use_logrocket>
                <enable_php_tracking>1</enable_php_tracking>
            </general>
            <environment>
                <enabled>1</enabled>
                <environment>local</environment>
                <tracing_enabled>1</tracing_enabled>
                <tracing_sample_rate>1.0</tracing_sample_rate>
                <dsn>***/1</dsn>
                <log_level>300</log_level>
                <mage_mode_development>1</mage_mode_development>
                <errorexception_reporting>32767</errorexception_reporting>
            </environment>
            <issue_grouping>
                <strip_static_content_version>1</strip_static_content_version>
                <strip_store_code>0</strip_store_code>
            </issue_grouping>
        </sentry>
    </default>
    <websites>
        <websiteone>
            <sentry>
                <environment>
                    <dsn>***/2</dsn>
                </environment>
            </sentry>
        </websiteone>
        <websitetwo>
            <sentry>
                <environment>
                    <dsn>***/3</dsn>
                </environment>
            </sentry>
        </bultex>
    </websitetwo>

The only element I kept in the env.php is the environment name (staging or production):

 'system' => [
        'default' => [
             'sentry' => [
                    'environment' => "staging"
             ]
        ]
]

@indykoning
Copy link
Member

While i like the idea, and LOVE the nitpick fixes 🚀
I'm afraid to give the impression to users that in contexts where the store is not clear e.g. the cron will send to the specified store's sentry instance.
A GraphQL call missing the store header would probably send the error to an unexpected sentry instance

@bruno-blackbird
Copy link
Author

Hi @indykoning Thanks for your answer.
The way I configured it, is a project for the default values (for all logs not linked to a store, for cron jobs or graphql queries).it
And one project per website on my instance.

This feature is only a new possibility that is not mandatory to use.

@GeKVe
Copy link

GeKVe commented Dec 3, 2024

Thank you so much for adding this PR @bruno-blackbird 👏 ! I've merged it alongside other improvements in my own fork. Much needed, and works perfectly! 🙏 THANK YOU!

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