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

Allow configuration of frontend-platform services without forking #14

Open
davidjoy opened this issue Oct 4, 2021 · 5 comments · Fixed by openedx/frontend-build#215
Labels
enhancement Relates to new features or improvements to existing features

Comments

@davidjoy
Copy link
Contributor

davidjoy commented Oct 4, 2021

frontend-platform is configurable from the perspective that each MFE can configure what services it uses (new relic, segment, etc.) It is not configurable, however, from the perspective of different instances of each MFE configuring different services.

This is a big problem for Open edX - there's no way for people to turn off New Relic, or replace Segment with something else. This may be a blocker for the community and prevent them from actually using any of our MFEs.

The only way to do this today is to fork the repo and change the source code in index.jsx to have the configuration you want.

This work likely depends on the Configuration 2.0 milestone in this project: https://github.com/openedx/frontend-wg/milestone/2

That will allow us to use the configuration file to import alternate implementations and pass them into frontend-platform's configuration, where they can be consumed during initialization.

@regisb
Copy link

regisb commented Nov 9, 2021

@davidjoy I'm trying to figure out where to call the configureLogging(...) function in order to stop using the NewRelicLoggingService. To do so, I am trying to emulate the logging behaviour in development. The docs say:

When in development mode, all messages will instead be sent to the console.

But this is an implementation detail of the NewRelicLoggingService, right? (source) So there is no actual development logging service to which we could default in production, right?

Furthermore, the frontend-build app forces usage of NewRelic and prevents the definition of empty NewRelic credentials (source). So even when defining empty/null New Relic credentials, the New Relic script will still be added to the build.

I think it's unacceptable that every request from every MFE on every Open edX platform out there results in a request to an external service. (users have complained about this here for instance)

This is an old issue and we need to resolve it in Maple. There are many different ways to address this, but I'm not sure which one you would agree with. Please suggest a solution. We can then work on the implementation.

@davidjoy
Copy link
Contributor Author

I think a short term solution might be similar to what we do with SEGMENT_KEY, in that if it's not provided, we shut down the segment integration. A similar solution with the New Relic-specific environment variables (shown here: https://github.com/edx/frontend-build/blob/master/config/webpack.prod.config.js#L175) could let us, at a minimum, 'turn off' the New Relic integration.

So all that said, the longer term solution is what's documented in this task. We should default to not having any logging or analytics services configured and allow operators to provide a config file that specifies an implementation to use, which will get passed through and used during initialization.

@davidjoy
Copy link
Contributor Author

And also, yes, I totally agree that it's unacceptable. This was both an oversight of the original design and a limitation of the current config method.

regisb added a commit to regisb/frontend-build that referenced this issue Nov 25, 2021
By defining the `ENABLE_NEW_RELIC='false'` environment variable, the new relic
logging integration will be disabled, which is a must-have for most non-edX
platforms.

Close openedx/wg-frontend#14
regisb added a commit to regisb/frontend-build that referenced this issue Nov 26, 2021
By defining the `ENABLE_NEW_RELIC='false'` environment variable, the new relic
logging integration will be disabled, which is a must-have for most non-edX
platforms.

Close openedx/wg-frontend#14
regisb added a commit to regisb/frontend-build that referenced this issue Nov 30, 2021
By defining the `ENABLE_NEW_RELIC='false'` environment variable, the new relic
logging integration will be disabled, which is a must-have for most non-edX
platforms.

Close openedx/wg-frontend#14
regisb added a commit to overhangio/tutor-mfe that referenced this issue Dec 14, 2021
- Previously, it was not possible to make the MFEs use the themed logo from the
  LMS. This changed when this PR was merged:
  https://github.com/edx/edx-platform/pull/29503 In Maple, the
  /theming/asset/images/logo.png url now redirects to the themed logo. Close #25.

- We disable New Relic globally by upgrading frontend-build for some packages.
  Close openedx/wg-frontend/issues/14
regisb added a commit to overhangio/tutor-mfe that referenced this issue Dec 14, 2021
- Previously, it was not possible to make the MFEs use the themed logo from the
  LMS. This changed when this PR was merged:
  https://github.com/edx/edx-platform/pull/29503 In Maple, the
  /theming/asset/images/logo.png url now redirects to the themed logo. Close #25.

- We disable New Relic globally by upgrading frontend-build for some packages.
  Close openedx/wg-frontend/issues/14
@regisb
Copy link

regisb commented Dec 14, 2021

This will be resolved in Maple by upgrading frontend-build from 8.0.4 to 8.2.0, where possible: https://github.com/overhangio/tutor-mfe/pull/32/files#diff-d4211adff501de27b7b5207952e99333f754dcd85227d6b1d19bab9e98a1defeR41
Thus, I'm closing this issue.

@regisb regisb closed this as completed Dec 14, 2021
@davidjoy
Copy link
Contributor Author

davidjoy commented Feb 3, 2022

Er, wait, I just noticed @regisb closed this. This is definitely still a problem, and is different than what was implemented in tutor. This is about changing service implementations in frontend-platform, which we can't do at all because the code doesn't support passing those in from environment variables. I'm going to reopen this!

@davidjoy davidjoy reopened this Feb 3, 2022
@arbrandes arbrandes added this to the MFE Config 2.0 milestone Jan 23, 2023
@arbrandes arbrandes added the enhancement Relates to new features or improvements to existing features label Jan 23, 2023
@arbrandes arbrandes removed this from the MFE Config 2.0 milestone Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Relates to new features or improvements to existing features
Projects
Status: Backlog
Development

Successfully merging a pull request may close this issue.

3 participants