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

temp: added new relic legacy support along with Datadog #26

Merged
merged 3 commits into from
May 13, 2024

Conversation

abdullahwaheed
Copy link
Contributor

Added temporary support for NewRelicLoggingService. We'll remove this once we'll fully adapt Datadog

@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

package.json Outdated
@@ -24,7 +24,8 @@
"homepage": "https://github.com/edx/frontend-logging#readme",
"dependencies": {
"@datadog/browser-logs": "^5.17.1",
"@datadog/browser-rum": "^5.17.1"
"@datadog/browser-rum": "^5.17.1",
"@edx/frontend-platform": "^8.0.2"
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we should include @edx/frontend-platform as a regular dependency here as we'd risk including 2 copies of frontend-platform (potentially unnecessary increasing bundle size), instead of relying on the frontend-platform already installed by the consuming MFE application.

Should this be treated as a peerDependencies instead?

I'm also a bit concerned around needing to rely on @edx/frontend-platform to rely on its NewRelicLoggingService due to peer dependency conflicts when trying to roll this out. For example, if we introduce a peer dependency on @edx/frontend-platform for v8, MFEs that have not yet upgraded to v8 will need ensure its upgraded in the MFE first before their edx-internal configs can begin consuming DatadogLoggingService.

If we do the peer dependency approach, we'd probably want to list out all supported major versions of @edx/frontend-platform to mitigate the potential for peer dependency conflicts when consuming frontends begin using DatadogLoggingService.

src/DatadogLoggingService.js Show resolved Hide resolved
@abdullahwaheed abdullahwaheed merged commit 8054e3d into master May 13, 2024
5 checks passed
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