perf(milestones): refactor (#8788)

# Which Problems Are Solved

Milestones used existing events from a number of aggregates. OIDC
session is one of them. We noticed in load-tests that the reduction of
the oidc_session.added event into the milestone projection is a costly
business with payload based conditionals. A milestone is reached once,
but even then we remain subscribed to the OIDC events. This requires the
projections.current_states to be updated continuously.


# How the Problems Are Solved

The milestone creation is refactored to use dedicated events instead.
The command side decides when a milestone is reached and creates the
reached event once for each milestone when required.

# Additional Changes

In order to prevent reached milestones being created twice, a migration
script is provided. When the old `projections.milestones` table exist,
the state is read from there and `v2` milestone aggregate events are
created, with the original reached and pushed dates.

# Additional Context

- Closes https://github.com/zitadel/zitadel/issues/8800

internal/notification/handlers/commands.go

@@ -22,5 +22,5 @@ type Commands interface {
 	HumanPhoneVerificationCodeSent(ctx context.Context, orgID, userID string, generatorInfo *senders.CodeGeneratorInfo) error
 	InviteCodeSent(ctx context.Context, orgID, userID string) error
 	UsageNotificationSent(ctx context.Context, dueEvent *quota.NotificationDueEvent) error
-	MilestonePushed(ctx context.Context, msType milestone.Type, endpoints []string, primaryDomain string) error
+	MilestonePushed(ctx context.Context, instanceID string, msType milestone.Type, endpoints []string) error
 }

internal/notification/handlers/integration_test/telemetry_pusher_test.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/zitadel/zitadel/internal/integration"
 	"github.com/zitadel/zitadel/internal/integration/sink"
+	"github.com/zitadel/zitadel/internal/repository/milestone"
 	"github.com/zitadel/zitadel/pkg/grpc/app"
 	"github.com/zitadel/zitadel/pkg/grpc/management"
 	"github.com/zitadel/zitadel/pkg/grpc/object"
@@ -32,12 +33,12 @@ func TestServer_TelemetryPushMilestones(t *testing.T) {
 
 	instance := integration.NewInstance(CTX)
 	iamOwnerCtx := instance.WithAuthorization(CTX, integration.UserTypeIAMOwner)
-	t.Log("testing against instance with primary domain", instance.Domain)
-	awaitMilestone(t, sub, instance.Domain, "InstanceCreated")
+	t.Log("testing against instance", instance.ID())
+	awaitMilestone(t, sub, instance.ID(), milestone.InstanceCreated)
 
 	projectAdded, err := instance.Client.Mgmt.AddProject(iamOwnerCtx, &management.AddProjectRequest{Name: "integration"})
 	require.NoError(t, err)
-	awaitMilestone(t, sub, instance.Domain, "ProjectCreated")
+	awaitMilestone(t, sub, instance.ID(), milestone.ProjectCreated)
 
 	redirectURI := "http://localhost:8888"
 	application, err := instance.Client.Mgmt.AddOIDCApp(iamOwnerCtx, &management.AddOIDCAppRequest{
@@ -52,14 +53,14 @@ func TestServer_TelemetryPushMilestones(t *testing.T) {
 		AccessTokenType: app.OIDCTokenType_OIDC_TOKEN_TYPE_JWT,
 	})
 	require.NoError(t, err)
-	awaitMilestone(t, sub, instance.Domain, "ApplicationCreated")
+	awaitMilestone(t, sub, instance.ID(), milestone.ApplicationCreated)
 
 	// create the session to be used for the authN of the clients
 	sessionID, sessionToken, _, _ := instance.CreatePasswordSession(t, iamOwnerCtx, instance.AdminUserID, "Password1!")
 
 	console := consoleOIDCConfig(t, instance)
 	loginToClient(t, instance, console.GetClientId(), console.GetRedirectUris()[0], sessionID, sessionToken)
-	awaitMilestone(t, sub, instance.Domain, "AuthenticationSucceededOnInstance")
+	awaitMilestone(t, sub, instance.ID(), milestone.AuthenticationSucceededOnInstance)
 
 	// make sure the client has been projected
 	require.EventuallyWithT(t, func(collectT *assert.CollectT) {
@@ -70,11 +71,11 @@ func TestServer_TelemetryPushMilestones(t *testing.T) {
 		assert.NoError(collectT, err)
 	}, time.Minute, time.Second, "app not found")
 	loginToClient(t, instance, application.GetClientId(), redirectURI, sessionID, sessionToken)
-	awaitMilestone(t, sub, instance.Domain, "AuthenticationSucceededOnApplication")
+	awaitMilestone(t, sub, instance.ID(), milestone.AuthenticationSucceededOnApplication)
 
 	_, err = integration.SystemClient().RemoveInstance(CTX, &system.RemoveInstanceRequest{InstanceId: instance.ID()})
 	require.NoError(t, err)
-	awaitMilestone(t, sub, instance.Domain, "InstanceDeleted")
+	awaitMilestone(t, sub, instance.ID(), milestone.InstanceDeleted)
 }
 
 func loginToClient(t *testing.T, instance *integration.Instance, clientID, redirectURI, sessionID, sessionToken string) {
@@ -134,7 +135,7 @@ func consoleOIDCConfig(t *testing.T, instance *integration.Instance) *app.OIDCCo
 	return apps.GetResult()[0].GetOidcConfig()
 }
 
-func awaitMilestone(t *testing.T, sub *sink.Subscription, primaryDomain, expectMilestoneType string) {
+func awaitMilestone(t *testing.T, sub *sink.Subscription, instanceID string, expectMilestoneType milestone.Type) {
 	for {
 		select {
 		case req := <-sub.Recv():
@@ -144,17 +145,17 @@ func awaitMilestone(t *testing.T, sub *sink.Subscription, primaryDomain, expectM
 			}
 			t.Log("received milestone", plain.String())
 			milestone := struct {
-				Type          string `json:"type"`
-				PrimaryDomain string `json:"primaryDomain"`
+				InstanceID string         `json:"instanceId"`
+				Type       milestone.Type `json:"type"`
 			}{}
 			if err := json.Unmarshal(req.Body, &milestone); err != nil {
 				t.Error(err)
 			}
-			if milestone.Type == expectMilestoneType && milestone.PrimaryDomain == primaryDomain {
+			if milestone.Type == expectMilestoneType && milestone.InstanceID == instanceID {
 				return
 			}
-		case <-time.After(2 * time.Minute): // why does it take so long to get a milestone !?
-			t.Fatalf("timed out waiting for milestone %s in domain %s", expectMilestoneType, primaryDomain)
+		case <-time.After(20 * time.Second):
+			t.Fatalf("timed out waiting for milestone %s for instance %s", expectMilestoneType, instanceID)
 		}
 	}
 }

internal/notification/handlers/mock/commands.mock.go

@@ -141,7 +141,7 @@ func (mr *MockCommandsMockRecorder) InviteCodeSent(arg0, arg1, arg2 any) *gomock
 }
 
 // MilestonePushed mocks base method.
-func (m *MockCommands) MilestonePushed(arg0 context.Context, arg1 milestone.Type, arg2 []string, arg3 string) error {
+func (m *MockCommands) MilestonePushed(arg0 context.Context, arg1 string, arg2 milestone.Type, arg3 []string) error {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "MilestonePushed", arg0, arg1, arg2, arg3)
 	ret0, _ := ret[0].(error)

internal/notification/handlers/telemetry_pusher.go

@@ -2,13 +2,9 @@ package handlers
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 	"time"
 
-	"github.com/zitadel/logging"
-
-	"github.com/zitadel/zitadel/internal/api/authz"
 	"github.com/zitadel/zitadel/internal/api/call"
 	"github.com/zitadel/zitadel/internal/command"
 	"github.com/zitadel/zitadel/internal/eventstore"
@@ -16,9 +12,7 @@ import (
 	"github.com/zitadel/zitadel/internal/notification/channels/webhook"
 	_ "github.com/zitadel/zitadel/internal/notification/statik"
 	"github.com/zitadel/zitadel/internal/notification/types"
-	"github.com/zitadel/zitadel/internal/query"
 	"github.com/zitadel/zitadel/internal/repository/milestone"
-	"github.com/zitadel/zitadel/internal/repository/pseudo"
 	"github.com/zitadel/zitadel/internal/zerrors"
 )
 
@@ -30,7 +24,6 @@ type TelemetryPusherConfig struct {
 	Enabled   bool
 	Endpoints []string
 	Headers   http.Header
-	Limit     uint64
 }
 
 type telemetryPusher struct {
@@ -54,7 +47,6 @@ func NewTelemetryPusher(
 		queries:  queries,
 		channels: channels,
 	}
-	handlerCfg.TriggerWithoutEvents = pusher.pushMilestones
 	return handler.NewHandler(
 		ctx,
 		&handlerCfg,
@@ -68,9 +60,9 @@ func (u *telemetryPusher) Name() string {
 
 func (t *telemetryPusher) Reducers() []handler.AggregateReducer {
 	return []handler.AggregateReducer{{
-		Aggregate: pseudo.AggregateType,
+		Aggregate: milestone.AggregateType,
 		EventReducers: []handler.EventReducer{{
-			Event:  pseudo.ScheduledEventType,
+			Event:  milestone.ReachedEventType,
 			Reduce: t.pushMilestones,
 		}},
 	}}
@@ -78,51 +70,20 @@ func (t *telemetryPusher) Reducers() []handler.AggregateReducer {
 
 func (t *telemetryPusher) pushMilestones(event eventstore.Event) (*handler.Statement, error) {
 	ctx := call.WithTimestamp(context.Background())
-	scheduledEvent, ok := event.(*pseudo.ScheduledEvent)
+	e, ok := event.(*milestone.ReachedEvent)
 	if !ok {
 		return nil, zerrors.ThrowInvalidArgumentf(nil, "HANDL-lDTs5", "reduce.wrong.event.type %s", event.Type())
 	}
-
-	return handler.NewStatement(event, func(ex handler.Executer, projectionName string) error {
-		isReached, err := query.NewNotNullQuery(query.MilestoneReachedDateColID)
-		if err != nil {
-			return err
+	return handler.NewStatement(event, func(handler.Executer, string) error {
+		// Do not push the milestone again if this was a migration event.
+		if e.ReachedDate != nil {
+			return nil
 		}
-		isNotPushed, err := query.NewIsNullQuery(query.MilestonePushedDateColID)
-		if err != nil {
-			return err
-		}
-		hasPrimaryDomain, err := query.NewNotNullQuery(query.MilestonePrimaryDomainColID)
-		if err != nil {
-			return err
-		}
-		unpushedMilestones, err := t.queries.Queries.SearchMilestones(ctx, scheduledEvent.InstanceIDs, &query.MilestonesSearchQueries{
-			SearchRequest: query.SearchRequest{
-				Limit:         t.cfg.Limit,
-				SortingColumn: query.MilestoneReachedDateColID,
-				Asc:           true,
-			},
-			Queries: []query.SearchQuery{isReached, isNotPushed, hasPrimaryDomain},
-		})
-		if err != nil {
-			return err
-		}
-		var errs int
-		for _, ms := range unpushedMilestones.Milestones {
-			if err = t.pushMilestone(ctx, scheduledEvent, ms); err != nil {
-				errs++
-				logging.Warnf("pushing milestone %+v failed: %s", *ms, err.Error())
-			}
-		}
-		if errs > 0 {
-			return fmt.Errorf("pushing %d of %d milestones failed", errs, unpushedMilestones.Count)
-		}
-		return nil
+		return t.pushMilestone(ctx, e)
 	}), nil
 }
 
-func (t *telemetryPusher) pushMilestone(ctx context.Context, event *pseudo.ScheduledEvent, ms *query.Milestone) error {
-	ctx = authz.WithInstanceID(ctx, ms.InstanceID)
+func (t *telemetryPusher) pushMilestone(ctx context.Context, e *milestone.ReachedEvent) error {
 	for _, endpoint := range t.cfg.Endpoints {
 		if err := types.SendJSON(
 			ctx,
@@ -135,20 +96,18 @@ func (t *telemetryPusher) pushMilestone(ctx context.Context, event *pseudo.Sched
 			&struct {
 				InstanceID     string         `json:"instanceId"`
 				ExternalDomain string         `json:"externalDomain"`
-				PrimaryDomain  string         `json:"primaryDomain"`
 				Type           milestone.Type `json:"type"`
 				ReachedDate    time.Time      `json:"reached"`
 			}{
-				InstanceID:     ms.InstanceID,
+				InstanceID:     e.Agg.InstanceID,
 				ExternalDomain: t.queries.externalDomain,
-				PrimaryDomain:  ms.PrimaryDomain,
-				Type:           ms.Type,
-				ReachedDate:    ms.ReachedDate,
+				Type:           e.MilestoneType,
+				ReachedDate:    e.GetReachedDate(),
 			},
-			event,
+			e,
 		).WithoutTemplate(); err != nil {
 			return err
 		}
 	}
-	return t.commands.MilestonePushed(ctx, ms.Type, t.cfg.Endpoints, ms.PrimaryDomain)
+	return t.commands.MilestonePushed(ctx, e.Agg.InstanceID, e.MilestoneType, t.cfg.Endpoints)
 }