config-remote-sync: record which state the run read in telemetry - #6120
Merged
Conversation
Co-authored-by: Isaac
Collaborator
Integration test reportCommit: 5f537fb
8 interesting tests: 4 SKIP, 3 KNOWN, 1 RECOVERED
Top 4 slowest tests (at least 2 minutes):
|
Contributor
Author
|
The |
shreyas-goenka
approved these changes
Aug 4, 2026
Collaborator
Integration test reportCommit: 72db775
389 interesting tests: 352 MISS, 32 FAIL, 4 RECOVERED, 1 SKIP
Top 50 slowest tests (at least 2 minutes):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
bundle_config_remote_sync_eventrecords what a run found but nothing about which resource state it read. Adds:state_serialstate_lineageuuid.New())state_source"local"or"remote"states_available_count0= ran against a synthesized empty statestate_resource_idsselected_resource_ids--select-idsThe two id fields share one message (
BundleConfigRemoteSyncResourceIds) so they can be compared directly. Field names, comments, per-type data labels, and the 1000-per-type cap are copied from the siblingbundle deploycommand — seelibs/telemetry/protos/bundle_deploy.go:27-33andphases.ResourceIdLimit(bundle/phases/deploy.go:327, which exists because the telemetry upload has a short timeout).No separate selector/matched counters: those are the lengths of the id lists, and the matched set is their intersection.
states_available_countis kept because it is not derivable — a state file holding zero resources and no state file at all both yield an empty id set.IndexDeployedResourcesis extracted fromResolveResourceSelectorsso the telemetry reuses the same index rather than duplicating the id-parsing logic.Why
no deployed <type> resource with id <id>currently has several indistinguishable causes, each with a different fix:states_available_count: 0PullResourcesStatepicks the highest serial, andAlwaysPulldoes not overwrite a local winner) →state_source: "local"with a highstate_serialstate_lineagediffers from the deploy's, andstate_resource_idsholds ids other than the requested oneAll four are reproducible locally today only by re-running with
--log-level debug. Everything above is already inStateDescat the point telemetry is collected — it just wasn't recorded.Privacy: types, counts, system-generated identifiers, and resource ids (same labels the deploy event already uses). No state paths, no resource keys or names, and no target names — the target is user-authored configuration, and
BundleDeployEventlikewise records onlytarget_count. Permissions/grants sub-resources are skipped: they index under the parent's type with a path-shaped object id that is not selectable and would be path-redacted.Tests
Acceptance snapshots cover the emitted payload on both engines —
bundle/telemetry/config-remote-sync{,-save,-recreate,-error}andbundle/config-remote-sync/{job_fields,resolve_variables}. The error case pinsstates_available_count: 0and the save case pins the id payload, so both of the informative-zero and id-reporting paths are asserted rather than described.resolve_variablesneeded astate_serialreplacement: the terraform path writes state more times than the direct path for the same script, so the value legitimately differs per engine and is not what that test asserts.