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

RUMM-2173 Add log level threshold option #867

Merged
merged 3 commits into from
Jun 20, 2022

Conversation

xgouchet
Copy link
Member

What and why?

Add a way to filter logs per status before they're sent to Datadog.

How?

Added an option in the logger builder, that's passed to the LogFileOutput.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes

@xgouchet xgouchet requested review from a team as code owners May 25, 2022 13:30
@xgouchet xgouchet force-pushed the xgouchet/RUMM-2173/log_filter branch from 630d8ff to 7b83ccc Compare May 25, 2022 14:22
.build()
```
{{% /tab %}}
{{% tab "Objective-C" %}}
```objective-c
DDLoggerBuilder *builder = [DDLogger builder];
[builder sendNetworkInfo:YES];
[builder setWithDatadogReportingThreshold:.info]
Copy link

Choose a reason for hiding this comment

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

It looks like there may be a missing semicolon at the end of this line. Could you confirm and update if needed?

Copy link
Member

@ncreated ncreated left a comment

Choose a reason for hiding this comment

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

Looks OK 👍 with a blocker on public API typo. I also left a suggestion on scoping the change to only Logger, without touching Crash Reporting code - LTMWDYT.

Comment on lines 362 to 363
/// set the minim log level reported to Datadog servers.
/// Any log with a level equal or above to the threshold will be sent
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// set the minim log level reported to Datadog servers.
/// Any log with a level equal or above to the threshold will be sent
/// Set the minim log level reported to Datadog servers.
/// Any log with a level equal or above to the threshold will be sent.

Sources/Datadog/Logger.swift Outdated Show resolved Hide resolved
tools/lint/run-linter.sh Outdated Show resolved Hide resolved
@xgouchet xgouchet requested review from ncreated and buraizu May 27, 2022 10:54
Copy link
Member

@ncreated ncreated left a comment

Choose a reason for hiding this comment

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

Looks fine, thanks! 👌

@xgouchet xgouchet force-pushed the xgouchet/RUMM-2173/log_filter branch from f384e22 to c6499e0 Compare May 27, 2022 11:55
@xgouchet xgouchet merged commit 5f77d15 into develop Jun 20, 2022
@ncreated ncreated mentioned this pull request Jul 7, 2022
6 tasks
@ncreated ncreated mentioned this pull request Jul 21, 2022
6 tasks
This was referenced Sep 16, 2022
@xgouchet xgouchet deleted the xgouchet/RUMM-2173/log_filter branch September 12, 2023 09:14
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