serde2: implement awsJson10 by lucix-aws · Pull Request #633 · aws/smithy-go

lucix-aws · 2026-03-04T16:50:10Z

tl;dr

new interfaces for schema-serde in protocols.go
implement ShapeSerializer/ShapeDeserializer for JSON
new module: aws-protocols, which is where all of the existing sdk-side protocol support will go (but in the runtime!)
I had to change up how schemas were generated, because they can reference each other recursively (e.g. dynamodb). So now, the schemas all get created, and then in an init block the members get initialized. I also unexported all of the fields on schema and switched to New- style creation since I think we want to preserve the internals here and just evolve public APIs on schema
- I am not exactly sure what I did, but the size of sqs schemas went from 70kb to 40, so that's cool

documents

There is some trickery with documents here. Basically we would like to support new documents that can deserialize to and from shapes, but we are locked in with the existing public APIs (per-service document.Interface). This supports the "old" way, where you pass any value into document.NewLazyDocument in the input, and it will reflection over that value and turn it into json.

The shape ser/deser interfaces accept a new sealed document.Value, which supports this old mechanism through the variant document.Opaque, but it will also support the new stuff which I've hinted at by filling out the other variants (e.g. document.Structure). Work to support that would be in a future PR.

performance notes

The JSON deserializer should be way more performant than what we had before, since we were deserializing the entire thing into a map[string]any and then re-copying that into the target output. Benchmarks pending.

The serializer is probably a slight regression because of how we have to wrap to the old encoder but there's a way out of this. We will basically just need to drop the old encoder and roll a new one that's better tailored to how the ShapeSerializer APIs work.

testing

I have regenerated the jsonrpc10 protocol tests downstream with this and they all pass. It wouldn't be possible to get that working in CI atm with how we have this new module though.

lucix-aws · 2026-03-04T17:16:05Z

relates #458

Madrigal

No major comments

Madrigal · 2026-03-04T19:58:41Z

aws-protocols/awsjson10/awsjson10.go

+	return nil
+}
+
+// TODO get the intermediate reader out of this thing and just operate on the


Don't we still need access to the headers?

I don't even understand this reading it back, the implementation looks fine to me, I just removed it.

serde.go

Madrigal · 2026-03-04T20:11:24Z

...ithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/serde2/ListDeserializer.java

+                    goTemplate("d.ReadDocument(s.ListMember(), &vv)");
+
+            case BIG_INTEGER, BIG_DECIMAL ->
+                    throw new CodegenException("big integer / big decimal unsupported");


has this always been unsupported?

Madrigal · 2026-03-04T20:12:47Z

...smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/serde2/ListSerializer.java

+        ));
+    }
+
+    // TODO sparse


what do you mean here?

🤔 did the protocol tests literally not have sparse collections? i'll have to check

evidently i put this here and then completely forgot about sparse lists maps, so that needs to be addressed here, the protocol tests just don't have them to catch this i guess

Madrigal · 2026-03-04T20:28:21Z

codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoCodegenContext.java

+        var operations = TopDownIndex.of(model).getContainedOperations(service());
+
+        var shapes = new HashSet<Shape>();
+        shapes.addAll(operations.stream()


hate to see this being walked thrice, but I don't know if there's a better way

Madrigal · 2026-03-04T20:28:59Z

codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoCodegenContext.java

+                .collect(toSet()));
+
+        return shapes.stream()
+                .sorted()


how are they sorted naturally, by type+name?

I'm not sure but when we do the shape walking they go into a set to not get duplicated, which ruins whatever order was there originally.

Madrigal · 2026-03-04T20:32:38Z

...hy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/ProtocolDocumentGenerator.java

+        writeInternalDocumentPackage("document.go", writer -> {
+            Symbol marshalerSymbol = getInternalDocumentSymbol("documentMarshaler",
+                    true);
+            Symbol unmarshalerSymbol = getInternalDocumentSymbol("documentUnmarshaler", true);


unrelated, but really hate these get(thing, true) signatures

Madrigal · 2026-03-04T20:33:55Z

codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/UnionGenerator.java

+                case BIG_DECIMAL -> writer.write("s.WriteBigDecimal($L, v.Value)", schemaName);
+                case STRUCTURE -> writer.write("s.WriteStruct($L, &v.Value)", schemaName); // struct variants are value types
+                case LIST, SET, MAP -> writer.write("serialize$L(s, $L, v.Value)", target.getId().getName(), schemaName);
+                default -> writer.write("// TODO: serialize union member type $L", target.getType());


didn't we already had the code for union types?

yes, this was an old thing for when i was adding in stuff here. replaced it with a throw on the remaining stuff

Madrigal · 2026-03-04T20:40:34Z

codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/CodegenVisitor.java

+
+                writer.write("");
+                writer.writeDocs("Initialize schema members after all schemas are declared to avoid initialization cycles");
+                writer.openBlock("func init() {", "}", () -> {


This is the first time we have init on generated clients, right? Don't think there's any negative about it, just want to clarify it

I am sort of concerned about this actually, i would like to benchmark how long we're actually in this, because it could affect binary startups (or like lambda coldstarts etc).

There is a route where we lazy-load schemas but I'm not sure all the sync.Once we'd have to add for that is worth the tradeoff.

I'd be curious to followup on this. Even if we do sync.Once we would just shuffle when we execute this, but maybe you have something else in mind

Co-authored-by: Luis Madrigal <599908+Madrigal@users.noreply.github.com>

* implement awsjson10 minus event streams * forgot about sparse * Update serde.go Co-authored-by: Luis Madrigal <599908+Madrigal@users.noreply.github.com> * Update serde.go Co-authored-by: Luis Madrigal <599908+Madrigal@users.noreply.github.com> * idk what this was about * drop the old todo stuff --------- Co-authored-by: Luis Madrigal <599908+Madrigal@users.noreply.github.com>

lucix-aws requested review from a team as code owners March 4, 2026 16:50

lucix-aws changed the base branch from main to feat-serde2 March 4, 2026 16:50

lucix-aws force-pushed the feat-serde2-jsonrpc branch from ef07899 to 799e7a1 Compare March 4, 2026 17:06

implement awsjson10 minus event streams

cd2a45f

lucix-aws force-pushed the feat-serde2-jsonrpc branch from 799e7a1 to cd2a45f Compare March 4, 2026 17:15

lucix-aws mentioned this pull request Mar 4, 2026

Move protocol support out of aws-sdk-go-v2 into smithy-go #458

Open

Madrigal reviewed Mar 4, 2026

View reviewed changes

lucix-aws and others added 5 commits March 4, 2026 20:09

forgot about sparse

080c2d1

Update serde.go

c2cbd82

Co-authored-by: Luis Madrigal <599908+Madrigal@users.noreply.github.com>

Update serde.go

6c4af55

Co-authored-by: Luis Madrigal <599908+Madrigal@users.noreply.github.com>

idk what this was about

fa0c06c

drop the old todo stuff

1ede910

Madrigal approved these changes Mar 5, 2026

View reviewed changes

lucix-aws merged commit 86f780a into feat-serde2 Mar 5, 2026
1 check passed

lucix-aws deleted the feat-serde2-jsonrpc branch March 5, 2026 18:09

Conversation

lucix-aws commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

tl;dr

documents

performance notes

testing

Uh oh!

lucix-aws commented Mar 4, 2026

Uh oh!

Madrigal left a comment

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!

Uh oh!

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!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lucix-aws commented Mar 4, 2026 •

edited

Loading