Skip to content

Conversation

@armcknight
Copy link
Member

@armcknight armcknight commented Oct 28, 2022

As I've been adding more logging, it's getting harder to keep track of things. This PR adds a new part to the format denoting which class/file the log originated from (and the line number). I also standardized how the different tokens are formatted in the log statement, like our SDK name, log level, and the newly added log source.

Before:

Sentry - debug:: SentryFileManager.cachePath: /Users/andrewmcknight/Library/Developer/CoreSimulator/Devices/9DA57C91-8188-405D-87DD-4CCFFD376F99/data/Containers/Data/Application/0D15944C-6F34-4C95-BE40-3E9B5CF41ED9/Library/Caches
Sentry - debug:: SentryHttpTransport: sendAllCachedEnvelopes start.
Sentry - debug:: SentryHttpTransport: No envelopes left to send.
Sentry - debug:: SentryHttpTransport: Finished sending.
Sentry - debug:: SDK initialized! Version: 7.29.0
Sentry - debug:: Sent 0 crash report(s)
Sentry - debug:: Integration installed: SentryCrashIntegration
Sentry - debug:: Not going to enable SentryANRTrackingIntegration because the debugger is attached.
Sentry - debug:: Integration installed: SentryScreenshotIntegration
Sentry - debug:: Integration installed: SentryUIEventTrackingIntegration
Sentry - debug:: Integration installed: SentryViewHierarchyIntegration
Sentry - debug:: Integration installed: SentryFramesTrackingIntegration
Sentry - debug:: Add breadcrumb: <SentryBreadcrumb: 0x600001861900, {

after

[Sentry] [debug] [SentryFileManager:50] SentryFileManager.cachePath: /Users/andrewmcknight/Library/Developer/CoreSimulator/Devices/9DA57C91-8188-405D-87DD-4CCFFD376F99/data/Containers/Data/Application/36CE0EA5-5C63-407B-96CC-743F0F81DCEC/Library/Caches
[Sentry] [debug] [SentryHttpTransport:241] sendAllCachedEnvelopes start.
[Sentry] [debug] [SentryHttpTransport:253] No envelopes left to send.
[Sentry] [debug] [SentryHttpTransport:320] Finished sending.
[Sentry] [debug] [SentrySDK:156] SDK initialized! Version: 7.29.0
[Sentry] [debug] [SentryCrashInstallationReporter:52] Sent 0 crash report(s)
[Sentry] [debug] [SentrySDK:390] Integration installed: SentryCrashIntegration
[Sentry] [debug] [SentryBaseIntegration:29] Not going to enable SentryANRTrackingIntegration because the debugger is attached.
[Sentry] [debug] [SentrySDK:390] Integration installed: SentryScreenshotIntegration
[Sentry] [debug] [SentrySDK:390] Integration installed: SentryUIEventTrackingIntegration
[Sentry] [debug] [SentrySDK:390] Integration installed: SentryViewHierarchyIntegration
[Sentry] [debug] [SentrySDK:390] Integration installed: SentryFramesTrackingIntegration
[Sentry] [debug] [SentryScope:128] Add breadcrumb: <SentryBreadcrumb: 0x60000279a1c0, {

Someone had actually started doing this from SentryHttpTransport, which I removed from the actual log strings to deduplicate as this solves the same problem from the logging macros themselves so that anywhere they're used, this appears in the logs. It must be done in the macros, since it uses the __FILE__ builtin; if that were used in SentryLogOutput.m, then all the log statements would contain [SentryLogOutput].

#skip-changelog

Comment on lines -114 to +113
SENTRY_LOG_DEBUG(@"SentryHttpTransport: RateLimit is active for all envelope items.");
SENTRY_LOG_DEBUG(@"RateLimit is active for all envelope items.");
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed these in favor of the automatic injection in the macros.

@github-actions

This comment was marked as off-topic.

Comment on lines 19 to 23
[SentryLog logWithMessage:[NSString stringWithFormat:__VA_ARGS__] andLevel:kSentryLevelDebug]
[SentryLog logWithMessage:[NSString stringWithFormat:@"[%@:%d] %@", \
[[[NSString stringWithUTF8String:__FILE__] \
lastPathComponent] stringByDeletingPathExtension], \
__LINE__, [NSString stringWithFormat:__VA_ARGS__]] \
andLevel:kSentryLevelDebug]
Copy link
Member Author

Choose a reason for hiding this comment

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

The change that automatically adds the log source to all logs using these macros.

Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

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

LGTM

@armcknight armcknight merged commit 4e037c4 into master Oct 31, 2022
@armcknight armcknight deleted the armcknight/ref/logging-improvements branch October 31, 2022 23:12
kevinrenskers added a commit that referenced this pull request Nov 1, 2022
* master:
  meta: add log source to format; update log format (#2337)
  test: disable flaky testFlush_CalledMultipleTimes_ImmediatelyReturnsFalse (#2339)
  fix: dont require profiler to start on main thread (#2345)
@philipphofmann
Copy link
Member

I have wanted to do this already for a while 😁. 🥇 to you @armcknight for finally doing that.

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.

5 participants