:boom: Use ExternalStorageReference protobuf message for payload references by jmaeagle99 · Pull Request #2311 · temporalio/sdk-go

jmaeagle99 · 2026-04-26T22:50:40Z

What was changed

Store and retrieve payload references as ExternalStorageReference protobuf message encoded with protojson.
Allow backward compatible retrieval of prerelease plain json encoding.
Embed ExternalStorageReference proto message definition in sdk-go until api-go contains the same definition. Generate structure and data such that wire format and data are exactly the same as would be in api-go.

Why?

Standardize on protobuf message for encoding storage references.

Checklist

How was this tested: Existing tests, compat tests, and new tests
Any docs updates needed? No

yuandrew · 2026-04-28T17:39:19Z

For reference, this is the API PR: temporalio/api#772

yuandrew · 2026-04-28T17:44:17Z

-type storageReference struct {
+// legacyStorageReference is the old wire format retained for backward compatibility
+// with payloads written by earlier prerelease SDK versions.
+type legacyStorageReference struct {


Why do we want/need to maintain the earlier prerelease SDK version? Since it's still in prerelease, should we just make the breaking change here, so we only need to support one format?

I guess we maybe don't want to completely break previous workflows, but do we plan on deprecating this legacy stuff before we hit public preview/GA?

We don't want to immediately break anyone who used the prerelease bits. We'll remove before GA.

yuandrew · 2026-04-28T17:46:06Z

-// payloads that are storage references rather than actual data.
-const metadataEncodingStorageRef = "json/external-storage-reference"
+// metadataMessageType is the key used in payload metadata to identify the proto
+// message type. Mirrors converter.MetadataMessageType without importing converter package.


nit: Do we want to add a mirroring comment to converter.MetadataMessageType that it matches this new value? Same question for metadataEncodingProtoJSON

yuandrew · 2026-04-28T17:48:10Z

+	]
+}`
+
+	externalStorageReferenceMessageType = string((*sdkpb.ExternalStorageReference)(nil).ProtoReflect().Descriptor().FullName())


Is this line needed? Don't we already set this value in internal_extstore.go?

yuandrew · 2026-04-28T17:51:17Z

-// IsStorageReference reports whether p is an external-storage reference payload
-// (i.e. its encoding metadata equals the internal storage-reference marker).
+// IsStorageReference reports whether p is an external-storage reference payload.
+// It recognises both the current protojson format (encoding=json/protobuf,


nit: recognises is the british form of recognizes? Not sure we use British spelling elsewhere in the SDK. I know we use Canadian forms of words (i.e. Behaviour), but not aware of any british spelling, haha

Suggested change

// It recognises both the current protojson format (encoding=json/protobuf,

// It recognizes both the current protojson format (encoding=json/protobuf,

yuandrew · 2026-04-28T18:03:06Z

@@ -0,0 +1,39 @@
+# internal/temporalapi


Any reason we need this over waiting for an API release to ingest the changes from there?

yuandrew · 2026-04-28T18:20:10Z

+
+// metadataEncodingProtoJSON is the standard protojson encoding value, shared with
+// ProtoJSONPayloadConverter. Mirrors converter.MetadataEncodingProtoJSON.
+const metadataEncodingProtoJSON = "json/protobuf"


technically this is breaking if we use an old SDK to read the new deployment format for a rolling deployment. A safer pattern would be to introduce the new format but continue to use the old, then a version later switch to the new format by default. But I assume we don't have time for this multi-release rollout. I'm fine without it, but not sure if we want to add a 💥 . It's a bit of an edge case

Added boom to title

yuandrew · 2026-04-28T18:25:39Z

+	externalStorageReferenceMessageType = string((*sdkpb.ExternalStorageReference)(nil).ProtoReflect().Descriptor().FullName())
+
+	protoMarshalOptions   = protojson.MarshalOptions{}
+	protoUnmarshalOptions = protojson.UnmarshalOptions{}


Do we want to have DiscardUnknown: true? For if an older SDK version tries to read a payload from a new SDK that added a new field to ExternalStorageReference

https://pkg.go.dev/google.golang.org/protobuf/encoding/protojson#UnmarshalOptions

Wouldn't we have had to add that option to the plain JSON converter in the old SDK though to work with the scenario that you outlined?

yuandrew · 2026-04-28T18:28:51Z

+// TestClaimDeserialization_OtherSdk_ProtoJson verifies that a full proto-JSON
+// payload produced by another language SDK (e.g. Python) is correctly parsed by
+// the Go SDK's payloadToStorageReference function.
+func TestClaimDeserialization_OtherSdk_ProtoJson(t *testing.T) {


JSON in ProtoJson should be all caps, same with PlainJson

jmaeagle99 force-pushed the storage-ref-proto branch from 0d127c6 to f9977e3 Compare April 27, 2026 20:00

Replace storageReference with ExternalStorageReference proto

bb13a69

jmaeagle99 force-pushed the storage-ref-proto branch from f9977e3 to bb13a69 Compare April 28, 2026 16:35

jmaeagle99 marked this pull request as ready for review April 28, 2026 16:37

jmaeagle99 requested a review from a team as a code owner April 28, 2026 16:37

yuandrew reviewed Apr 28, 2026

View reviewed changes

jmaeagle99 changed the title ~~Use ExternalStorageReference protobuf message for payload references~~ 💥 Use ExternalStorageReference protobuf message for payload references Apr 28, 2026

	// It recognises both the current protojson format (encoding=json/protobuf,
	// It recognizes both the current protojson format (encoding=json/protobuf,

Conversation

jmaeagle99 commented Apr 26, 2026

What was changed

Why?

Checklist

Uh oh!

yuandrew commented Apr 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants