Skip to content

iOS: Logging Part 2 – Basic File Persistence - #69587

Closed
gmedori wants to merge 11 commits into
masterfrom
ios/log-persistence
Closed

iOS: Logging Part 2 – Basic File Persistence#69587
gmedori wants to merge 11 commits into
masterfrom
ios/log-persistence

Conversation

@gmedori

@gmedori gmedori commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

This PR adds the first increment of persistent logging support to TeleportKit.

It:

  • centralizes console and file log formatting in a shared formatter
  • captures associated errors separately from log metadata
  • introduces injectable filesystem and file clients
  • adds RotatingFileLogHandler and the shared RotatingFileWriter
  • serializes synchronous filesystem work away from logging callers
  • writes queued records and informational logs in order
  • implements flush() as an ordered inbox marker that synchronizes all earlier writes
  • adds a macOS-compatible LogBackends test target and shared Xcode scheme
  • updates Verify logging call sites to use the associated error parameter

Scope

This PR intentionally implements only basic append and flush behavior. Rotation, retention, oversized-record truncation, inbox backpressure, dropped-record notices, and advanced failure recovery will follow in separate PRs.

The file handler is not yet bootstrapped into Verify.

Testing

Running the unit tests for this PR should be done on the command line inside the TeleportKit directory via the swift test command. This requires Swift 6.4. If you have the Xcode 27 beta installed, you can do so by using xcrun:

xcrun swift test

Test Cases

  • Previously enqueued records are appended in order before flush() returns.
  • Records enqueued after an earlier flush are still processed.
  • Missing nested parent directories are created before writing.
  • RotatingFileLogHandler sends its formatted record through the shared writer.

@gmedori gmedori added devicetrust no-changelog Indicates that a PR does not require a changelog entry no-test-plan Bypasses the test plan validation bot iOS labels Aug 11, 2026
@gmedori

gmedori commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Apologies for the chunky PR folks! I saw how big it was getting and tried to document a little extra to make it easier to review. Please let me know if it's burdensome and I'll take another stab at breaking it down.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2a990a9f1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines -5 to -8
<key>NSCameraUsageDescription</key>
<string>Teleport Verify uses the built-in QR code scanner to receive enrollment data from the Teleport Web UI</string>
<key>NSFaceIDUsageDescription</key>
<string>Teleport Verify uses Face ID to confirm your presence when proving this device's identity</string>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this now live in mobile/Verify/Verify.xcodeproj/project.pbxproj?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah it does, though it wasn't an intentional change on my part. Sometimes, when you're changing Xcode versions (I updated to Xcode 27 beta 5 with this change) it reorganizes things based on its own internal preferences. It doesn't happen often, but it does happen as this change demonstrates.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this mean that if someone else on the team decides not to run beta 5 that their IDE will revert the change?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it's possible! I've been pretty cavalier with upgrading to the latest beta since I'm functionally the only one building the app at the moment, but should other people take interest in contributing in the near future, the common practice I would employ is having a long-lived xcode27 branch that I would periodically merge main into so that we can keep up with changes in Xcode 27 over the course of the summer and be able to cut over to it at some point.

@zmb3 zmb3 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What do we end up doing with these logs?

Is there some debug view or something where users would be able to see them or send them to us?

Comment thread mobile/Verify/Verify.xcodeproj/project.pbxproj
Comment on lines -5 to -8
<key>NSCameraUsageDescription</key>
<string>Teleport Verify uses the built-in QR code scanner to receive enrollment data from the Teleport Web UI</string>
<key>NSFaceIDUsageDescription</key>
<string>Teleport Verify uses Face ID to confirm your presence when proving this device's identity</string>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this mean that if someone else on the team decides not to run beta 5 that their IDE will revert the change?

@gmedori

gmedori commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

What do we end up doing with these logs?

Is there some debug view or something where users would be able to see them or send them to us?

That's the idea, though the UI isn't in place yet. They'll be recorded locally and if a user has a problem, we can ask them to go into the settings view and send us the logs via email.

@gmedori
gmedori force-pushed the ios/log-persistence branch from b05473c to f03035d Compare August 13, 2026 16:58
@gmedori
gmedori force-pushed the ios/log-persistence branch from f03035d to 14afbbf Compare August 13, 2026 17:16
Base automatically changed from ios/logging to master August 13, 2026 17:34
@gmedori
gmedori force-pushed the ios/log-persistence branch from 14afbbf to ee56483 Compare August 13, 2026 17:35
@gmedori

gmedori commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Closing this to re-open in the new core repo.

@gmedori gmedori closed this Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devicetrust iOS no-changelog Indicates that a PR does not require a changelog entry no-test-plan Bypasses the test plan validation bot size/lg

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants