refactor: replace proto serialization with JSON and migrate callsites by yushan8 · Pull Request #222 · uber/tango

yushan8 · 2026-07-20T03:28:30Z

Summary

Intent: Proto definitions are currently leaking into the storage. We should Put the actual entities to be encoded/decoded from the storage, not the proto definitions.

Remove all proto imports from core/storage/ — serialize entity structs directly via encoding/json
Move write functions from reader files into graphwriter.go
Controller reads entity.GetTargetGraphResponse and entity.GetChangedTargetsResponse internally, converting to proto via mapper only at stream.Send
Changed-targets pipeline fully migrated to entity types
Orchestrator uses mapper.ResultToTargetGraph + SplitBySize
Consolidate service.chunking.* into a single max_message_bytes config field
Remove old chunking code from core/common
Update query-bench example

Test plan

grep -r "tangopb\|gogo/protobuf" core/storage/ returns zero results
make build && make test — all 19 test targets pass
make gazelle — BUILD files in sync

Stack

feat(entity): add entity types for target graph and changed targets #210 (entity types)
feat(mapper): add entity-proto mappers #211 (mappers + streaming)
refactor: replace proto serialization with JSON and migrate callsites #222 — this PR (storage JSON + callsite migration + cleanup)

Ran integration tests and make bench to make sure storage is compatible with current changes.

…210) ## Summary - Add `entity.OptimizedTarget`, `entity.Metadata`, and `entity.GetTargetGraphResponse` as compact, proto-free representations of the target graph wire types - Add `entity.ChangedTarget` and `entity.GetChangedTargetsResponse` for the changed-targets path - These entity types mirror the proto definitions but live in a dependency-free package, preventing proto from leaking into storage, orchestrator, and other non-transport layers ## Test plan - [x] `make build && make test && make gazelle` ## Stack 1. **#210 — this PR** (entity types) 2. #211 (mappers + streaming) 3. #222 (storage JSON rewrite) 4. #212 (callsite migration + cleanup) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

xytan0056 · 2026-07-21T22:34:26Z

 		totalDuration += elapsed
 		fmt.Printf("run %d: targethasher: %v  (%d targets)\n", i+1, elapsed.Round(time.Millisecond), len(targethasherResult.TargetNames))
 		start = time.Now()
-		response, err := common.ResultToGetTargetGraphResponse(


reminder to remove dead code in common, now that a bunch of functions in common are not used

yes, common is completely removed in this PR.

xytan0056 · 2026-07-21T22:45:55Z

@@ -1,4 +1,4 @@
-// Copyright (c) 2026 Uber Technologies, Inc.
+// Copyright (c) 2025 Uber Technologies, Inc.


xytan0056 · 2026-07-21T22:46:58Z

-		isEmpty: isEmpty,
+	return &reader[T]{
+		rc:  resp.ReadCloser,
+		dec: json.NewDecoder(resp.ReadCloser),


could be backwards incompatible here, the old cache is still proto based. Would that be an accepted risk?

we can bump the storage version, so new instances of the service will write/read from the json entity, not the proto object.

## Summary Adds entity-proto mapping functions for the controller and orchestrator layers: - `internal/mapper/` — proto↔entity converters (`GetTargetGraphResponseToProto`, `ChangedTargetsResponseToProto`, `ProtoToGetTargetGraphRequest`, `ProtoToBuildDescription`, `ToProtoError`) - `mapper/` — top-level `ResultToTargetGraph` for converting `targethasher.Result` into ID-mapped entity types (importable by external consumers) ## Stack 1. #226 (streaming utilities) 2. **This PR** (entity-proto mappers) 3. #222 (storage JSON + entity migration) ## Test plan - [x] `make build && make test && make gazelle` Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

- Export DefaultMaxMessageBytes in config package and use it from controller instead of hardcoded value - Update copyright year to 2026 in core/storage/reader.go - Add Size() to entity.ChangedTarget and inline size-based splitting in compareTargetGraphs, eliminating the wasteful proto conversion that was only used for size measurement - Add ProtoToChangedTargetsResponse mapper for cache read boundary Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

- Update copyright year to 2026 in changedtargetsreader.go - Remove dead code from core/common/utils.go (ResultToGetTargetGraphResponse, ChunkMetadata, chunkTargets, splitMap, and chunk size constants) - Fix defer syntax in compareTargetGraphs - Remove unused common import from orchestrator - Update entity/changed_targets.go to use wire.VarintSize Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

liujsj · 2026-07-22T01:56:19Z

-		rc:      gogio.NewDelimitedReader(resp.ReadCloser, maxMessageSize),
-		isEmpty: isEmpty,
+	return &reader[T]{
+		rc:  resp.ReadCloser,


Prior we were calling the messages to a max size. What was the reason to remove it?

https://pkg.go.dev/github.com/gogo/protobuf/io#NewDelimitedReader protobuf reader requires a MaxSize input. json.NewDecoder doesn't need a max-size parameter.
Since we are already splitting the entities by bytes when writing to the storage, every Read() call will read at most max_message_bytes from the storage.

Remove all proto from core/storage, replacing with encoding/json. Migrate controller and orchestrator to entity types, converting to proto only at the gRPC boundary. Consolidate chunking config into max_message_bytes. Remove old chunking code from core/common. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…ation Update toChangeType to return entity.ChangeType, mapper to cast via int32, and test assertions to use entity constants. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Run gazelle to remove stale //config and //core/common deps from controller BUILD.bazel. Replace context.Background() with t.Context() in test helper. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Remove _defaultMaxMessageBytes constant and inline the value with a TODO to provide it via config.Parse. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

- Export DefaultMaxMessageBytes in config package and use it from controller instead of hardcoded value - Update copyright year to 2026 in core/storage/reader.go - Add Size() to entity.ChangedTarget and inline size-based splitting in compareTargetGraphs, eliminating the wasteful proto conversion that was only used for size measurement - Add ProtoToChangedTargetsResponse mapper for cache read boundary Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

- Update copyright year to 2026 in changedtargetsreader.go - Remove dead code from core/common/utils.go (ResultToGetTargetGraphResponse, ChunkMetadata, chunkTargets, splitMap, and chunk size constants) - Fix defer syntax in compareTargetGraphs - Remove unused common import from orchestrator - Update entity/changed_targets.go to use wire.VarintSize Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…efactor Restore explanatory comments in linkRequestCtx that were incidentally deleted while migrating to the max_message_bytes config field.

Add streaming.SplitChangedTargets, mirroring SplitTargetGraph, and use it in getchangedtargets instead of an inlined split loop.

…argets Switch OptimizedTarget.Size() and ChangedTarget.Size() to value receivers so []OptimizedTarget and []ChangedTarget satisfy Sizer, then have SplitTargetGraph and SplitChangedTargets call the shared SplitBySize for item-grouping instead of duplicating the same loop.

…ites Both target-graph and changed-targets response chunking need the same two independent things: split items by size (SplitBySize) and split metadata by size (SplitMetadata), then wrap each group into the caller's own response type. Rather than a combinator function that blurs those two concerns together, each of the three callers (native orchestrator, query-bench example, controller) composes SplitBySize and SplitMetadata directly and does its own wrapping — so SplitBySize now handles OptimizedTarget and ChangedTarget the same way, and SplitMetadata does only what its name says.

yushan8 requested review from a team as code owners July 20, 2026 03:28

This was referenced Jul 20, 2026

feat(entity): add entity types for target graph and changed targets #210

Merged

feat(mapper): add entity-proto mappers #211

Merged

refactor: migrate callsites to entity types and consolidate config #212

Closed

yushan8 force-pushed the yushan/streaming-and-mappers branch from 6fc3266 to 0f5a89c Compare July 20, 2026 03:42

yushan8 force-pushed the yushan/storage-json branch 2 times, most recently from 1a54757 to 2252f29 Compare July 20, 2026 03:46

yushan8 force-pushed the yushan/streaming-and-mappers branch 2 times, most recently from 5fb83ae to 7f55e85 Compare July 20, 2026 04:52

yushan8 force-pushed the yushan/storage-json branch 2 times, most recently from 8677311 to 614cddf Compare July 20, 2026 05:27

yushan8 force-pushed the yushan/streaming-and-mappers branch from 7f55e85 to e762db8 Compare July 20, 2026 05:27

yushan8 changed the title ~~refactor(storage): replace proto serialization with JSON entity encoding~~ refactor: replace proto serialization with JSON and migrate callsites Jul 20, 2026

yushan8 force-pushed the yushan/streaming-and-mappers branch 2 times, most recently from dff47a4 to 9c060bc Compare July 20, 2026 17:17

yushan8 force-pushed the yushan/storage-json branch from 7ec0e66 to 2870453 Compare July 20, 2026 17:27

yushan8 mentioned this pull request Jul 20, 2026

feat(mapper): add entity-proto mappers #224

Merged

1 task

yushan8 force-pushed the yushan/streaming-and-mappers branch from 9c060bc to 63d2138 Compare July 20, 2026 18:21

yushan8 force-pushed the yushan/storage-json branch 2 times, most recently from 140a936 to 1f1b327 Compare July 20, 2026 18:33

yushan8 force-pushed the yushan/streaming-and-mappers branch from 63d2138 to 7b87081 Compare July 20, 2026 18:41

yushan8 force-pushed the yushan/storage-json branch from 1f1b327 to 6d0f3a7 Compare July 20, 2026 18:42

yushan8 force-pushed the yushan/streaming-and-mappers branch from 7b87081 to 23bf3ac Compare July 20, 2026 19:32

yushan8 force-pushed the yushan/storage-json branch 3 times, most recently from e57a2e1 to 1f8366f Compare July 20, 2026 21:47

yushan8 force-pushed the yushan/streaming-and-mappers branch from e762709 to 8f360d1 Compare July 20, 2026 22:09

yushan8 force-pushed the yushan/storage-json branch from 1f8366f to e9517ce Compare July 20, 2026 22:13

xytan0056 reviewed Jul 21, 2026

View reviewed changes

Base automatically changed from yushan/streaming-split to main July 22, 2026 00:18

yushan8 force-pushed the yushan/storage-json branch from 4860e6e to 2010498 Compare July 22, 2026 01:14

xytan0056 approved these changes Jul 22, 2026

View reviewed changes

liujsj reviewed Jul 22, 2026

View reviewed changes

Comment thread controller/getchangedtargets.go Outdated

liujsj reviewed Jul 22, 2026

View reviewed changes

Comment thread controller/controller.go

liujsj reviewed Jul 22, 2026

View reviewed changes

yushan8 force-pushed the yushan/storage-json branch 2 times, most recently from 806404a to 90e1e60 Compare July 22, 2026 20:30

yushan8 marked this pull request as ready for review July 22, 2026 21:59

yushan8 marked this pull request as draft July 22, 2026 22:06

yushan8 and others added 9 commits July 22, 2026 15:33

refactor: use entity.ChangeType instead of int32 for change classific…

06c9261

…ation Update toChangeType to return entity.ChangeType, mapper to cast via int32, and test assertions to use entity constants. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

fix(controller): update BUILD deps and use t.Context() in tests

4fb7f45

Run gazelle to remove stale //config and //core/common deps from controller BUILD.bazel. Replace context.Background() with t.Context() in test helper. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

refactor(controller): inline default max message bytes

4f13471

Remove _defaultMaxMessageBytes constant and inline the value with a TODO to provide it via config.Parse. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

refactor(controller): restore comments dropped in max_message_bytes r…

44e2234

…efactor Restore explanatory comments in linkRequestCtx that were incidentally deleted while migrating to the max_message_bytes config field.

refactor(controller): move changed-targets chunking into streaming

8bbcba7

Add streaming.SplitChangedTargets, mirroring SplitTargetGraph, and use it in getchangedtargets instead of an inlined split loop.

yushan8 force-pushed the yushan/storage-json branch from 90e1e60 to 5589819 Compare July 22, 2026 22:38

yushan8 marked this pull request as ready for review July 22, 2026 22:39

yushan8 force-pushed the yushan/storage-json branch from 5589819 to 02f6a52 Compare July 22, 2026 22:58

yushan8 force-pushed the yushan/storage-json branch from 02f6a52 to fb4b255 Compare July 22, 2026 23:04

yushan8 merged commit 4d4de3d into main Jul 22, 2026
10 checks passed

yushan8 deleted the yushan/storage-json branch July 22, 2026 23:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: replace proto serialization with JSON and migrate callsites#222

refactor: replace proto serialization with JSON and migrate callsites#222
yushan8 merged 10 commits into
mainfrom
yushan/storage-json

yushan8 commented Jul 20, 2026 •

edited

Loading

Uh oh!

xytan0056 Jul 21, 2026

Uh oh!

yushan8 Jul 22, 2026

Uh oh!

xytan0056 Jul 21, 2026

Uh oh!

xytan0056 Jul 21, 2026

Uh oh!

yushan8 Jul 22, 2026

Uh oh!

Uh oh!

Uh oh!

liujsj Jul 22, 2026

Uh oh!

yushan8 Jul 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -1,4 +1,4 @@
		// Copyright (c) 2026 Uber Technologies, Inc.
		// Copyright (c) 2025 Uber Technologies, Inc.

Uh oh!

Conversation

yushan8 commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Stack

Uh oh!

xytan0056 Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

yushan8 Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

xytan0056 Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

xytan0056 Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

yushan8 Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

liujsj Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

yushan8 Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yushan8 commented Jul 20, 2026 •

edited

Loading