## What changed?
Added support for defining protos in chasm libs.
## Why?
Keep everything local to the library.
## How did you test it?
- [x] built
- [x] run locally and tested manually
## What changed?
Enable functional tests with OpenSearch 2 in CICD
## Why?
Run compatibility with OpenSearch 2
## How did you test it?
- [ ] built
- [ ] run locally and tested manually
- [ ] covered by existing tests
- [ ] added new unit test(s)
- [ ] added new functional test(s)
## Potential risks
## What changed?
<!-- Describe what has changed in this PR -->
Remove license header from every file. Because it is really hard to
follow in this PR here is the summary:
1. License header is removed from all `.go` and `.proto` files
:fireworks::fireworks:🎆.
2. `LICENSE` file in the root directory has only Temporal and Uber
copyrights.
3. 5 other `LICENSE` files added to the packages which have copyrights
different from Temporal and Uber: Datadog, Xargin, "Mat Ryer, Tyler
Bunnell and contributors".
4. `license_file` flag is removed from all code generation tools.
5. `copyright_file` flag is removed from `go:generate mockgen`
directive.
6. All copyright related targets are removed from `Makefile`.
7. Updated Temporal copyright year to 2025 everywhere.
## Why?
<!-- Tell your future self why have you made these changes -->
I double checked with legal department that it is not needed to have
license header in every file. One file per repo is enough. I put all
copyrights to the root `LICENSE` file and removed header from all other
files. Also updated tools and `Makefile`.
## What changed?
Send raw history events from history to matching service similar to the
change in https://github.com/temporalio/temporal/pull/7179.
Instead of deserializing history blobs to history events, history
service will send the raw bytes to matching service. History service
uses a different proto message to do that. Matching client response
proto has History message in the same field. GRPC's proto
deserialization will automatically deserialize raw history batch bytes
to History message. Matching to frontend response still have
deserialized History message in the response. And there is not much CPU
usage to save by changing this.
## Why?
We have seen incidents of high history CPU usage when large amount of
history events are pulled. With this change we can reduce the CPU burden
on history service during this time.
## How did you test it?
Unit tests
## Potential risks
## Documentation
## Is hotfix candidate?
No
## What changed?
Change to send raw history blobs from history service to frontend
service. History service returns a new proto message that has a repeated
bytes history field.
This response is wire compatible with the original response which has
temporal.api.history.v1.History type for this field. This allows history
service to not deserialize events from this data blob. This considerably
reduces CPU usage.
History service still needs event_id and version decoded from history
events. For this we use a new proto message StrippedHistoryEvent which
has these two fields only. It takes considerably less CPU to decode
events to this struct.
## Why?
We have seen incidents of high history CPU usage when large number of
GetWorkflowExecutionHistory calls are made to workflows which has large
history. With this change we can reduce the CPU burden on history
service during this API call.
## How did you test it?
Existing unit and functional tests.
## Potential risks
## Documentation
## Is hotfix candidate?
- **Revert "Fix history batch validations (#7281)"**
- **Revert "Send raw history blobs from history service to frontend
(#7179)"**
## What changed?
Revert the changes for returning raw history event from history to
frontend.
## Why?
Some logic is moved from history to frontend handler. This might cause
issues during upgrade.
I will push a change to first copy these operations to frontend. Once
that change is released, I will reapply this change.
## How did you test it?
## Potential risks
## Documentation
## Is hotfix candidate?
## What changed?
Change to send raw history blobs from history service to frontend
service. History service returns a new proto message that has a repeated
bytes history field.
This response is wire compatible with the original response which has
temporal.api.history.v1.History type for this field. This allows history
service to not deserialize events from this data blob. This considerably
reduces CPU usage.
History service still needs event_id and version decoded from history
events. For this we use a new proto message StrippedHistoryEvent which
has these two fields only. It takes considerably less CPU to decode
events to this struct.
## Why?
We have seen incidents of high history CPU usage when large number of
GetWorkflowExecutionHistory calls are made to workflows which has large
history. With this change we can reduce the CPU burden on history
service during this API call.
## How did you test it?
Existing unit tests and manual test to run workflows.
## Potential risks
## Documentation
## Is hotfix candidate?
No
## What changed?
<!-- Describe what has changed in this PR -->
Open up Grafana Tempo's OTEL HTTP port, too.
## Why?
<!-- Tell your future self why have you made these changes -->
It's useful to send traces via curl.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
## Documentation
<!-- Have you made sure this change doesn't falsify anything currently
stated in `docs/`? If significant
new behavior is added, have you described that in `docs/`? -->
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
## What changed?
<!-- Describe what has changed in this PR -->
Added Grafana Tempo - an OTEL collector - to the developer setup.
Since we're already using Grafana, Tempo seemed like a logical choice.
## Why?
<!-- Tell your future self why have you made these changes -->
It allows developers to see all the RPC calls they are making (e.g. when
running the server locally or a functional test).
Tempo allows filtering by workflow ID. _(note that not _all_ places are
tagged yet, but lots of them)_
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
```
make start-grafana-tempo
make OTEL=true start
```
```
tctl --ns default namespace register
tctl wf start --tq test --wid test -wt test
```
<img width="3352" alt="image"
src="https://github.com/user-attachments/assets/5809691a-da32-4a04-8e80-700f8acd47e5"
/>
And running a functional test from the IDE:
<img width="3346" alt="image"
src="https://github.com/user-attachments/assets/061bfaf3-0a34-4b4e-a06c-497dfa3a948a"
/>
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
## Documentation
<!-- Have you made sure this change doesn't falsify anything currently
stated in `docs/`? If significant
new behavior is added, have you described that in `docs/`? -->
Updated docs/development/tracing.md
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
## What changed?
<!-- Describe what has changed in this PR -->
Using the latest `grafana`, `temporalio/ui` and `prometheus` images for
our local dev setup.
## Why?
<!-- Tell your future self why have you made these changes -->
The current versions are 6+ months old.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
Started locally.
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
## Documentation
<!-- Have you made sure this change doesn't falsify anything currently
stated in `docs/`? If significant
new behavior is added, have you described that in `docs/`? -->
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
## What changed?
Leverage gotestsum to automatically retry failed tests in all suites
(unit, integration, functional, xdc, ndc).
By default, tests will be retried twice if there are no more than 10
failed tests in the suite.
### Changes to the `Makefile`
Add `FAILED_TEST_RETRIES` variable, defaulting to `2`.
Removed the use of `SINGLE_TEST_ARGS`, `TEST_ARGS` can be used instead.
Moved `-race` flag to its own separate flag and freed up `TEST_ARGS` to
be set by callers without.
Added:
```
# Whether or not to test with the race detector. All of (1 on y yes t true) are true values.
TEST_RACE_FLAG ?= on
# Whether or not to shuffle tests. All of (1 on y yes t true) are true values.
TEST_SHUFFLE_FLAG ?= on
```
Added `COMPILED_TEST_ARGS` that combines the common flags and options to
be reused by all test invocations.
### Changes to `run-tests.yml`
Removed the use of `SINGLE_TEST_ARGS`.
Merged the `Run single unit test` step into `Run unit tests`.
Set `FAILED_TEST_RETRIES: "0"` on flaky test detection steps.
Removed steps to upload test results to buildkite and relevant script /
Makefile content.
Moved flaky test detection to after the upload test results (to GH) step
in all jobs.
## Why?
So we can enable flaky tests to get coverage without hurting the CI
experience.
## What changed?
Run cassandra with smaller heap (2GB) in CI. Default (on standard GHA
workers) comes out to 4GB, so this is 1/2 of what it was before.
## Why?
Try to reduce CI flakiness.
## What changed?
Add workflow dispatch options to the functional tests Github Action to
allow us to run n-iterations of a single functional test with a
configurable timeout.
There is also an option to run n-iterations of a single unit test,
although it may be faster to run that locally.
WARNING: For functional tests, this will definitely be oomkilled for
n>=100, likely for n=>50 too. I suggest to start with n=20 to see how
the memory goes and then increase from there. Different DBs may use
different amounts of RAM also.
## Why?
To aid in the diagnosis and treatment of flaky tests.
## How did you test it?
Tested in github actions.
Here is the action run normally, with no new input parameters:
https://github.com/temporalio/temporal/actions/runs/11261156403
Here is the action run on one test multiple times:
https://github.com/temporalio/temporal/actions/runs/11261236304
While we still have buildkite, the uploaded test results will be
uploaded. You can find them by going to
https://buildkite.com/organizations/temporal/analytics/suites/temporal-public/runs?branch=all+branches
and looking for a recent run with `"job: functional-test"` and the
commit hash you used.
Here is the buildkite output of the run above:
https://buildkite.com/organizations/temporal/analytics/suites/temporal-public/runs/39901afd-35f1-8171-9257-e0bada374824
## Potential risks
Our functional test pipeline could be broken by this PR, but we would
notice that pretty immediately
## Documentation
How to run it yourself
1. Go to
https://github.com/temporalio/temporal/actions/workflows/run-tests.yml
2. Click "run workflow" on the upper right hand side
5. Set Commit SHA to the latest commit on the branch
6. Select your desired options
7. Click the green "Run workflow" button
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
## What changed?
Fix buf-breaking script to account for already merged commit
## Why?
Re-running GHA run-tests for old commits can fail if there are changes
to proto files in main branch after the commit you're trying to test.
## What changed?
- Add (back) make target buf-breaking to run buf's breaking proto
changes check against the parent commit and the main branch.
- Add buf-breaking to ci-build-misc so it runs in CI.
- Fix goimports target to use shell variables instead of git variables
to make make faster, and also fix the binary name (was just broken).
- GitHub Actions: run tests on PR head, not merge.
## Why?
Detecting breaking proto changes early avoids bugs.
## How did you test it?
Manual testing with deliberate breaking changes.
## What changed?
- Rearrange make targets and scripts so that generated proto files (in
`api`) are fully generated before replacing existing files.
- Put all steps for generating proto files in a single script.
## Why?
- Previously, if any part of proto generation failed (including
installing dependencies), the repo would be left in a broken state with
either missing files, or incorrectly-generated files, depending on where
the failure was. This runs all the generation steps in a designated
order before touching any existing files, then moves them over in one
step. On failure, the new files are left in `api.new`.
- Relying on make ordering is icky.
## How did you test it?
manual testing
## What changed?
<!-- Describe what has changed in this PR -->
Separate test execution and test result upload. And check XML file
exists.
Previously introduced here
https://github.com/temporalio/temporal/pull/6049
## Why?
<!-- Tell your future self why have you made these changes -->
make aborts execution when the previous target fails. Oops.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
Scenarios:
- [x] no Buildkite secret set
- [x] make target has test errors
- [x] make target has no test errors
- [x] make target aborts (no XML file written)
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
## Documentation
<!-- Have you made sure this change doesn't falsify anything currently
stated in `docs/`? If significant
new behavior is added, have you described that in `docs/`? -->
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
## What changed?
<!-- Describe what has changed in this PR -->
Uploading test results to Buildkite for analytical purposes.
⚠️ This will not work for any PR from a fork (Temporal developer or
not), due to GitHub's security model. It has to be a branch on this repo
to run successfully (includes `main`, ofc).
## Why?
<!-- Tell your future self why have you made these changes -->
To be able to analyze test flakiness.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
Ran this PR and saw test run in Buildkite.
<img width="1261" alt="Screenshot 2024-05-31 at 5 11 07 PM"
src="https://github.com/temporalio/temporal/assets/159852/f04f7392-12e2-4325-a161-ea038e9fe281">
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
## Documentation
<!-- Have you made sure this change doesn't falsify anything currently
stated in `docs/`? If significant
new behavior is added, have you described that in `docs/`? -->
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
## What changed?
<!-- Describe what has changed in this PR -->
Update temporalio/ui docker image version
## Why?
<!-- Tell your future self why have you made these changes -->
New ui features.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
## Documentation
<!-- Have you made sure this change doesn't falsify anything currently
stated in `docs/`? If significant
new behavior is added, have you described that in `docs/`? -->
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
## What changed?
<!-- Describe what has changed in this PR -->
I reorganized our docs into `docs/{admin,architecture,development}`.
## Why?
<!-- Tell your future self why have you made these changes -->
We discussed this internally, but I believe this is a better
segmentation for the different audiences looking at our docs. I plan on
adding more stuff to docs/admin like metrics and dynamic config docs.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
I'm mainly looking for broken links.
- I went through each markdown file in docs manually.
- I looked through all inspection errors in the IDE.
- I looked at all references to "docs/" or "develop/" in our code.
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
## Documentation
<!-- Have you made sure this change doesn't falsify anything currently
stated in `docs/`? If significant
new behavior is added, have you described that in `docs/`? -->
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
## What changed?
<!-- Describe what has changed in this PR -->
Remove buildkite configs and references.
## Why?
<!-- Tell your future self why have you made these changes -->
Buildkite is no longer used.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
Existing tests.
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
No.
## Documentation
<!-- Have you made sure this change doesn't falsify anything currently
stated in `docs/`? If significant
new behavior is added, have you described that in `docs/`? -->
N/A
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
No.
## What changed?
<!-- Describe what has changed in this PR -->
Remove support for standard visibility
## Why?
<!-- Tell your future self why have you made these changes -->
Standard visibility is deprecated.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
## What changed?
<!-- Describe what has changed in this PR -->
- allow configuring a custom OTEL `service.name` prefix
- allow configuring an OTEL exporter via env vars (following the [OTEL
spec](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#configuration-options))
**Note that the OTEL exporter from the environment variables will only
be created if there isn't already one from the config.** Creating an
additional one wouldn't make any sense since the env variables would be
applied to both anyways and there would not be any difference between
them.
## Why?
<!-- Tell your future self why have you made these changes -->
Addresses:
- https://github.com/temporalio/temporal/issues/4042
- https://github.com/temporalio/temporal/issues/4041
- our own (increasing) need to enable OpenTelemetry tracing without
touching the Server config (for ad-hoc debugging when working on a SDK,
for example)
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
I ran an OTEL collector to verify it sends the tracing exports.
These are the env vars I set to make it work:
```go
os.Setenv("OTEL_TRACES_EXPORTER", "oltp")
os.Setenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE", "true")
```
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
I doubt anyone would accidentally have these env variables specified and
turn on OpenTelemetry by accident.
Unless these are specified; no behavior change occurs.
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
No
## What changed?
Now client can request not to append timestamp to scheduled workflow ID.
## Why?
User request.
## How did you test it?
Functional test.
## Potential risks
N/A
## Documentation
To the best of my ability.
## Is hotfix candidate?
No
## What changed?
Update Go SDK to v1.26.0-rc.2
## Why?
Use a tagged Go SDK for the server release
---------
Co-authored-by: Roey Berman <roey@temporal.io>
## What changed?
<!-- Describe what has changed in this PR -->
I updated the docs for the DLQ to include a few more things and made
some structural changes.
## Why?
<!-- Tell your future self why have you made these changes -->
To make the docs more comprehensive and better structured.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
## What changed?
<!-- Describe what has changed in this PR -->
Switch from `docker-compose` to `docker compose`.
## Why?
<!-- Tell your future self why have you made these changes -->
`docker-compose` was deprecated some time ago.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
Run locally.
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
No risks.
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
No.
## What changed?
<!-- Describe what has changed in this PR -->
Update `base-ci-builder` image to 1.10.8.
## Why?
<!-- Tell your future self why have you made these changes -->
It updates `alpine` image to 3.19.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
## What was changed
I bumped the version of api-go and sdk-go and _slightly_ altered our
nettest RPC factory interface to deal with changes in v1.60.0 of go-grpc
## Why?
To fix the protojson DOS vulns recently patched in the upstream
golang/protobuf. See https://github.com/temporalio/api-go/pull/143 for
details
## How did you test it?
I pulled the tests added to the protojson repo into our fork
## Potential risks
N/A
## Is hotfix candidate?
No as it requires all our other proto changes which aren't released
## What changed?
<!-- Describe what has changed in this PR -->
Github workflow to run tests.
I couldn't add tests on `windows-latest` nor `macos-latest` at this moment.
In Windows, there are issues with filepaths in the archiver (eg: `file:///home/` vs `file:///c:\\`, note that in Windows, it needs a forward slash before the actual path `c:\\`, but it messes up with the actual path)
In MacOS, it doesn't have Docker installed by default.
Also, in both Windows and MacOS, some TLS related unit tests are failing.
## Why?
<!-- Tell your future self why have you made these changes -->
Replace buildkite.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? -->
Ran in my fork: https://github.com/rodrigozhou/temporal/actions/runs/7401242980
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this change to production? -->
No.
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) -->
No.
## What changed?
<!-- Describe what has changed in this PR -->
Specify patch version of Cassandra image to use in buildkite.
## Why?
<!-- Tell your future self why have you made these changes -->
It seems that the latest version (3.11.16) has some issues with buildkite (`Cassandra 3.0 and later require Java 8u40 or later.`)
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? -->
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this change to production? -->
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) -->
## What changed?
I replaced the outstandingTasks map with an ordered treemap and
optimized
completeTask to only scan what was necessary to update the ack level.
## Why?
The old implementation of completeTask required a full scan of the task
map in order to move the ack level which had terrible performance.
By storing tasks in an ordered set we can limit the scan's size by
stopping at the first unacked task.
This trades addTask performance for completeTask performance but since
all
added tasks are presumably completed we should be fine with 1/3 the
performance on addTask for 227x the completeTask performance. With this
change both operations run in about the same amount of time.
Before:
```
$ go test -bench=AckManager ./service/matching/... -run=FooBarBaz
goos: darwin
goarch: arm64
pkg: go.temporal.io/server/service/matching
BenchmarkAckManager_AddTask-12 22768 52206 ns/op
BenchmarkAckManager_CompleteTask-12 38 29293019 ns/op
```
After:
```
$ go test -bench=AckManager ./service/matching -run=FooBarBaz
goos: darwin
goarch: arm64
pkg: go.temporal.io/server/service/matching
BenchmarkAckManager_AddTask-12 8127 147226 ns/op
BenchmarkAckManager_CompleteTask-12 8626 136614 ns/op
```
## How did you test it?
I added both tests and benchmarks to ensure the ackManager worked as
before
## Potential risks
None.
## Is hotfix candidate?
No
## What changed?
All `tests/*_test.go` were renamed to `tests/*go`, their `Test*Suite`
functions were moved out to `tests/all_test.go`.
## Why?
Following up on #5192 and #5188 — making test suites available to
third-party packages.
## How did you test it?
Ran tests.
## Potential risks
Merge conflicts in other PRs.
## Is hotfix candidate?
No.
<!-- Describe what has changed in this PR -->
**What changed?**
Adding documentation on how to deal with messages that are added to
DLQs.
<!-- Tell your future self why have you made these changes -->
**Why?**
This will help operators to find messages in DLQ, the reason why it got
enqueued into DLQ etc.
It also has useful tdbg commands for deleting or re-enqueuing the
messages from DLQ
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
**What changed?**
gogo/protobuf has been replaced with Google's official go compiler.
**Why?**
gogo/protobuf has been deprecated for some time and the community is
moving on, building new tools (like vtproto) atop google's v2 compiler.
**How did you test it?**
`make test`
**Potential risks**
1. The change from embedded gogo-generated-structs to
google-generated-pointers-to-structs created a risk of nil pointer
exceptions. I've fixed all the ones our tests found but it's possible
there are more lurking in the new code.
2. This change may cause our performance to decrease. Certainly
encoding/deconding of proto objects will become slower, but the overuse
of pointers by the google compiler may negatively affect our overall
performance. We'll need to keep an eye on the GC stats
3. This breaks the HTTP API. We will not support [shortand payload
encoding](https://github.com/temporalio/proposals/blob/master/api/http-api.md#payload-formatting)
in this first pass; that will come once this initial work is in testing.
**Breaking changes for developers**
- `*time.Time` in proto structs will now be
[timestamppb.Timestamp](https://pkg.go.dev/google.golang.org/protobuf@v1.31.0/types/known/timestamppb#section-documentation)
- `*time.Duration` will now be
[durationpb.Duration](https://pkg.go.dev/google.golang.org/protobuf/types/known/durationpb)
- V2-generated structs embed locks, so you cannot dereference them. `go
vet` will scream at you about this. If you need a copy, use
`proto.Clone`.
- If the performance of this sucks then I will either update our code
generator to add shallow-clone methods or hand-roll the ones we need
- Proto enums will, when formatted to JSON, now be in
`SCREAMING_SNAKE_CASE` rather than `PascalCase`. We decided (in
discussion with the SDK team) that now was as good a time as any to rip
the bandage off.
- Proto objects, or objects embedding protos, cannot be compared using
`reflect.DeepEqual` or _anything_ that uses it. This includes `testify`
and `mock` equality testers!
- You will need to use the `common/testing/protorequire`,
`common/testing/protoassert`, or `common/testing/protomock` packages
instead. I've implemented proto-compatible matchers and assertions there
for all cases I've encountered
- If you need `reflect.DeepEqual` for any reason you can use
`go.temporal.io/api/temporalproto.DeepEqual` instead
Note that history loading will not be impacted by the JSON changes: I
rewrote history loading to dynamically fix incoming history JSON data
(like all our other sdks); you can find this code in [my fork of our go
API](https://github.com/tdeebswihart/temporal-api-go/blob/master/internal/temporalhistoryv1/load.go)
alongside its tests.
**🚨Sharp Edges Introduced🚨**
Beware `*timestamppb.Timestamp.AsTime()`. If you need to extract a time
value from a proto time (timestamppb) **always** make sure to check
whether it's nil first. When the proto object is `nil` `AsTime()` will
return a non-zero time at the proto epoch: UTC midnight on January 1,
1970.
I've made this mistake multiple times during this transition and each
time it's been a pain to debug
**Is hotfix candidate?**
No.
## What changed?
Makes changes summarized in
https://github.com/temporalio/temporal/assets/52205/0660a021-588a-4573-94d3-301cae0c0040
### Empty responses on server-timeout
The behavior of `PollWorkflowExecutionUpdate` and
`UpdateWorkflowExecution` was changed as follows:
- They introduce a "server-imposed deadline" equal to current time plus
`LongPollExpirationInterval`
- If a timeout occurs due to the server-imposed deadline, then they do
not return an error, but return a response with an empty outcome.
- Otherwise, their behavior is unchanged (in particular, if a timeout
occurs due context deadline expiry, they return the error as before)
### Non-blocking poll
- Change the Update Poll endpoint so that omitting the `WaitPolicy` now
means "do not wait; respond with current Update status without
blocking". Previously omitting the `WaitPolicy` was interpreted to mean
"attempt to wait until Completed"
- Also return the `UpdateRef`
## Why?
### Empty responses on server-timeout
- This makes the behavior of the endoint consistent with
`GetWorkflowExecutionHistory`.
- This makes sense, because that endpoint is used in an analogous way
(e.g. `GetWorkflowExecutionHistory` is used for `wfHandle.result()` and
`PollWorkflowExecutionUpdateRequest` is used for
`updateHandle.result()`, and both involve polling for the result).
- In particular, a polling client interprets
timeout-on-server-imposed-deadline to mean that it should continue
polling, and we would prefer not to use an error to communicate this
non-exceptional situation.
### Non-blocking poll
SDK clients may use non-blocking poll to, for example
- Obtain current status of an Update (perhaps `updateHandle.describe()`
- Determine whether an Update with a given ID already exists / is
running
### How did you test it?
- Unit tests
- Modified Typescript SDK to not send `WaitPolicy`, ran a feature test
against the modified server and modified SDK, and confirmed API
communication as expected (this revealed that I needed to stop Frontend
mutating the request to set `WaitPolicy` to `Completed`).
- I changed the poll loop in sdk-typescript to no longer catch
`DeadlineExceeded`, ran the `update/basic_async` feature test against
the local SDK and against server binaries built prior to and with this
commit, and confirmed that the change in this PR fixed the poll loop.
### Potential risks
- Could break Update operations in SDKs
### Is hotfix candidate?
- No
<!-- Describe what has changed in this PR -->
**What changed?**
Replace xwb1989/sqlparser with forked temporalio/sqlparser
<!-- Tell your future self why have you made these changes -->
**Why?**
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Unit tests
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
No.
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No.
<!-- Describe what has changed in this PR -->
**What changed?**
I upgraded our zap version from v1.24.0 to v1.26.0, which contains
support for pkg/errors. See [this
issue](https://github.com/uber-go/zap/issues/303) and [this
commit](5fc2db7f38).
<img width="448" alt="image"
src="https://github.com/temporalio/temporal/assets/5942963/7d15d91a-27d3-45f6-9628-30bdcac38771">
<!-- Tell your future self why have you made these changes -->
**Why?**
Before this change, our logs would only contain the stack trace from
where the logger itself was invoked, not from the source of where the
error was generated or wrapped. This provided very little useful
information.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
I ran a custom [build of
server](https://gist.github.com/MichaelSnowden/c649dfd1efeb92f10bc72a040a792a8d)
which overwrote some deep code in history to return an error. I then set
up docker-compose to output logs -> promtail -> loki -> grafana. Then, I
queried Grafana to verify that the error log contained an "errorVerbose"
field with the stack trace from where my error was generated. As you can
see from the below image, the stack trace does appear under this field,
and if you turn on JSON parsing and newline escaping, you can both see
it rendered correctly, and you can copy-paste the stack trace.
<img width="983" alt="image"
src="https://github.com/temporalio/temporal/assets/5942963/cb9b0c83-b146-4060-9eac-3ccf9b807657">
<img width="683" alt="image"
src="https://github.com/temporalio/temporal/assets/5942963/6fa28f2f-5d04-47ae-b8a3-7512c3dd85e7">
The stack trace from Grafana:
```
oopsie woopsie
main.(*faultyShardEngine).StartWorkflowExecution
/Users/mikey/src/temporalio/temporal/.scratches/main.go:39
go.temporal.io/server/service/history.(*Handler).StartWorkflowExecution
/Users/mikey/src/temporalio/temporal/service/history/handler.go:595
go.temporal.io/server/api/historyservice/v1._HistoryService_StartWorkflowExecution_Handler.func1
/Users/mikey/src/temporalio/temporal/api/historyservice/v1/service.pb.go:1300
...
```
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
The stack traces are pretty deep because of all our gRPC interceptors.
However, we can definitely fix that later if we want by filtering the
`pkg/errors.StackTrace`. I'd rather do that in a follow-up after getting
support for this initial change first, though.
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No.
<!-- Describe what has changed in this PR -->
**What changed?**
Allow rate limiting a batch operation. Fixes
https://github.com/temporalio/temporal/issues/4926.
<!-- Tell your future self why have you made these changes -->
**Why?**
Batch operations are run server side and may effect millions of
executions, this in turn may overload workers and disrupt normal
operations.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
I started the Server locally and initiated a batch operation from the
CLI ([CLI changes be found
here](https://github.com/temporalio/cli/pull/366)):
- [x] uses provided limit
- [x] uses server limit when not provided
- [x] caps it at server limit
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
Rate limiting to be incorrect and slow down processing or disrupt
cluster.
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No
**What changed?**
- In CI, build most of the tests and dependencies and reuse them in
subsequent steps.
- Additionally pre-build dependencies separately on a weekly schedule
(in buildkite configuration) and reuse them when possible.
- Add more test sharding for functional tests (split the main functional
test suite into three).
**Why?**
Increase parallelism, reduce latency of getting test results, reduce
granularity of retries
**How did you test it?**
lots of testing on buildkite
**Potential risks**
**Is hotfix candidate?**
<!-- Describe what has changed in this PR -->
**What changed?**
Using `gotestsum` - [as suggested by
Buildkite](https://buildkite.com/docs/test-analytics/golang-collectors)
- and uploading the artifacts to their Analytics product.
[Results will be available
here](https://buildkite.com/organizations/temporal/analytics/suites/temporal-public?branch=all+branches).
<!-- Tell your future self why have you made these changes -->
**Why?**
We want to have better insight into which tests are flaky and/or slow.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Running this branch in Buildkite.
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
None?
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No.
<!-- Describe what has changed in this PR -->
**What changed?**
`Driver` interface (with two implementations) was added to the
postgresql persistence package in order to support other drivers than
`lib/pq`. Fixes https://github.com/temporalio/temporal/issues/1775
Essentially, this PR does
https://github.com/temporalio/temporal/pull/4914 in a
backward-compatible and extendable way
<!-- Tell your future self why have you made these changes -->
**Why?**
Since `lib/pq` does not support multi-node clusters
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Existing tests
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
No, since you should explicitly change the driver in your config file.
However, `connectAttributes` depend on the driver (e.g.,
`binary_parameters` is supported only by `lib/pq` and is not a part of
postresql standard), so one should be careful changing drivers in
production.
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No
