kean / Pulse

Network logger for Apple platforms
https://pulselogger.com
MIT License
6.27k stars 298 forks source link

Questions #26

Closed shaps80 closed 3 years ago

shaps80 commented 3 years ago

Hi,

Love this and have been wanting something like this for years so I'm so pleased that you've taken the time and patience to implement it (and soooo well too!)

I have a few questions as we have very strict privacy and GDPR requirements I need to satisfy before I can adopt this library.

  1. I know its backed by Apple's OSS logging framework, not OSLog – how do you recommend preventing private data from not being logged?
  2. Is it possible to prevent storing images and other non-text response data?

For more context on (2) our app allows users to upload documents to our backend but we'd never want those logged and stored. These can be (and usually are) highly sensitive documents.

Any advice or help here would be appreciated as I'd really love to adopt this ASAP!

kean commented 3 years ago

Hi,

In my use case, I only enable it in the debug builds and never in production.

  1. There isn't currently anything in the framework to distinguish between public and private data. I would suggest adding it on the app level. I'm open to suggestions. I can add something like interpolation to the LoggerStore API. But in the case of swift-log, we are limited by its APIs. And it's not clear what to do with network requests.
  2. No, but it's in the backlog (currently not publicly available). Contributions are welcome. I think non-text data should probably not be stored by default. And you have full control of what requests you log with manual logging.
shaps80 commented 3 years ago

That all makes sense! I will take a deeper look next week and likely submit PRs!

shaps80 commented 3 years ago

@kean One more question, I don't seem to be able to open .pulse documents on iOS (iPad specifically). Is that unimplemented as of yet?

kean commented 3 years ago

You should be able to do that by going to Settings / Browse Files in PulseUI.MainView.

If you want to be able to open them right from the Files, you need an app on your device registered to handle these file types. There is a dedicated Pulse iOS app just for that, but you'll currently need to compile it from source as I haven't put it on the store yet.

shaps80 commented 3 years ago

That's what I'm talking about. I did all of that but its not working. I checked the project, don't see an registered document types at all. I can recheck later when I'm back at home but I'm fairly sure it wasn't there?

Also opening from the app (likely the same reason) the files are disabled as it doesn't know what it can open.

Seems to work fine with the Mac app.

shaps80 commented 3 years ago

Side note, found some UI bugs in dark mode too, I'll submit PRs for them when I can if you like. I'd love to be a guinea pig for this project and contribute as we come across. Could really help with working across support issues, etc... especially with production users.

shaps80 commented 3 years ago

Ahh I found you have the document stuff defined on the iOS framework but not the demo app. I've added that, can add to a PR too if you like?

kean commented 3 years ago

Absolutely, contributions are welcome – that's the primary reason I decided to open-source it. The project is still in its early stages, so there are plenty of improvements to be made.

Ahh I found you have the document stuff defined on the iOS framework but not the demo app.

You need to compile the document-based app (not the demo). It knows how to open .pulse documents (see func scene(_ scene: UIScene, openURLContexts URLContexts: Set<UIOpenURLContext>), the demo app doesn't. The demo target shows how you can integrate PulseUI framework into your app. It can a bit confusing because the UI is almost the same.

the files are disabled as it doesn't know what it can open.

Interesting, I'm not sure if I tested it separately without having the dedicated document-based app installed.

shaps80 commented 3 years ago

So I got it working in the demo app just adding the document types. I'm confusedly your comment though because the iOS one doesn't even allow device selection. The demo does.


EDIT: Ignore this, not sure what I was doing wrong before but its working as you described now 👍

shaps80 commented 3 years ago

In my use case, I only enable it in the debug builds and never in production.

Out of curiosity is there a reason you don't use it in production? Any concerns I should be aware of?

kean commented 3 years ago

There isn't much use of it in production unless you have a way to get the logs from the users.

It's also still in its early stages. I don't want to risk running this code in prod. There are no known issues, but better safe than sorry.

shaps80 commented 3 years ago

Ok good to know. I’ll test internally for now then. You did provide sharing options though. I don’t see why that couldn’t be used with users.

Our specific case is that support calls come in and we have zero understanding (from a logs pov) of what was happening. So I’m kinda hoping this slowly could provide insights.

We have a few thousand users a week (new users as it’s not an app you reuse, single purpose ID check then uninstall). I’ll start with TestFlight users and go from there.

vg-identance commented 3 years ago

In my use case, I only enable it in the debug builds and never in production.

@kean How did you achieve this? I need to setup it for Debug and Custom configurations and disable (would be better for me that it wouldn't link framework) for Release?

kean commented 3 years ago

I wrap the code that configures the network logging in #if DEBUG.

vg-identance commented 3 years ago

I wrap the code that configures the network logging in #if DEBUG.

Thanks! I use this and combine it with pod '****', '****', :configurations => ['Debug', 'Custom'] in another dependency to avoid linking framework. But I don't know yet how to do that with SPM integrated framework. Any ideas or plans to integrate with Cocoapods?

kean commented 3 years ago

The framework is relatively small, it adds a couple of hundred of KBs to the binary. I simply keep it in the binary. And it doesn't do anything without you actively enabling it.

No plans for CocoaPods.

vg-identance commented 3 years ago

Understood. For me it is important to be sure that external dependency will not break prod build in any way. Like FB SDK which could break prod builds just by linking Framework (https://github.com/facebook/facebook-ios-sdk/issues/1427). A bit paranoid about that :)

No plans for CocoaPods.

Why? No pressure, just really interesting why to not integrate all popular distribution services like Cocoapods or Carthage.

kean commented 3 years ago

There is not enough demand for supporting other package managers anymore.

I'm going to close this ticket. For follow up questions, please open a discussion rather than an issue.

kean commented 3 years ago

@vg-identance, if you want to integrate it in a different way, I recommend XCFrameworks. It's super easy to create one. I have the instructions here https://kean.blog/pulse/guides/installation#frameworks-xcframeworks. That's how I integrate it.