Part 1: Log systemName in DataflowWorkUnitClient, Commit, and core worker states - #39561
Part 1: Log systemName in DataflowWorkUnitClient, Commit, and core worker states#39561rwiggles wants to merge 2 commits into
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
|
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 |
|
Assigning reviewers: R: @kennknowles added as fallback since no labels match configuration Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
There was a problem hiding this comment.
can we rename DataflowWorkerLoggingMDC.setStageName to DataflowWorkerLoggingMDC.setSytemStageName or DataflowWorkerLoggingMDC.setFusedStageName?
There was a problem hiding this comment.
Renamed to setSystemStageName and getSystemStageName to align with getSystemName() on MapTask and ComputationState.
|
|
||
| writer.println("Active Keys: <br>"); | ||
| for (ComputationState computationState : allComputationStates.get()) { | ||
| writer.print(computationState.getComputationId()); |
There was a problem hiding this comment.
getComputationId here will be useful. Maybe add getSystemName in addition to computationId..
This is printed on debug capture and not visble directly to users.
There was a problem hiding this comment.
Updated to print both computationId and systemName in the debug capture output.
…core worker states - Rename DataflowWorkerLoggingMDC stageName methods to systemStageName per reviewer feedback. - Log both computationId and systemName in MetricsDataProvider debug output.
|
Thank you for the review @arunpandianp! I have pushed a fixup commit addressing both review comments:
|
| writeIfNotEmpty(generator, "thread", String.valueOf(record.getThreadID())); | ||
| writeIfNotEmpty(generator, "job", DataflowWorkerLoggingMDC.getJobId()); | ||
| writeIfNotEmpty(generator, "stage", DataflowWorkerLoggingMDC.getStageName()); | ||
| writeIfNotEmpty(generator, "stage", DataflowWorkerLoggingMDC.getSystemStageName()); |
There was a problem hiding this comment.
can you make sure the values show up as expected in consol logs? consider attaching a screenshot showing the difference with the PR.
First in a series of changes to log the fused stage name instead of the computationId. Fused stage name or SystemName is visible to users on the pipeline execution graph which makes it easier to understand.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
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, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.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)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.