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

Use structured logging on Fuchsia #49918

Merged
merged 1 commit into from
Jan 30, 2024
Merged

Conversation

mbrase
Copy link
Contributor

@mbrase mbrase commented Jan 20, 2024

This change migrates off of the old fuchsia logging apis to use the
structured logging apis. The initial FIDL connection is made during
global initialization (before main()) and the initial minimum log level
is queried from the system. Later on, once the main loop is initialized,
we setup an async task to listen for additional log interest changes
from the system. The advantage of doing this on the main loop is that we
avoid spawning an additional background thread in the process (the
legacy logging apis use the background thread approach).

One added benefit of this change is it reduces the size of the
dart/flutter runner far packages by about 250kb in release mode, because
libsyslog.so and libbackend_fuchsia_globals.so are no longer needed.

flutter/flutter#141924

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@mbrase mbrase force-pushed the strutured_logging branch 2 times, most recently from b44c2ed to 06f290c Compare January 23, 2024 19:17
@mbrase mbrase self-assigned this Jan 24, 2024
@mbrase mbrase marked this pull request as ready for review January 24, 2024 17:54
@mbrase mbrase requested a review from jrwang January 25, 2024 01:21
Copy link
Contributor

@jrwang jrwang left a comment

Choose a reason for hiding this comment

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

LGTM

fml/platform/fuchsia/log_state.h Outdated Show resolved Hide resolved
fml/platform/fuchsia/log_state.h Outdated Show resolved Hide resolved
@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 25, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 25, 2024
Copy link
Contributor

auto-submit bot commented Jan 25, 2024

auto label is removed for flutter/engine/49918, due to - The status or check suite cla/google has failed. Please fix the issues identified (or deflake) before re-applying this label.

This change migrates off of the old fuchsia logging apis to use the
structured logging apis. The initial FIDL connection is made during
global initialization (before main()) and the initial minimum log level
is queried from the system. Later on, once the main loop is initialized,
we setup an async task to listen for additional log interest changes
from the system. The advantage of doing this on the main loop is that we
avoid spawning an additional background thread in the process (the
legacy logging apis use the background thread approach).
@mbrase
Copy link
Contributor Author

mbrase commented Jan 26, 2024

This is ready to merge, but I'm holding off until b/322387862 is resolved.

@mbrase mbrase merged commit 19f6f40 into flutter:main Jan 30, 2024
27 checks passed
@mbrase mbrase deleted the strutured_logging branch January 30, 2024 04:27
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 30, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jan 30, 2024
…142515)

flutter/engine@438e9b4...0e586d1

2024-01-30 jonahwilliams@google.com [Impeller] Add interface for submitting multiple command buffers at once. (flutter/engine#50139)
2024-01-30 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 4.1.0 to 4.3.0 (flutter/engine#50165)
2024-01-30 skia-flutter-autoroll@skia.org Roll Skia from 083c1a4d9767 to 4e992fb3a9db (1 revision) (flutter/engine#50164)
2024-01-30 mbrase@google.com Use structured logging on Fuchsia (flutter/engine#49918)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC matanl@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 30, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jan 30, 2024
…142543)

flutter/engine@438e9b4...0e4342c

2024-01-30 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] Add interface for submitting multiple command buffers at once." (flutter/engine#50174)
2024-01-30 skia-flutter-autoroll@skia.org Roll Skia from a3d46fac53be to 7df4d35e11cf (3 revisions) (flutter/engine#50173)
2024-01-30 skia-flutter-autoroll@skia.org Roll Skia from c2fada52fdc4 to a3d46fac53be (2 revisions) (flutter/engine#50171)
2024-01-30 skia-flutter-autoroll@skia.org Roll Skia from 7dc9ba2e8c90 to c2fada52fdc4 (1 revision) (flutter/engine#50170)
2024-01-30 skia-flutter-autoroll@skia.org Roll Skia from 1c0eae94fc09 to 7dc9ba2e8c90 (1 revision) (flutter/engine#50169)
2024-01-30 skia-flutter-autoroll@skia.org Roll Skia from 4e992fb3a9db to 1c0eae94fc09 (3 revisions) (flutter/engine#50167)
2024-01-30 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from Hqi_x_A9lYsY58VSn... to Z-xFM2ILZJw22eU8q... (flutter/engine#50166)
2024-01-30 jonahwilliams@google.com [Impeller] Add interface for submitting multiple command buffers at once. (flutter/engine#50139)
2024-01-30 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 4.1.0 to 4.3.0 (flutter/engine#50165)
2024-01-30 skia-flutter-autoroll@skia.org Roll Skia from 083c1a4d9767 to 4e992fb3a9db (1 revision) (flutter/engine#50164)
2024-01-30 mbrase@google.com Use structured logging on Fuchsia (flutter/engine#49918)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from Hqi_x_A9lYsY to Z-xFM2ILZJw2

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC matanl@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants