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

[#30703][prism] Update logging handling #32826

Merged
merged 4 commits into from
Oct 20, 2024

Conversation

lostluck
Copy link
Contributor

@lostluck lostluck commented Oct 16, 2024

  • Migrate the SDK to consistently use "log/slog" since we've moved to Go 1.21 as base.
  • Improve SDK side logging: logs should be emitted to the prism process at least.
    • TODO in separate PR: Check that Prism is outputting to stderr for local Java and Python execution.
    • Improve the format, clarify the log source for SDK logs. Better field grouping.
    • Include SDK sourced Custom Data structured logging fields if present.
  • Improve Prism's ability to debug issues in the Docker image
    • Print the Docker log when an unexpected context cancellation occurs.
    • Add new debug print when we're unable to copy the Google Auth file to the container.
  • Standalone Prism logging improvements:
  • New flag "log_kind" which sets the log formatting, to one of "json", "text", and "dev". "dev" is the default.
    • "dev" uses the zero dependency "github.com/golang-cz/devslog" package to format console logs instead of the default slog behavior.
    • Adds colour and better structure, more suitable for development uses.
  • New flag "debug" which sets the log level for the Prism binary to Debug, printing all the various debuging prints. Intended for Core SDK developers and Prism developers.
    • Enables Source file lines for prism locations.
  • Improve plumbing of a logger through a job. This work is ongoing, eg. TODO: element manager.
  • Various minor improvements to logging messages, to wrap with slog.Attrs to reduce allocations.

New Development console logs.
Screenshot 2024-10-16 4 22 11 PM

SDK side Errors:
Screenshot 2024-10-16 4 13 51 PM

Prism Debug prints:

Screenshot 2024-10-16 4 10 18 PM

Exact colours will vary on your Terminal configuration.


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
  • Update CHANGES.md with noteworthy changes.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests
Go tests

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

@lostluck
Copy link
Contributor Author

R: @byronellis @damondouglas

This is a smaller change than it looks, it only touches so many files because of the swap to the Standard Library slog package. Otherwise most files can be reviewed independently.

Copy link
Contributor

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment assign set of reviewers

@lostluck lostluck requested a review from damondouglas October 17, 2024 17:34
Copy link
Contributor

@damondouglas damondouglas left a comment

Choose a reason for hiding this comment

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

LGTM

@lostluck lostluck merged commit 1ba33b8 into apache:master Oct 20, 2024
7 checks passed
@lostluck lostluck deleted the updatePrismLogging branch October 20, 2024 15:39
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.

2 participants