I mostly have nitty comments! Good stuff 💯

Reviewed 8 of 9 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @adityamaru and @irfansharif)

pkg/roachpb/span_config.go, line 113 at r1 (raw file):

// TestingSystemTargetDefaultSpanConfig exports the default span config that
// applies to spanconfig.SystemTargets for testing purposes.
func TestingSystemTargetDefaultSpanConfig() SpanConfig {

Should we call this TestingDefaultSystemSpanConfiguration?

pkg/spanconfig/spanconfig.go, line 116 at r1 (raw file):

	// configuration. Translate also accounts for and negotiates subzone spans.
	Translate(ctx context.Context, ids descpb.IDs,
		shouldGenerateSystemTargetRecords bool) ([]Record, hlc.Timestamp, error)

Similar to my comment above, should we call this generateSystemSpanConfigurations?

pkg/spanconfig/spanconfigreconciler/reconciler.go, line 382 at r1 (raw file):

					allIDs = append(allIDs, update.GetDescriptorUpdate().ID)
				} else if update.IsProtectedTimestampUpdate() {
					shouldGenerateSystemTargetRecords = true

Should we revert this until we teach + test system span configurations to the reconciler? Wiring this up right now, without the KV changes, might cause issues.

pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go, line 157 at r1 (raw file):

// generateSystemTargetRecords is responsible for generating all the SpanConfigs
// that apply to spanconfig.SystemTargets.
func (s *SQLTranslator) generateSystemTargetRecords(

s/generateSystemTargetRecords/generateSystemSpanConfigRecords/?

pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go, line 187 at r1 (raw file):

	// Aggregate tenant target protections.
	tenantProtections := ptsStateReader.getProtectionPoliciesForTenants()

Centralizing the discussion we've had in a couple of places, do you prefer this approach over returning ptpb.Target's from the ptsStateReader and writing conversions from a ptpb.Target to a spanconfig.Target (and vice versa)?

pkg/spanconfig/spanconfigtestutils/utils.go, line 277 at r1 (raw file):

// diffs the given config against the default config that applies to
// spanconfig.SystemTargets, and returns a string for the mismatched fields.
func PrintSystemTargetSpanConfigDiffedAgainstDefault(conf roachpb.SpanConfig) string {

s/PrintSystemTargetSpanConfigDiffedAgainstDefault/PrintSystemSpanConfigDiffedAgainstDefault/g

pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/datadriven_test.go, line 174 at r1 (raw file):

						output.WriteString(fmt.Sprintf("%-42s %s\n", record.Target.GetSpan(),
							spanconfigtestutils.PrintSpanConfigDiffedAgainstDefaults(record.Config)))
					} else {

nit: might be worth using a switch statement which explicitly checks for IsSpanTarget and IsSystemTarget and panics in the default case.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @arulajmani and @irfansharif)

pkg/roachpb/span_config.go, line 113 at r1 (raw file):

Done.

pkg/spanconfig/spanconfig.go, line 116 at r1 (raw file):

Done.

pkg/spanconfig/spanconfigreconciler/reconciler.go, line 382 at r1 (raw file):

Sounds good, added a todo.

pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go, line 157 at r1 (raw file):

Done.

pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go, line 187 at r1 (raw file):

After the discussions/refactor of the methods in your other PRs, I'm happy with where this is right now. If we think there's some cleanup to be done in the future we can revisit.

pkg/spanconfig/spanconfigtestutils/utils.go, line 277 at r1 (raw file):

Done.

pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/datadriven_test.go, line 174 at r1 (raw file):

Done.

once we introduce a new directive to translate system span configurations

Reviewed 9 of 9 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @adityamaru, @arulajmani, and @irfansharif)

pkg/spanconfig/spanconfigtestutils/utils.go, line 280 at r2 (raw file):

// PrintSystemSpanConfigDiffedAgainstDefault is a helper function that
// diffs the given config against the default config that applies to

nit: "the default system span config"

pkg/spanconfig/spanconfigtestutils/utils.go, line 284 at r2 (raw file):

func PrintSystemSpanConfigDiffedAgainstDefault(conf roachpb.SpanConfig) string {
	if conf.Equal(roachpb.TestingDefaultSystemSpanConfiguration()) {
		return "system target default"

nit: "default system span config"

pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/datadriven_test.go, line 163 at r2 (raw file):

				sqlTranslator := tenant.SpanConfigSQLTranslator().(spanconfig.SQLTranslator)
				records, _, err := sqlTranslator.Translate(ctx, descpb.IDs{objID},
					true /* generateSystemSpanConfigurations */)

Should we create a new directive to just generate system span configurations? As is, it's slightly confusing that these things are getting generated when translating a table or a database.