[Go SDK + Protos] Fix Proto Spec for Pane encoding + Go SDK implementation.

[lostluck]

While implementing Triggers + Panes for Prism, I ran into inexplicable failing tests.

It turns out that portable encoding of panes in the protos does not match the implementations in the Python, and Java SDKs, or the internal Dataflow implementation, and never has in the 5 years since it was added to the protocol buffers.

Specifically, the bits for first and last panes are swapped with first being the 7 indexed bit, and last being the 6 indexed bit.

Compare Python and Java
https://github.com/apache/beam/blob/master/sdks/python/apache_beam/utils/windowed_value.py#L92

https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/windowing/PaneInfo.java#L134

To Go's current implementation.

https://github.com/apache/beam/blob/release-2.62.0/sdks/go/pkg/beam/core/graph/coder/panes.go#L32

It is worth noting that the Go implemenation didn't have the swap some years ago when it was first added in b0e9f26 but it wasn't unit tested at the time.

But due to a different error in the implementation, the bit swap occurred in 657caa8 where it was made to match the spec in the protocol buffers on top of fixing the other error.

This error wasn't caught until now due to Triggers (and thus Panes) being among the last features implemented in the SDK, and the least tested due to no real portable implementations of TestStream necessary to exercise them. The Go SDK doesn't currently provide facilities to assert against specific pane properties, and it is unlikely for there to be user code for this since it seems like the Go SDK also doesn't propagate panes properly. See https://github.com/apache/beam/pull/31174/files and #31153 .

---

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)

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

[github-actions]

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

[lostluck]

The failures are unrelated to this change.

---

Looks like the Java Precommit and Examples are hit by an unrelated license generation issue, Tracked in #33836 .

---

Prism doesn't yet support BoundedTried metrics, and tests were added in the last week or two to the JAva test suite, causing the prism suite to fail.

org.apache.beam.sdk.metrics.MetricsTest$AttemptedMetricTests > testAllAttemptedMetrics FAILED
java.lang.AssertionError at MetricsTest.java:435

org.apache.beam.sdk.metrics.MetricsTest$AttemptedMetricTests > testAttemptedBoundedTrieMetrics FAILED
java.lang.AssertionError at MetricsTest.java:475

org.apache.beam.sdk.metrics.MetricsTest$CommittedMetricTests > testCommittedBoundedTrieMetrics FAILED

Fixed by #33843

[lostluck]

R: @kennknowles @jrmccluskey

[github-actions]

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

[kennknowles]

These are equivalent, but I do prefer the phrasing you have altered it to.

[lostluck]

AFAICT They are not.

AfterEndOfWindow is defined as having the sub triggers as implicitly repeating with no default behavior if a sub trigger isn't defined (technically, it defaults to the never trigger). So, Repeat { AfterEndOfWindow } has only an ontime, and maybe a closing firing. The repeat isn't semantically useful.