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

Android Embedding Refactor PR32: Clean up logs in new embedding. #9351

Conversation

matthew-carroll
Copy link
Contributor

Android Embedding Refactor PR32: Clean up logs in new embedding.

This PR:

  • Migrates all Log usages in the new embedding to the io.flutter.Log proxy
  • Removes logs that I believe are not useful for typical embedding debugging
  • Adds logs that I believe would be useful for typical embedding debugging
  • Adjusts all log levels to what seemed like an appropriate level to me

Address #33544 and #33102

Copy link
Contributor

@mklim mklim left a comment

Choose a reason for hiding this comment

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

LGTM

This is highly subjective and they're already marked as verbose so I'm not blocking merging the PR on this, but for what it's worth the verbose logs here would be total noise to me if I were trying to debug something in these classes. I'd want to go through the source code myself (or ideally attach a debugger) first to really grok how the parts aligned in detail in order to make sense of the method names from logcat, and at that point I'd either already have a debugger to check the execution path or I'd be able to add logs myself to specific interesting codepaths while I was reading through them myself. Just looking at a chain of method names in logcat on its own would probably be too much info to be useful, since I wouldn't be able to tell what was normal and what was potentially off and relevant to the specific thing I was looking into.

@matthew-carroll
Copy link
Contributor Author

@mklim if it's worth a discussion, feel free to grab me. As one who generally knows the pieces in the embedding, I've found that these messages illuminate the order that things are happening, which can be difficult to track mentally. They're verbose so that unless you really want them, you don't need to see them. But if there is a way to make the logs more useful then I'm happy to talk it over.

@matthew-carroll matthew-carroll merged commit f2e76a8 into flutter:master Jun 18, 2019
@goderbauer
Copy link
Member

Some of these logs show up in my terminal during a regular flutter run:

V/DartMessenger( 7739): Received message from Dart over channel 'flutter/platform'
V/DartMessenger( 7739): Deferring to registered handler to process message.
V/PlatformChannel( 7739): Received 'SystemSound.play' message.
V/DartMessenger( 7739): Received message from Dart over channel 'flutter/platform'
V/DartMessenger( 7739): Deferring to registered handler to process message.
V/PlatformChannel( 7739): Received 'SystemSound.play' message.
V/DartMessenger( 7739): Received message from Dart over channel 'flutter/platform'
V/DartMessenger( 7739): Deferring to registered handler to process message.
V/PlatformChannel( 7739): Received 'SystemSound.play' message.
V/DartMessenger( 7739): Received message from Dart over channel 'flutter/platform'
V/DartMessenger( 7739): Deferring to registered handler to process message.
V/PlatformChannel( 7739): Received 'SystemSound.play' message.
V/DartMessenger( 7739): Received message from Dart over channel 'flutter/platform'
V/DartMessenger( 7739): Deferring to registered handler to process message.
V/PlatformChannel( 7739): Received 'SystemSound.play' message.
V/DartMessenger( 7739): Received message from Dart over channel 'flutter/platform'
V/DartMessenger( 7739): Deferring to registered handler to process message.
V/PlatformChannel( 7739): Received 'SystemSound.play' message.

Was that intentional? They feel like spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants