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.

Sign up for GitHub

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

chore(mac): add breadcrumbs for Sentry #12939

Merged
merged 11 commits into from
Jan 28, 2025
Merged

chore(mac): add breadcrumbs for Sentry #12939

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8e833c8
chore(mac): reworked sentry testing
sgschantz Jan 13, 2025
76d0d69
chore(mac): add KMSentryHelper class, and new tags and breadcrumbs
sgschantz Jan 16, 2025
162dc40
chore(mac): added architecture tag, more breadcrumbs
sgschantz Jan 20, 2025
2b39027
chore(mac): add KMSentryHelper to test project
sgschantz Jan 20, 2025
d6ce0af
chore(mac): added tag for lifecycleState
sgschantz Jan 22, 2025
4a04937
Merge branch 'master' into chore/mac/11623-add-breadcrumbs
sgschantz Jan 22, 2025
b0776a4
chore(mac): Merge branch 'chore/mac/11623-add-breadcrumbs' of https:/…
sgschantz Jan 22, 2025
0ec2664
chore(mac): clean up comments
sgschantz Jan 22, 2025
c9c47e8
chore(mac): fixed else statement for architecture tag
sgschantz Jan 23, 2025
077229e
chore(mac): added comments and renamed function
sgschantz Jan 27, 2025
e5820f5
chore(mac): remove extraneous code
sgschantz Jan 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions mac/Keyman4MacIM/Keyman4MacIM.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@
29BE9D872CA3C21900B67DE7 /* KMModifierMapping.m in Sources */ = {isa = PBXBuildFile; fileRef = 29BE9D862CA3C21900B67DE7 /* KMModifierMapping.m */; };
29C1CDE22C5B2F8B003C23BB /* KMSettingsRepository.m in Sources */ = {isa = PBXBuildFile; fileRef = D861B03E2C5747F70003675E /* KMSettingsRepository.m */; };
29C1CDE32C5B2F8B003C23BB /* KMDataRepository.m in Sources */ = {isa = PBXBuildFile; fileRef = 29015ABC2C58D86F00CCBB94 /* KMDataRepository.m */; };
29DBBF672D35053E00D8E33C /* KMSentryHelper.m in Sources */ = {isa = PBXBuildFile; fileRef = 29DBBF662D35053E00D8E33C /* KMSentryHelper.m */; };
29DD5F442CFEF88000683388 /* SILAndikaV1RGB.png in Resources */ = {isa = PBXBuildFile; fileRef = 29DD5F432CFEF88000683388 /* SILAndikaV1RGB.png */; };
29E67D042D3E4A86003CDE8F /* KMSentryHelper.m in Sources */ = {isa = PBXBuildFile; fileRef = 29DBBF662D35053E00D8E33C /* KMSentryHelper.m */; };
37A245C12565DFA6000BBF92 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 37A245C02565DFA6000BBF92 /* Assets.xcassets */; };
37AE5C9D239A7B770086CC7C /* qrcode.min.js in Resources */ = {isa = PBXBuildFile; fileRef = 37AE5C9C239A7B770086CC7C /* qrcode.min.js */; };
37C2B0CB25FF2C350092E16A /* Help in Resources */ = {isa = PBXBuildFile; fileRef = 37C2B0CA25FF2C340092E16A /* Help */; };
Expand Down Expand Up @@ -275,6 +277,8 @@
29D470972C648D5200224B4F /* el */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = el; path = el.lproj/KMKeyboardHelpWindowController.strings; sourceTree = "<group>"; };
29D470982C648D5200224B4F /* el */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = el; path = el.lproj/MainMenu.strings; sourceTree = "<group>"; };
29D470992C648D7100224B4F /* el */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = el; path = el.lproj/Localizable.strings; sourceTree = "<group>"; };
29DBBF652D35053E00D8E33C /* KMSentryHelper.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = KMSentryHelper.h; sourceTree = "<group>"; };
29DBBF662D35053E00D8E33C /* KMSentryHelper.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = KMSentryHelper.m; sourceTree = "<group>"; };
29DD5F432CFEF88000683388 /* SILAndikaV1RGB.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = SILAndikaV1RGB.png; sourceTree = "<group>"; };
29DD8400276C49E20066A16E /* am */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = am; path = am.lproj/KMAboutWindowController.strings; sourceTree = "<group>"; };
29DD8401276C49E20066A16E /* am */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = am; path = am.lproj/preferences.strings; sourceTree = "<group>"; };
Expand Down Expand Up @@ -433,6 +437,17 @@
path = Privacy;
sourceTree = "<group>";
};
29DBBF642D34FFE000D8E33C /* Logging */ = {
isa = PBXGroup;
children = (
29B4A0D42BF7675A00682049 /* KMLogs.h */,
29B4A0D32BF7675A00682049 /* KMLogs.m */,
29DBBF652D35053E00D8E33C /* KMSentryHelper.h */,
29DBBF662D35053E00D8E33C /* KMSentryHelper.m */,
);
path = Logging;
sourceTree = "<group>";
};
2CDC487CDFFBB26D621D73BD /* Pods */ = {
isa = PBXGroup;
children = (
Expand Down Expand Up @@ -564,8 +579,6 @@
989C9C0A1A7876DE00A20425 /* Keyman4MacIM */ = {
isa = PBXGroup;
children = (
29B4A0D42BF7675A00682049 /* KMLogs.h */,
29B4A0D32BF7675A00682049 /* KMLogs.m */,
299ABD6F29ECE75B00AA5948 /* KeySender.m */,
299ABD7029ECE75B00AA5948 /* KeySender.h */,
D861B03D2C5747F70003675E /* KMSettingsRepository.h */,
Expand All @@ -575,6 +588,7 @@
297A501128DF4D360074EB1B /* Privacy */,
98FE105B1B4DE86300525F54 /* Categories */,
98D6DA791A799EE700B09822 /* Frameworks */,
29DBBF642D34FFE000D8E33C /* Logging */,
989C9C121A7876DE00A20425 /* Images.xcassets */,
98BDD3681BC3511200FAC7C4 /* Keyman4MacIM.entitlements */,
98BF92401BF01CBE0002126A /* KMAboutWindow */,
Expand Down Expand Up @@ -1000,6 +1014,7 @@
9A3D6C5D221531B0008785A3 /* KMOSVersion.m in Sources */,
989C9C111A7876DE00A20425 /* main.m in Sources */,
290BC680274B9DB1005CD1C3 /* KMPackageInfo.m in Sources */,
29DBBF672D35053E00D8E33C /* KMSentryHelper.m in Sources */,
29B4A0D52BF7675A00682049 /* KMLogs.m in Sources */,
98BF924F1BF02DC20002126A /* KMBarView.m in Sources */,
E240F599202DED740000067D /* KMPackage.m in Sources */,
Expand Down Expand Up @@ -1038,6 +1053,7 @@
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
29E67D042D3E4A86003CDE8F /* KMSentryHelper.m in Sources */,
29C1CDE22C5B2F8B003C23BB /* KMSettingsRepository.m in Sources */,
29C1CDE32C5B2F8B003C23BB /* KMDataRepository.m in Sources */,
2915DC512BFE35DB0051FC52 /* KMLogs.m in Sources */,
Expand Down
36 changes: 28 additions & 8 deletions mac/Keyman4MacIM/Keyman4MacIM/KMConfiguration/KMConfigurationWindowController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#import "KMDownloadKBWindowController.h"
#import "KMDataRepository.h"
#import "KMLogs.h"
#import "KMSentryHelper.h"

@interface KMConfigurationWindowController ()
@property (nonatomic, weak) IBOutlet NSTableView *tableView;
Expand Down Expand Up @@ -360,21 +361,27 @@ - (BOOL)tableView:(NSTableView *)tableView acceptDrop:(id<NSDraggingInfo>)info r
- (void)checkBoxAction:(id)sender {
NSButton *checkBox = (NSButton *)sender;
NSString *kmxFilePath = [self kmxFilePathAtIndex:checkBox.tag];
NSString * kmxFileName = [kmxFilePath lastPathComponent];
NSString *partialPath = [KMDataRepository.shared trimToPartialPath:kmxFilePath];
os_log_debug([KMLogs uiLog], "checkBoxAction, kmxFilePath = %{public}@ for checkBox.tag %li, partialPath = %{public}@", kmxFilePath, checkBox.tag, partialPath);
if (checkBox.state == NSOnState) {
os_log_debug([KMLogs uiLog], "Adding active keyboard: %{public}@", kmxFilePath);
os_log_debug([KMLogs uiLog], "enabling active keyboard: %{public}@", kmxFileName);
NSString *message = [NSString stringWithFormat:@"enabling active keyboard: %@", kmxFileName];
[KMSentryHelper addUserBreadCrumb:@"config" message:message];
[self.activeKeyboards addObject:partialPath];
[self saveActiveKeyboards];
}
else if (checkBox.state == NSOffState) {
os_log_debug([KMLogs uiLog], "Disabling active keyboard: %{public}@", kmxFilePath);
os_log_debug([KMLogs uiLog], "disabling active keyboard: %{public}@", kmxFileName);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be easier if addUserBreadCrumb happened inside os_log_debug()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

os_log_debug is a macOS API, so I can't combine there. I'd like to combine Sentry and macOS logging in a single call, but I am also instructed not to wrap the macOS API call because it tracks the location. So if I leave it in place, I would only be wrapping Sentry (which I am essentially doing to simplify it) and that still leaves two function calls from the location I want to log.

NSString *message = [NSString stringWithFormat:@"disabling active keyboard: %@", kmxFileName];
[KMSentryHelper addUserBreadCrumb:@"config" message:message];
[self.activeKeyboards removeObject:partialPath];
[self saveActiveKeyboards];
}
}

- (void)infoAction:(id)sender {
[KMSentryHelper addUserBreadCrumb:@"user" message:@"getting package information"];
NSButton *infoButton = (NSButton *)sender;
NSString *packagePath = [self packagePathAtIndex:infoButton.tag];
if (packagePath != nil) {
Expand All @@ -389,6 +396,7 @@ - (void)infoAction:(id)sender {
}

- (void)helpAction:(id)sender {
[KMSentryHelper addUserBreadCrumb:@"user" message:@"displaying help"];
NSButton *helpButton = (NSButton *)sender;
NSString *packagePath = [self packagePathAtIndex:helpButton.tag];
if (packagePath != nil) {
Expand All @@ -406,22 +414,33 @@ - (void)removeAction:(id)sender {
NSButton *deleteButton = (NSButton *)sender;
NSDictionary *info = [self.tableContents objectAtIndex:deleteButton.tag];
NSString *deleteKeyboardMessage = NSLocalizedString(@"message-confirm-delete-keyboard", nil);
NSString *keyboardName = nil;

if ([info objectForKey:@"HeaderTitle"] != nil) {
keyboardName = [info objectForKey:@"HeaderTitle"];
[self.deleteAlertView setMessageText:[NSString localizedStringWithFormat:deleteKeyboardMessage, keyboardName]];
} else {
keyboardName = [info objectForKey:kKMKeyboardNameKey];
[self.deleteAlertView setMessageText:[NSString localizedStringWithFormat:deleteKeyboardMessage, keyboardName]];
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is identical to l.421, can they be pulled out of the if/else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, thanks for spotting that!

}

if ([info objectForKey:@"HeaderTitle"] != nil)
[self.deleteAlertView setMessageText:[NSString localizedStringWithFormat:deleteKeyboardMessage, [info objectForKey:@"HeaderTitle"]]];
else
[self.deleteAlertView setMessageText:[NSString localizedStringWithFormat:deleteKeyboardMessage, [info objectForKey:kKMKeyboardNameKey]]];

os_log_debug([KMLogs configLog], "entered removeAction for keyboardName: %{public}@", keyboardName);

[self.deleteAlertView beginSheetModalForWindow:self.window completionHandler:^(NSModalResponse returnCode) {
if (returnCode == NSAlertFirstButtonReturn) {
os_log_debug([KMLogs uiLog], "confirm delete keyboard alert dismissed");
[self deleteFileAtIndex:[NSNumber numberWithInteger:deleteButton.tag]];

os_log_debug([KMLogs configLog], "removeAction for keyboardName: %{public}@", keyboardName);
[KMSentryHelper addUserBreadCrumb:@"configure" message:[NSString stringWithFormat:@"remove keyboard: %@", keyboardName]];
}
self.deleteAlertView = nil;
}];
}

- (IBAction)downloadAction:(id)sender {
[KMSentryHelper addUserBreadCrumb:@"user" message:@"download keyboard"];

if (self.AppDelegate.infoWindow_.window != nil)
[self.AppDelegate.infoWindow_ close];

Expand Down Expand Up @@ -453,7 +472,8 @@ - (void)handleRequestToInstallPackage:(KMPackage *) package {
- (void)installPackageFile:(NSString *)kmpFile {
// kmpFile could be a temp file (in fact, it always is!), so don't display the name.
os_log_debug([KMLogs dataLog], "kmpFile - ready to unzip/install Package File: %{public}@", kmpFile);

[KMSentryHelper addInfoBreadCrumb:@"configure" message:@"install package file"];

BOOL didUnzip = [self.AppDelegate unzipFile:kmpFile];

if (!didUnzip) {
Expand Down
1 change: 1 addition & 0 deletions mac/Keyman4MacIM/Keyman4MacIM/KMDownloadKeyboard/KMDownloadKBWindowController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#import "KMDownloadKBWindowController.h"
#import "KMInputMethodAppDelegate.h"
#import "KMLogs.h"
#import "KMSentryHelper.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, I'll remove


@interface KMDownloadKBWindowController ()
@property (nonatomic, weak) IBOutlet WebView *webView;
Expand Down
10 changes: 7 additions & 3 deletions mac/Keyman4MacIM/Keyman4MacIM/KMInputController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#import "KMLogs.h"
#import "InputMethodKit/InputMethodKit.h"
#import "KMInputMethodLifecycle.h"

#import "KMSentryHelper.h"

@implementation KMInputController

Expand All @@ -25,7 +25,7 @@ - (KMInputMethodAppDelegate *)appDelegate {

- (id)initWithServer:(IMKServer *)server delegate:(id)delegate client:(id)inputClient
{
os_log_debug([KMLogs lifecycleLog], "initWithServer, active app: '%{public}@'", [KMInputMethodLifecycle getClientApplicationId]);
os_log_debug([KMLogs lifecycleLog], "initWithServer, active app: '%{public}@'", [KMInputMethodLifecycle getRunningApplicationId]);

self = [super initWithServer:server delegate:delegate client:inputClient];
if (self) {
Expand Down Expand Up @@ -84,7 +84,7 @@ - (void)inputMethodChangedClient:(NSNotification *)notification {
if (_eventHandler != nil) {
[_eventHandler deactivate];
}
_eventHandler = [[KMInputMethodEventHandler alloc] initWithClient:[KMInputMethodLifecycle getClientApplicationId] client:self.client];
_eventHandler = [[KMInputMethodEventHandler alloc] initWithClient:[KMInputMethodLifecycle getRunningApplicationId] client:self.client];

}

Expand All @@ -111,17 +111,21 @@ - (void)menuAction:(id)sender {
NSInteger itag = mItem.tag;
os_log_debug([KMLogs uiLog], "Keyman menu clicked - tag: %lu", itag);
if (itag == CONFIG_MENUITEM_TAG) {
[KMSentryHelper addUserBreadCrumb:@"menu" message:@"Configuration..."];
[self showConfigurationWindow:sender];
}
else if (itag == OSK_MENUITEM_TAG) {
[KMSentryHelper addUserBreadCrumb:@"menu" message:@"On-screen Keyboard"];
[KMSettingsRepository.shared writeShowOskOnActivate:YES];
os_log_debug([KMLogs oskLog], "menuAction OSK_MENUITEM_TAG, updating settings writeShowOsk to YES");
[self.appDelegate showOSK];
}
else if (itag == ABOUT_MENUITEM_TAG) {
[KMSentryHelper addUserBreadCrumb:@"menu" message:@"About"];
[self.appDelegate showAboutWindow];
}
else if (itag >= KEYMAN_FIRST_KEYBOARD_MENUITEM_TAG) {
[KMSentryHelper addUserBreadCrumb:@"menu" message:@"Selected Keyboard"];
[self.appDelegate selectKeyboardFromMenu:itag];
}
}
Expand Down
1 change: 1 addition & 0 deletions mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodAppDelegate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ static const int KEYMAN_FIRST_KEYBOARD_MENUITEM_INDEX = 0;
- (void)postKeyboardEventWithSource: (CGEventSourceRef)source code:(CGKeyCode) virtualKey postCallback:(PostEventCallback)postEvent;
- (KeymanVersionInfo)versionInfo;
- (void)registerConfigurationWindow:(NSWindowController *)window;
- (BOOL)canForceSentryEvents;
@end

#endif /* KMInputMethodAppDelegate_h */
Loading
Loading