-
Notifications
You must be signed in to change notification settings - Fork 135
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
RUMM-3355 Core v2 Configuration #1332
Conversation
76da2d5
to
aab583a
Compare
Datadog ReportBranch report: ✅ |
aab583a
to
7cd8f41
Compare
8515fdf
to
6f780be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really good 👍 - nice to see last V1 things disappear.
let debug = configuration.processInfo.arguments.contains(LaunchArguments.Debug) | ||
if debug { | ||
consolePrint("⚠️ Overriding verbosity, and upload frequency due to \(LaunchArguments.Debug) launch argument") | ||
Datadog.verbosityLevel = .debug |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question/ @maxep What is the plan for getting rid of global .verbosityLevel
? Do we have necessary items in the backlog?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a plan to get rid of it? 🤔
Is is used by DD.logger
which is also global and since it's for development it makes sense keep it this way, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBC: sorry, I don't mean "removing it" 🙂. Do we consider attaching .verbosityLevel
to SDK core instance, or we want to keep it global? Global would mean seeing logs from all instances of the SDK (even not mine). Whatever we decide it is rather low priority.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! Indeed, it would make sense to prefix the console-log entry with the core instance name. It will require a core-specific logger in the same manner we want Telemetry per core. I will add a ticket for that 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea 👍
3a64d24
to
1a40a3e
Compare
d07e4a0
to
1a40a3e
Compare
1a40a3e
to
4381481
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🏅 🏆
What and why?
Introduce public interface to create a
DatadogCore
instance.How?
Default:
Advanced:
Review checklist
Custom CI job configuration (optional)