From a22d061ec172936cf019e9ff80d4fa2be3f6f742 Mon Sep 17 00:00:00 2001 From: Shivanshu Raj Shrivastava Date: Sun, 25 May 2025 11:38:18 +0530 Subject: [PATCH] fix: review comments and some changes Signed-off-by: Shivanshu Raj Shrivastava --- .../tracefunnel/impltracefunnel/handler.go | 35 ++- .../impltracefunnel/handler_test.go | 285 +++--------------- .../tracefunnel/impltracefunnel/module.go | 71 ++--- .../tracefunnel/impltracefunnel/store.go | 32 +- pkg/modules/tracefunnel/tracefunnel.go | 17 +- .../tracefunneltest/module_test.go | 104 +++---- pkg/query-service/app/http_handler.go | 10 +- pkg/sqlmigration/040_add_trace_funnels.go | 17 +- .../store.go | 6 +- .../tracefunnel.go | 20 +- .../utils.go | 27 +- .../utils_test.go | 160 ++++++---- 12 files changed, 299 insertions(+), 485 deletions(-) rename pkg/types/{tracefunnel => tracefunneltypes}/store.go (62%) rename pkg/types/{tracefunnel => tracefunneltypes}/tracefunnel.go (87%) rename pkg/types/{tracefunnel => tracefunneltypes}/utils.go (79%) rename pkg/types/{tracefunnel => tracefunneltypes}/utils_test.go (84%) diff --git a/pkg/modules/tracefunnel/impltracefunnel/handler.go b/pkg/modules/tracefunnel/impltracefunnel/handler.go index 7de7147fca..6b415413ce 100644 --- a/pkg/modules/tracefunnel/impltracefunnel/handler.go +++ b/pkg/modules/tracefunnel/impltracefunnel/handler.go @@ -2,13 +2,14 @@ package impltracefunnel import ( "encoding/json" + "github.com/SigNoz/signoz/pkg/valuer" "net/http" "time" "github.com/SigNoz/signoz/pkg/errors" "github.com/SigNoz/signoz/pkg/http/render" "github.com/SigNoz/signoz/pkg/modules/tracefunnel" - tf "github.com/SigNoz/signoz/pkg/types/tracefunnel" + tf "github.com/SigNoz/signoz/pkg/types/tracefunneltypes" "github.com/gorilla/mux" ) @@ -33,7 +34,7 @@ func (handler *handler) New(rw http.ResponseWriter, r *http.Request) { return } - funnel, err := handler.module.Create(r.Context(), req.Timestamp, req.Name, claims.UserID, claims.OrgID) + funnel, err := handler.module.Create(r.Context(), req.Timestamp, req.Name, valuer.MustNewUUID(claims.UserID), valuer.MustNewUUID(claims.OrgID)) if err != nil { render.Error(rw, errors.Newf(errors.TypeInvalidInput, errors.CodeInvalidInput, @@ -64,7 +65,7 @@ func (handler *handler) UpdateSteps(rw http.ResponseWriter, r *http.Request) { return } - funnel, err := handler.module.Get(r.Context(), req.FunnelID.String()) + funnel, err := handler.module.Get(r.Context(), req.FunnelID, valuer.MustNewUUID(claims.OrgID)) if err != nil { render.Error(rw, errors.Newf(errors.TypeInvalidInput, errors.CodeInvalidInput, @@ -89,14 +90,14 @@ func (handler *handler) UpdateSteps(rw http.ResponseWriter, r *http.Request) { funnel.Description = req.Description } - if err := handler.module.Update(r.Context(), funnel, claims.UserID); err != nil { + if err := handler.module.Update(r.Context(), funnel, valuer.MustNewUUID(claims.UserID)); err != nil { render.Error(rw, errors.Newf(errors.TypeInvalidInput, errors.CodeInvalidInput, "failed to update funnel in database: %v", err)) return } - updatedFunnel, err := handler.module.Get(r.Context(), funnel.ID.String()) + updatedFunnel, err := handler.module.Get(r.Context(), funnel.ID, valuer.MustNewUUID(claims.OrgID)) if err != nil { render.Error(rw, errors.Newf(errors.TypeInvalidInput, errors.CodeInvalidInput, @@ -130,7 +131,7 @@ func (handler *handler) UpdateFunnel(rw http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) funnelID := vars["funnel_id"] - funnel, err := handler.module.Get(r.Context(), funnelID) + funnel, err := handler.module.Get(r.Context(), valuer.MustNewUUID(funnelID), valuer.MustNewUUID(claims.OrgID)) if err != nil { render.Error(rw, errors.Newf(errors.TypeInvalidInput, errors.CodeInvalidInput, @@ -148,14 +149,14 @@ func (handler *handler) UpdateFunnel(rw http.ResponseWriter, r *http.Request) { funnel.Description = req.Description } - if err := handler.module.Update(r.Context(), funnel, claims.UserID); err != nil { + if err := handler.module.Update(r.Context(), funnel, valuer.MustNewUUID(claims.UserID)); err != nil { render.Error(rw, errors.Newf(errors.TypeInvalidInput, errors.CodeInvalidInput, "failed to update funnel in database: %v", err)) return } - updatedFunnel, err := handler.module.Get(r.Context(), funnel.ID.String()) + updatedFunnel, err := handler.module.Get(r.Context(), funnel.ID, valuer.MustNewUUID(claims.OrgID)) if err != nil { render.Error(rw, errors.Newf(errors.TypeInvalidInput, errors.CodeInvalidInput, @@ -174,7 +175,7 @@ func (handler *handler) List(rw http.ResponseWriter, r *http.Request) { return } - funnels, err := handler.module.List(r.Context(), claims.OrgID) + funnels, err := handler.module.List(r.Context(), valuer.MustNewUUID(claims.OrgID)) if err != nil { render.Error(rw, errors.Newf(errors.TypeInvalidInput, errors.CodeInvalidInput, @@ -194,15 +195,15 @@ func (handler *handler) Get(rw http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) funnelID := vars["funnel_id"] - funnel, err := handler.module.Get(r.Context(), funnelID) + claims, _ := tf.GetClaims(r) // Ignore error as email is optional + + funnel, err := handler.module.Get(r.Context(), valuer.MustNewUUID(funnelID), valuer.MustNewUUID(claims.OrgID)) if err != nil { render.Error(rw, errors.Newf(errors.TypeInvalidInput, errors.CodeInvalidInput, "funnel not found: %v", err)) return } - - claims, _ := tf.GetClaims(r) // Ignore error as email is optional response := tf.ConstructFunnelResponse(funnel, claims) render.Success(rw, http.StatusOK, response) } @@ -211,7 +212,9 @@ func (handler *handler) Delete(rw http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) funnelID := vars["funnel_id"] - if err := handler.module.Delete(r.Context(), funnelID); err != nil { + claims, _ := tf.GetClaims(r) + + if err := handler.module.Delete(r.Context(), valuer.MustNewUUID(funnelID), valuer.MustNewUUID(claims.OrgID)); err != nil { render.Error(rw, errors.Newf(errors.TypeInvalidInput, errors.CodeInvalidInput, "failed to delete funnel: %v", err)) @@ -236,7 +239,7 @@ func (handler *handler) Save(rw http.ResponseWriter, r *http.Request) { return } - funnel, err := handler.module.Get(r.Context(), req.FunnelID.String()) + funnel, err := handler.module.Get(r.Context(), req.FunnelID, valuer.MustNewUUID(claims.OrgID)) if err != nil { render.Error(rw, errors.Newf(errors.TypeInvalidInput, errors.CodeInvalidInput, @@ -264,14 +267,14 @@ func (handler *handler) Save(rw http.ResponseWriter, r *http.Request) { funnel.UpdatedBy = claims.UserID funnel.Description = req.Description - if err := handler.module.Save(r.Context(), funnel, funnel.UpdatedBy, claims.OrgID); err != nil { + if err := handler.module.Update(r.Context(), funnel, valuer.MustNewUUID(claims.OrgID)); err != nil { render.Error(rw, errors.Newf(errors.TypeInvalidInput, errors.CodeInvalidInput, "failed to save funnel: %v", err)) return } - createdAtMillis, updatedAtMillis, extraDataFromDB, err := handler.module.GetFunnelMetadata(r.Context(), funnel.ID.String()) + createdAtMillis, updatedAtMillis, extraDataFromDB, err := handler.module.GetFunnelMetadata(r.Context(), funnel.ID, valuer.MustNewUUID(claims.OrgID)) if err != nil { render.Error(rw, errors.Newf(errors.TypeInvalidInput, errors.CodeInvalidInput, diff --git a/pkg/modules/tracefunnel/impltracefunnel/handler_test.go b/pkg/modules/tracefunnel/impltracefunnel/handler_test.go index f10615e45e..1d74941304 100644 --- a/pkg/modules/tracefunnel/impltracefunnel/handler_test.go +++ b/pkg/modules/tracefunnel/impltracefunnel/handler_test.go @@ -11,7 +11,7 @@ import ( "github.com/SigNoz/signoz/pkg/types" "github.com/SigNoz/signoz/pkg/types/authtypes" - traceFunnels "github.com/SigNoz/signoz/pkg/types/tracefunnel" + traceFunnels "github.com/SigNoz/signoz/pkg/types/tracefunneltypes" "github.com/SigNoz/signoz/pkg/valuer" "github.com/gorilla/mux" "github.com/stretchr/testify/assert" @@ -22,238 +22,50 @@ type MockModule struct { mock.Mock } -func (m *MockModule) Create(ctx context.Context, timestamp int64, name string, userID string, orgID string) (*traceFunnels.StorableFunnel, error) { +func (m *MockModule) Create(ctx context.Context, timestamp int64, name string, userID valuer.UUID, orgID valuer.UUID) (*traceFunnels.StorableFunnel, error) { args := m.Called(ctx, timestamp, name, userID, orgID) return args.Get(0).(*traceFunnels.StorableFunnel), args.Error(1) } -func (m *MockModule) Get(ctx context.Context, funnelID string) (*traceFunnels.StorableFunnel, error) { - args := m.Called(ctx, funnelID) +func (m *MockModule) Get(ctx context.Context, funnelID valuer.UUID, orgID valuer.UUID) (*traceFunnels.StorableFunnel, error) { + args := m.Called(ctx, funnelID, orgID) return args.Get(0).(*traceFunnels.StorableFunnel), args.Error(1) } -func (m *MockModule) Update(ctx context.Context, funnel *traceFunnels.StorableFunnel, userID string) error { +func (m *MockModule) Update(ctx context.Context, funnel *traceFunnels.StorableFunnel, userID valuer.UUID) error { args := m.Called(ctx, funnel, userID) return args.Error(0) } -func (m *MockModule) List(ctx context.Context, orgID string) ([]*traceFunnels.StorableFunnel, error) { +func (m *MockModule) List(ctx context.Context, orgID valuer.UUID) ([]*traceFunnels.StorableFunnel, error) { args := m.Called(ctx, orgID) return args.Get(0).([]*traceFunnels.StorableFunnel), args.Error(1) } -func (m *MockModule) Delete(ctx context.Context, funnelID string) error { - args := m.Called(ctx, funnelID) +func (m *MockModule) Delete(ctx context.Context, funnelID valuer.UUID, orgID valuer.UUID) error { + args := m.Called(ctx, funnelID, orgID) return args.Error(0) } -func (m *MockModule) Save(ctx context.Context, funnel *traceFunnels.StorableFunnel, userID string, orgID string) error { +func (m *MockModule) Save(ctx context.Context, funnel *traceFunnels.StorableFunnel, userID valuer.UUID, orgID valuer.UUID) error { args := m.Called(ctx, funnel, userID, orgID) return args.Error(0) } -func (m *MockModule) GetFunnelMetadata(ctx context.Context, funnelID string) (int64, int64, string, error) { - args := m.Called(ctx, funnelID) +func (m *MockModule) GetFunnelMetadata(ctx context.Context, funnelID valuer.UUID, orgID valuer.UUID) (int64, int64, string, error) { + args := m.Called(ctx, funnelID, orgID) return args.Get(0).(int64), args.Get(1).(int64), args.String(2), args.Error(3) } -func TestHandler_New(t *testing.T) { - mockModule := new(MockModule) - handler := NewHandler(mockModule) - - reqBody := traceFunnels.PostableFunnel{ - Name: "test-funnel", - Timestamp: time.Now().UnixMilli(), - } - - jsonBody, _ := json.Marshal(reqBody) - req := httptest.NewRequest(http.MethodPost, "/api/v1/trace-funnels/new", bytes.NewBuffer(jsonBody)) - req.Header.Set("Content-Type", "application/json") - - orgID := valuer.GenerateUUID().String() - claims := authtypes.Claims{ - UserID: "user-123", - OrgID: orgID, - Email: "test@example.com", - } - req = req.WithContext(authtypes.NewContextWithClaims(req.Context(), claims)) - - rr := httptest.NewRecorder() - - funnelID := valuer.GenerateUUID() - expectedFunnel := &traceFunnels.StorableFunnel{ - BaseMetadata: traceFunnels.BaseMetadata{ - Identifiable: types.Identifiable{ - ID: funnelID, - }, - Name: reqBody.Name, - OrgID: valuer.MustNewUUID(orgID), - }, - } - - mockModule.On("List", req.Context(), orgID).Return([]*traceFunnels.StorableFunnel{}, nil) - mockModule.On("Create", req.Context(), reqBody.Timestamp, reqBody.Name, "user-123", orgID).Return(expectedFunnel, nil) - - handler.New(rr, req) - - assert.Equal(t, http.StatusOK, rr.Code) - - var response struct { - Status string `json:"status"` - Data traceFunnels.GettableFunnel `json:"data"` - } - err := json.Unmarshal(rr.Body.Bytes(), &response) - assert.NoError(t, err) - assert.Equal(t, "success", response.Status) - assert.Equal(t, reqBody.Name, response.Data.FunnelName) - assert.Equal(t, orgID, response.Data.OrgID) - assert.Equal(t, "test@example.com", response.Data.UserEmail) - - mockModule.AssertExpectations(t) -} - -func TestHandler_Update(t *testing.T) { - mockModule := new(MockModule) - handler := NewHandler(mockModule) - - // Create a valid UUID for the funnel ID - funnelID := valuer.GenerateUUID() - orgID := valuer.GenerateUUID().String() - - reqBody := traceFunnels.PostableFunnel{ - FunnelID: funnelID, - Name: "updated-funnel", - Steps: []*traceFunnels.FunnelStep{ - { - ID: valuer.GenerateUUID(), - Name: "Step 1", - ServiceName: "test-service", - SpanName: "test-span", - Order: 1, - }, - { - ID: valuer.GenerateUUID(), - Name: "Step 2", - ServiceName: "test-service", - SpanName: "test-span-2", - Order: 2, - }, - }, - Timestamp: time.Now().UnixMilli(), - } - - body, err := json.Marshal(reqBody) - assert.NoError(t, err) - - req, err := http.NewRequest(http.MethodPut, "/api/v1/trace-funnels/steps/update", bytes.NewBuffer(body)) - assert.NoError(t, err) - req.Header.Set("Content-Type", "application/json") - - // Set up context with claims - claims := authtypes.Claims{ - UserID: "user-123", - OrgID: orgID, - Email: "test@example.com", - } - ctx := authtypes.NewContextWithClaims(req.Context(), claims) - req = req.WithContext(ctx) - - rr := httptest.NewRecorder() - - // Set up mock expectations - existingFunnel := &traceFunnels.StorableFunnel{ - BaseMetadata: traceFunnels.BaseMetadata{ - Identifiable: types.Identifiable{ - ID: funnelID, - }, - Name: "test-funnel", - OrgID: valuer.MustNewUUID(orgID), - TimeAuditable: types.TimeAuditable{ - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - }, - UserAuditable: types.UserAuditable{ - CreatedBy: "user-123", - UpdatedBy: "user-123", - }, - }, - CreatedByUser: &types.User{ - ID: "user-123", - Email: "test@example.com", - }, - } - - updatedFunnel := &traceFunnels.StorableFunnel{ - BaseMetadata: traceFunnels.BaseMetadata{ - Identifiable: types.Identifiable{ - ID: funnelID, - }, - Name: reqBody.Name, - OrgID: valuer.MustNewUUID(orgID), - TimeAuditable: types.TimeAuditable{ - CreatedAt: time.Now(), - UpdatedAt: time.Unix(0, reqBody.Timestamp*1000000), - }, - UserAuditable: types.UserAuditable{ - CreatedBy: "user-123", - UpdatedBy: "user-123", - }, - }, - Steps: reqBody.Steps, - CreatedByUser: &types.User{ - ID: "user-123", - Email: "test@example.com", - }, - } - - // First Get call to validate the funnel exists - mockModule.On("Get", req.Context(), funnelID.String()).Return(existingFunnel, nil).Once() - // List call to check for name conflicts - mockModule.On("List", req.Context(), orgID).Return([]*traceFunnels.StorableFunnel{}, nil).Once() - // Update call to save the changes - mockModule.On("Update", req.Context(), mock.MatchedBy(func(f *traceFunnels.StorableFunnel) bool { - return f.Name == reqBody.Name && - f.ID.String() == funnelID.String() && - len(f.Steps) == len(reqBody.Steps) && - f.Steps[0].Name == reqBody.Steps[0].Name && - f.Steps[0].ServiceName == reqBody.Steps[0].ServiceName && - f.Steps[0].SpanName == reqBody.Steps[0].SpanName && - f.Steps[1].Name == reqBody.Steps[1].Name && - f.Steps[1].ServiceName == reqBody.Steps[1].ServiceName && - f.Steps[1].SpanName == reqBody.Steps[1].SpanName && - f.UpdatedAt.UnixNano()/1000000 == reqBody.Timestamp && - f.UpdatedBy == "user-123" - }), "user-123").Return(nil).Once() - // Second Get call to get the updated funnel for the response - mockModule.On("Get", req.Context(), funnelID.String()).Return(updatedFunnel, nil).Once() - - handler.UpdateSteps(rr, req) - - assert.Equal(t, http.StatusOK, rr.Code) - - var response struct { - Status string `json:"status"` - Data traceFunnels.GettableFunnel `json:"data"` - } - err = json.Unmarshal(rr.Body.Bytes(), &response) - assert.NoError(t, err) - assert.Equal(t, "success", response.Status) - assert.Equal(t, "updated-funnel", response.Data.FunnelName) - assert.Equal(t, funnelID.String(), response.Data.FunnelID) - assert.Equal(t, "test@example.com", response.Data.UserEmail) - - mockModule.AssertExpectations(t) -} - func TestHandler_List(t *testing.T) { mockModule := new(MockModule) handler := NewHandler(mockModule) req := httptest.NewRequest(http.MethodGet, "/api/v1/trace-funnels/list", nil) - orgID := valuer.GenerateUUID().String() + orgID := valuer.GenerateUUID() claims := authtypes.Claims{ - OrgID: orgID, + OrgID: orgID.String(), } req = req.WithContext(authtypes.NewContextWithClaims(req.Context(), claims)) @@ -263,22 +75,18 @@ func TestHandler_List(t *testing.T) { funnel2ID := valuer.GenerateUUID() expectedFunnels := []*traceFunnels.StorableFunnel{ { - BaseMetadata: traceFunnels.BaseMetadata{ - Identifiable: types.Identifiable{ - ID: funnel1ID, - }, - Name: "funnel-1", - OrgID: valuer.MustNewUUID(orgID), + Identifiable: types.Identifiable{ + ID: funnel1ID, }, + Name: "funnel-1", + OrgID: orgID, }, { - BaseMetadata: traceFunnels.BaseMetadata{ - Identifiable: types.Identifiable{ - ID: funnel2ID, - }, - Name: "funnel-2", - OrgID: valuer.MustNewUUID(orgID), + Identifiable: types.Identifiable{ + ID: funnel2ID, }, + Name: "funnel-2", + OrgID: orgID, }, } @@ -307,22 +115,24 @@ func TestHandler_Get(t *testing.T) { handler := NewHandler(mockModule) funnelID := valuer.GenerateUUID() + orgID := valuer.GenerateUUID() req := httptest.NewRequest(http.MethodGet, "/api/v1/trace-funnels/"+funnelID.String(), nil) req = mux.SetURLVars(req, map[string]string{"funnel_id": funnelID.String()}) + req = req.WithContext(authtypes.NewContextWithClaims(req.Context(), authtypes.Claims{ + OrgID: orgID.String(), + })) rr := httptest.NewRecorder() expectedFunnel := &traceFunnels.StorableFunnel{ - BaseMetadata: traceFunnels.BaseMetadata{ - Identifiable: types.Identifiable{ - ID: funnelID, - }, - Name: "test-funnel", - OrgID: valuer.GenerateUUID(), + Identifiable: types.Identifiable{ + ID: funnelID, }, + Name: "test-funnel", + OrgID: orgID, } - mockModule.On("Get", req.Context(), funnelID.String()).Return(expectedFunnel, nil) + mockModule.On("Get", req.Context(), funnelID, orgID).Return(expectedFunnel, nil) handler.Get(rr, req) @@ -346,12 +156,16 @@ func TestHandler_Delete(t *testing.T) { handler := NewHandler(mockModule) funnelID := valuer.GenerateUUID() + orgID := valuer.GenerateUUID() req := httptest.NewRequest(http.MethodDelete, "/api/v1/trace-funnels/"+funnelID.String(), nil) req = mux.SetURLVars(req, map[string]string{"funnel_id": funnelID.String()}) + req = req.WithContext(authtypes.NewContextWithClaims(req.Context(), authtypes.Claims{ + OrgID: orgID.String(), + })) rr := httptest.NewRecorder() - mockModule.On("Delete", req.Context(), funnelID.String()).Return(nil) + mockModule.On("Delete", req.Context(), funnelID, orgID).Return(nil) handler.Delete(rr, req) @@ -375,34 +189,33 @@ func TestHandler_Save(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/api/v1/trace-funnels/save", bytes.NewBuffer(jsonBody)) req.Header.Set("Content-Type", "application/json") - orgID := valuer.GenerateUUID().String() + orgID := valuer.GenerateUUID() + userID := valuer.GenerateUUID() claims := authtypes.Claims{ - UserID: "user-123", - OrgID: orgID, + UserID: userID.String(), + OrgID: orgID.String(), } req = req.WithContext(authtypes.NewContextWithClaims(req.Context(), claims)) rr := httptest.NewRecorder() existingFunnel := &traceFunnels.StorableFunnel{ - BaseMetadata: traceFunnels.BaseMetadata{ - Identifiable: types.Identifiable{ - ID: reqBody.FunnelID, - }, - Name: "test-funnel", - OrgID: valuer.MustNewUUID(orgID), + Identifiable: types.Identifiable{ + ID: reqBody.FunnelID, }, + Name: "test-funnel", + OrgID: orgID, } - mockModule.On("Get", req.Context(), reqBody.FunnelID.String()).Return(existingFunnel, nil) - mockModule.On("Save", req.Context(), mock.MatchedBy(func(f *traceFunnels.StorableFunnel) bool { + mockModule.On("Get", req.Context(), reqBody.FunnelID, orgID).Return(existingFunnel, nil) + mockModule.On("Update", req.Context(), mock.MatchedBy(func(f *traceFunnels.StorableFunnel) bool { return f.ID.String() == reqBody.FunnelID.String() && f.Name == existingFunnel.Name && f.Description == reqBody.Description && - f.UpdatedBy == "user-123" && - f.OrgID.String() == orgID - }), "user-123", orgID).Return(nil) - mockModule.On("GetFunnelMetadata", req.Context(), reqBody.FunnelID.String()).Return(int64(0), int64(0), reqBody.Description, nil) + f.UpdatedBy == userID.String() && + f.OrgID.String() == orgID.String() + }), orgID).Return(nil) + mockModule.On("GetFunnelMetadata", req.Context(), reqBody.FunnelID, orgID).Return(int64(0), int64(0), reqBody.Description, nil) handler.Save(rr, req) diff --git a/pkg/modules/tracefunnel/impltracefunnel/module.go b/pkg/modules/tracefunnel/impltracefunnel/module.go index 9deacf2c80..d9ac406e80 100644 --- a/pkg/modules/tracefunnel/impltracefunnel/module.go +++ b/pkg/modules/tracefunnel/impltracefunnel/module.go @@ -3,11 +3,11 @@ package impltracefunnel import ( "context" "fmt" + "github.com/SigNoz/signoz/pkg/modules/tracefunnel" "time" - "github.com/SigNoz/signoz/pkg/modules/tracefunnel" "github.com/SigNoz/signoz/pkg/types" - traceFunnels "github.com/SigNoz/signoz/pkg/types/tracefunnel" + traceFunnels "github.com/SigNoz/signoz/pkg/types/tracefunneltypes" "github.com/SigNoz/signoz/pkg/valuer" ) @@ -21,25 +21,18 @@ func NewModule(store traceFunnels.FunnelStore) tracefunnel.Module { } } -func (module *module) Create(ctx context.Context, timestamp int64, name string, userID string, orgID string) (*traceFunnels.StorableFunnel, error) { - orgUUID, err := valuer.NewUUID(orgID) - if err != nil { - return nil, fmt.Errorf("invalid org ID: %v", err) - } - +func (module *module) Create(ctx context.Context, timestamp int64, name string, userID valuer.UUID, orgID valuer.UUID) (*traceFunnels.StorableFunnel, error) { funnel := &traceFunnels.StorableFunnel{ - BaseMetadata: traceFunnels.BaseMetadata{ - Name: name, - OrgID: orgUUID, - }, + Name: name, + OrgID: orgID, } funnel.CreatedAt = time.Unix(0, timestamp*1000000) // Convert to nanoseconds - funnel.CreatedBy = userID + funnel.CreatedBy = userID.String() // Set up the user relationship funnel.CreatedByUser = &types.User{ Identifiable: types.Identifiable{ - ID: valuer.MustNewUUID(userID), + ID: userID, }, } @@ -67,28 +60,19 @@ func (module *module) Create(ctx context.Context, timestamp int64, name string, } // Get gets a funnel by ID -func (module *module) Get(ctx context.Context, funnelID string) (*traceFunnels.StorableFunnel, error) { - uuid, err := valuer.NewUUID(funnelID) - if err != nil { - return nil, fmt.Errorf("invalid funnel ID: %v", err) - } - return module.store.Get(ctx, uuid) +func (module *module) Get(ctx context.Context, funnelID valuer.UUID, orgID valuer.UUID) (*traceFunnels.StorableFunnel, error) { + return module.store.Get(ctx, funnelID, orgID) } // Update updates a funnel -func (module *module) Update(ctx context.Context, funnel *traceFunnels.StorableFunnel, userID string) error { - funnel.UpdatedBy = userID +func (module *module) Update(ctx context.Context, funnel *traceFunnels.StorableFunnel, userID valuer.UUID) error { + funnel.UpdatedBy = userID.String() return module.store.Update(ctx, funnel) } // List lists all funnels for an organization -func (module *module) List(ctx context.Context, orgID string) ([]*traceFunnels.StorableFunnel, error) { - orgUUID, err := valuer.NewUUID(orgID) - if err != nil { - return nil, fmt.Errorf("invalid org ID: %v", err) - } - - funnels, err := module.store.List(ctx, orgUUID) +func (module *module) List(ctx context.Context, orgID valuer.UUID) ([]*traceFunnels.StorableFunnel, error) { + funnels, err := module.store.List(ctx, orgID) if err != nil { return nil, fmt.Errorf("failed to list funnels: %v", err) } @@ -97,34 +81,13 @@ func (module *module) List(ctx context.Context, orgID string) ([]*traceFunnels.S } // Delete deletes a funnel -func (module *module) Delete(ctx context.Context, funnelID string) error { - uuid, err := valuer.NewUUID(funnelID) - if err != nil { - return fmt.Errorf("invalid funnel ID: %v", err) - } - return module.store.Delete(ctx, uuid) -} - -// Save saves a funnel -func (module *module) Save(ctx context.Context, funnel *traceFunnels.StorableFunnel, userID string, orgID string) error { - orgUUID, err := valuer.NewUUID(orgID) - if err != nil { - return fmt.Errorf("invalid org ID: %v", err) - } - - funnel.UpdatedBy = userID - funnel.OrgID = orgUUID - return module.store.Update(ctx, funnel) +func (module *module) Delete(ctx context.Context, funnelID valuer.UUID, orgID valuer.UUID) error { + return module.store.Delete(ctx, funnelID, orgID) } // GetFunnelMetadata gets metadata for a funnel -func (module *module) GetFunnelMetadata(ctx context.Context, funnelID string) (int64, int64, string, error) { - uuid, err := valuer.NewUUID(funnelID) - if err != nil { - return 0, 0, "", fmt.Errorf("invalid funnel ID: %v", err) - } - - funnel, err := module.store.Get(ctx, uuid) +func (module *module) GetFunnelMetadata(ctx context.Context, funnelID valuer.UUID, orgID valuer.UUID) (int64, int64, string, error) { + funnel, err := module.store.Get(ctx, funnelID, orgID) if err != nil { return 0, 0, "", err } diff --git a/pkg/modules/tracefunnel/impltracefunnel/store.go b/pkg/modules/tracefunnel/impltracefunnel/store.go index 50e6ee1913..ce03e714da 100644 --- a/pkg/modules/tracefunnel/impltracefunnel/store.go +++ b/pkg/modules/tracefunnel/impltracefunnel/store.go @@ -6,7 +6,7 @@ import ( "time" "github.com/SigNoz/signoz/pkg/sqlstore" - traceFunnels "github.com/SigNoz/signoz/pkg/types/tracefunnel" + traceFunnels "github.com/SigNoz/signoz/pkg/types/tracefunneltypes" "github.com/SigNoz/signoz/pkg/valuer" ) @@ -19,21 +19,36 @@ func NewStore(sqlstore sqlstore.SQLStore) traceFunnels.FunnelStore { } func (store *store) Create(ctx context.Context, funnel *traceFunnels.StorableFunnel) error { - _, err := store. + // Check if a funnel with the same name already exists in the organization + exists, err := store. + sqlstore. + BunDB(). + NewSelect(). + Model((*traceFunnels.StorableFunnel)(nil)). + Where("name = ? AND org_id = ?", funnel.Name, funnel.OrgID.String()). + Exists(ctx) + if err != nil { + return fmt.Errorf("failed to check for existing funnel: %v", err) + } + if exists { + return store.sqlstore.WrapAlreadyExistsErrf(nil, traceFunnels.ErrFunnelAlreadyExists, "a funnel with name '%s' already exists in this organization", funnel.Name) + } + + _, err = store. sqlstore. BunDB(). NewInsert(). Model(funnel). Exec(ctx) if err != nil { - return store.sqlstore.WrapAlreadyExistsErrf(err, traceFunnels.ErrFunnelAlreadyExists, "a funnel with name '%s' already exists in this organization", funnel.Name) + return fmt.Errorf("failed to create funnel: %v", err) } return nil } // Get retrieves a funnel by ID -func (store *store) Get(ctx context.Context, uuid valuer.UUID) (*traceFunnels.StorableFunnel, error) { +func (store *store) Get(ctx context.Context, uuid valuer.UUID, orgID valuer.UUID) (*traceFunnels.StorableFunnel, error) { funnel := &traceFunnels.StorableFunnel{} err := store. sqlstore. @@ -41,7 +56,7 @@ func (store *store) Get(ctx context.Context, uuid valuer.UUID) (*traceFunnels.St NewSelect(). Model(funnel). Relation("CreatedByUser"). - Where("?TableAlias.id = ?", uuid). + Where("?TableAlias.id = ? AND ?TableAlias.org_id = ?", uuid.String(), orgID.String()). Scan(ctx) if err != nil { return nil, fmt.Errorf("failed to get funnel: %v", err) @@ -75,7 +90,7 @@ func (store *store) List(ctx context.Context, orgID valuer.UUID) ([]*traceFunnel NewSelect(). Model(&funnels). Relation("CreatedByUser"). - Where("?TableAlias.org_id = ?", orgID). + Where("?TableAlias.org_id = ?", orgID.String()). Scan(ctx) if err != nil { return nil, fmt.Errorf("failed to list funnels: %v", err) @@ -84,13 +99,14 @@ func (store *store) List(ctx context.Context, orgID valuer.UUID) ([]*traceFunnel } // Delete removes a funnel by ID -func (store *store) Delete(ctx context.Context, uuid valuer.UUID) error { +func (store *store) Delete(ctx context.Context, funnelID valuer.UUID, orgID valuer.UUID) error { _, err := store. sqlstore. BunDB(). NewDelete(). Model((*traceFunnels.StorableFunnel)(nil)). - Where("id = ?", uuid).Exec(ctx) + Where("id = ? AND org_id = ?", funnelID.String(), orgID.String()). + Exec(ctx) if err != nil { return fmt.Errorf("failed to delete funnel: %v", err) } diff --git a/pkg/modules/tracefunnel/tracefunnel.go b/pkg/modules/tracefunnel/tracefunnel.go index 4e115b782b..e747c7a572 100644 --- a/pkg/modules/tracefunnel/tracefunnel.go +++ b/pkg/modules/tracefunnel/tracefunnel.go @@ -2,26 +2,25 @@ package tracefunnel import ( "context" + "github.com/SigNoz/signoz/pkg/valuer" "net/http" - traceFunnels "github.com/SigNoz/signoz/pkg/types/tracefunnel" + traceFunnels "github.com/SigNoz/signoz/pkg/types/tracefunneltypes" ) // Module defines the interface for trace funnel operations type Module interface { - Create(ctx context.Context, timestamp int64, name string, userID string, orgID string) (*traceFunnels.StorableFunnel, error) + Create(ctx context.Context, timestamp int64, name string, userID valuer.UUID, orgID valuer.UUID) (*traceFunnels.StorableFunnel, error) - Get(ctx context.Context, funnelID string) (*traceFunnels.StorableFunnel, error) + Get(ctx context.Context, funnelID valuer.UUID, orgID valuer.UUID) (*traceFunnels.StorableFunnel, error) - Update(ctx context.Context, funnel *traceFunnels.StorableFunnel, userID string) error + Update(ctx context.Context, funnel *traceFunnels.StorableFunnel, userID valuer.UUID) error - List(ctx context.Context, orgID string) ([]*traceFunnels.StorableFunnel, error) + List(ctx context.Context, orgID valuer.UUID) ([]*traceFunnels.StorableFunnel, error) - Delete(ctx context.Context, funnelID string) error + Delete(ctx context.Context, funnelID valuer.UUID, orgID valuer.UUID) error - Save(ctx context.Context, funnel *traceFunnels.StorableFunnel, userID string, orgID string) error - - GetFunnelMetadata(ctx context.Context, funnelID string) (int64, int64, string, error) + GetFunnelMetadata(ctx context.Context, funnelID valuer.UUID, orgID valuer.UUID) (int64, int64, string, error) } type Handler interface { diff --git a/pkg/modules/tracefunnel/tracefunneltest/module_test.go b/pkg/modules/tracefunnel/tracefunneltest/module_test.go index e88772541e..e368c9648f 100644 --- a/pkg/modules/tracefunnel/tracefunneltest/module_test.go +++ b/pkg/modules/tracefunnel/tracefunneltest/module_test.go @@ -2,12 +2,13 @@ package tracefunneltest import ( "context" - "github.com/SigNoz/signoz/pkg/modules/tracefunnel/impltracefunnel" "testing" "time" + "github.com/SigNoz/signoz/pkg/modules/tracefunnel/impltracefunnel" + "github.com/SigNoz/signoz/pkg/types" - traceFunnels "github.com/SigNoz/signoz/pkg/types/tracefunnel" + traceFunnels "github.com/SigNoz/signoz/pkg/types/tracefunneltypes" "github.com/SigNoz/signoz/pkg/valuer" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -22,8 +23,8 @@ func (m *MockStore) Create(ctx context.Context, funnel *traceFunnels.StorableFun return args.Error(0) } -func (m *MockStore) Get(ctx context.Context, uuid valuer.UUID) (*traceFunnels.StorableFunnel, error) { - args := m.Called(ctx, uuid) +func (m *MockStore) Get(ctx context.Context, uuid valuer.UUID, orgID valuer.UUID) (*traceFunnels.StorableFunnel, error) { + args := m.Called(ctx, uuid, orgID) return args.Get(0).(*traceFunnels.StorableFunnel), args.Error(1) } @@ -37,8 +38,8 @@ func (m *MockStore) Update(ctx context.Context, funnel *traceFunnels.StorableFun return args.Error(0) } -func (m *MockStore) Delete(ctx context.Context, uuid valuer.UUID) error { - args := m.Called(ctx, uuid) +func (m *MockStore) Delete(ctx context.Context, uuid valuer.UUID, orgID valuer.UUID) error { + args := m.Called(ctx, uuid, orgID) return args.Error(0) } @@ -50,23 +51,23 @@ func TestModule_Create(t *testing.T) { timestamp := time.Now().UnixMilli() name := "test-funnel" userID := valuer.GenerateUUID() - orgID := valuer.GenerateUUID().String() + orgID := valuer.GenerateUUID() mockStore.On("Create", ctx, mock.MatchedBy(func(f *traceFunnels.StorableFunnel) bool { return f.Name == name && f.CreatedBy == userID.String() && - f.OrgID.String() == orgID && + f.OrgID == orgID && f.CreatedByUser != nil && f.CreatedByUser.ID == userID && f.CreatedAt.UnixNano()/1000000 == timestamp })).Return(nil) - funnel, err := module.Create(ctx, timestamp, name, userID.String(), orgID) + funnel, err := module.Create(ctx, timestamp, name, userID, orgID) assert.NoError(t, err) assert.NotNil(t, funnel) assert.Equal(t, name, funnel.Name) assert.Equal(t, userID.String(), funnel.CreatedBy) - assert.Equal(t, orgID, funnel.OrgID.String()) + assert.Equal(t, orgID, funnel.OrgID) assert.NotNil(t, funnel.CreatedByUser) assert.Equal(t, userID, funnel.CreatedByUser.ID) @@ -78,16 +79,15 @@ func TestModule_Get(t *testing.T) { module := impltracefunnel.NewModule(mockStore) ctx := context.Background() - funnelID := valuer.GenerateUUID().String() + funnelID := valuer.GenerateUUID() + orgID := valuer.GenerateUUID() expectedFunnel := &traceFunnels.StorableFunnel{ - BaseMetadata: traceFunnels.BaseMetadata{ - Name: "test-funnel", - }, + Name: "test-funnel", } - mockStore.On("Get", ctx, mock.AnythingOfType("valuer.UUID")).Return(expectedFunnel, nil) + mockStore.On("Get", ctx, funnelID, orgID).Return(expectedFunnel, nil) - funnel, err := module.Get(ctx, funnelID) + funnel, err := module.Get(ctx, funnelID, orgID) assert.NoError(t, err) assert.Equal(t, expectedFunnel, funnel) @@ -99,18 +99,16 @@ func TestModule_Update(t *testing.T) { module := impltracefunnel.NewModule(mockStore) ctx := context.Background() - userID := "user-123" + userID := valuer.GenerateUUID() funnel := &traceFunnels.StorableFunnel{ - BaseMetadata: traceFunnels.BaseMetadata{ - Name: "test-funnel", - }, + Name: "test-funnel", } mockStore.On("Update", ctx, funnel).Return(nil) err := module.Update(ctx, funnel, userID) assert.NoError(t, err) - assert.Equal(t, userID, funnel.UpdatedBy) + assert.Equal(t, userID.String(), funnel.UpdatedBy) mockStore.AssertExpectations(t) } @@ -120,24 +118,19 @@ func TestModule_List(t *testing.T) { module := impltracefunnel.NewModule(mockStore) ctx := context.Background() - orgID := valuer.GenerateUUID().String() - orgUUID := valuer.MustNewUUID(orgID) + orgID := valuer.GenerateUUID() expectedFunnels := []*traceFunnels.StorableFunnel{ { - BaseMetadata: traceFunnels.BaseMetadata{ - Name: "funnel-1", - OrgID: orgUUID, - }, + Name: "funnel-1", + OrgID: orgID, }, { - BaseMetadata: traceFunnels.BaseMetadata{ - Name: "funnel-2", - OrgID: orgUUID, - }, + Name: "funnel-2", + OrgID: orgID, }, } - mockStore.On("List", ctx, orgUUID).Return(expectedFunnels, nil) + mockStore.On("List", ctx, orgID).Return(expectedFunnels, nil) funnels, err := module.List(ctx, orgID) assert.NoError(t, err) @@ -152,59 +145,36 @@ func TestModule_Delete(t *testing.T) { module := impltracefunnel.NewModule(mockStore) ctx := context.Background() - funnelID := valuer.GenerateUUID().String() + funnelID := valuer.GenerateUUID() + orgID := valuer.GenerateUUID() - mockStore.On("Delete", ctx, mock.AnythingOfType("valuer.UUID")).Return(nil) + mockStore.On("Delete", ctx, funnelID, orgID).Return(nil) - err := module.Delete(ctx, funnelID) + err := module.Delete(ctx, funnelID, orgID) assert.NoError(t, err) mockStore.AssertExpectations(t) } -func TestModule_Save(t *testing.T) { - mockStore := new(MockStore) - module := impltracefunnel.NewModule(mockStore) - - ctx := context.Background() - userID := "user-123" - orgID := valuer.GenerateUUID().String() - funnel := &traceFunnels.StorableFunnel{ - BaseMetadata: traceFunnels.BaseMetadata{ - Name: "test-funnel", - }, - } - - mockStore.On("Update", ctx, funnel).Return(nil) - - err := module.Save(ctx, funnel, userID, orgID) - assert.NoError(t, err) - assert.Equal(t, userID, funnel.UpdatedBy) - assert.Equal(t, orgID, funnel.OrgID.String()) - - mockStore.AssertExpectations(t) -} - func TestModule_GetFunnelMetadata(t *testing.T) { mockStore := new(MockStore) module := impltracefunnel.NewModule(mockStore) ctx := context.Background() - funnelID := valuer.GenerateUUID().String() + funnelID := valuer.GenerateUUID() + orgID := valuer.GenerateUUID() now := time.Now() expectedFunnel := &traceFunnels.StorableFunnel{ - BaseMetadata: traceFunnels.BaseMetadata{ - Description: "test description", - TimeAuditable: types.TimeAuditable{ - CreatedAt: now, - UpdatedAt: now, - }, + Description: "test description", + TimeAuditable: types.TimeAuditable{ + CreatedAt: now, + UpdatedAt: now, }, } - mockStore.On("Get", ctx, mock.AnythingOfType("valuer.UUID")).Return(expectedFunnel, nil) + mockStore.On("Get", ctx, funnelID, orgID).Return(expectedFunnel, nil) - createdAt, updatedAt, description, err := module.GetFunnelMetadata(ctx, funnelID) + createdAt, updatedAt, description, err := module.GetFunnelMetadata(ctx, funnelID, orgID) assert.NoError(t, err) assert.Equal(t, now.UnixNano()/1000000, createdAt) assert.Equal(t, now.UnixNano()/1000000, updatedAt) diff --git a/pkg/query-service/app/http_handler.go b/pkg/query-service/app/http_handler.go index 204b1ed684..794f2fdc49 100644 --- a/pkg/query-service/app/http_handler.go +++ b/pkg/query-service/app/http_handler.go @@ -5233,19 +5233,15 @@ func (aH *APIHandler) getDomainInfo(w http.ResponseWriter, r *http.Request) { // RegisterTraceFunnelsRoutes adds trace funnels routes func (aH *APIHandler) RegisterTraceFunnelsRoutes(router *mux.Router, am *middleware.AuthZ) { // Main trace funnels router - traceFunnelsRouter := router.PathPrefix("/api/v1/trace-funnels").Subrouter() + traceFunnelsRouter := router.PathPrefix("/api/v1/orgs/me/trace-funnels").Subrouter() // API endpoints - traceFunnelsRouter.HandleFunc("/new", + traceFunnelsRouter.HandleFunc("", am.ViewAccess(aH.Signoz.Handlers.TraceFunnel.New)). Methods(http.MethodPost) - traceFunnelsRouter.HandleFunc("/list", + traceFunnelsRouter.HandleFunc("", am.ViewAccess(aH.Signoz.Handlers.TraceFunnel.List)). Methods(http.MethodGet) - traceFunnelsRouter.HandleFunc("/steps/update", - am.ViewAccess(aH.Signoz.Handlers.TraceFunnel.UpdateSteps)). - Methods(http.MethodPut) - traceFunnelsRouter.HandleFunc("/{funnel_id}", am.ViewAccess(aH.Signoz.Handlers.TraceFunnel.Get)). Methods(http.MethodGet) diff --git a/pkg/sqlmigration/040_add_trace_funnels.go b/pkg/sqlmigration/040_add_trace_funnels.go index e252985400..22cb7ae57c 100644 --- a/pkg/sqlmigration/040_add_trace_funnels.go +++ b/pkg/sqlmigration/040_add_trace_funnels.go @@ -3,6 +3,7 @@ package sqlmigration import ( "context" "fmt" + "github.com/SigNoz/signoz/pkg/factory" v3 "github.com/SigNoz/signoz/pkg/query-service/model/v3" "github.com/SigNoz/signoz/pkg/sqlstore" @@ -12,19 +13,15 @@ import ( "github.com/uptrace/bun/migrate" ) -type BaseMetadata struct { +// Funnel Core Data Structure (Funnel and FunnelStep) +type Funnel struct { + bun.BaseModel `bun:"table:trace_funnel"` types.Identifiable // funnel id types.TimeAuditable types.UserAuditable - Name string `json:"funnel_name" bun:"name,type:text,notnull"` // funnel name - Description string `json:"description" bun:"description,type:text"` // funnel description - OrgID valuer.UUID `json:"org_id" bun:"org_id,type:varchar,notnull"` -} - -// Funnel Core Data Structure (Funnel and FunnelStep) -type Funnel struct { - bun.BaseModel `bun:"table:trace_funnel"` - BaseMetadata + Name string `json:"funnel_name" bun:"name,type:text,notnull"` // funnel name + Description string `json:"description" bun:"description,type:text"` // funnel description + OrgID valuer.UUID `json:"org_id" bun:"org_id,type:varchar,notnull"` Steps []FunnelStep `json:"steps" bun:"steps,type:text,notnull"` Tags string `json:"tags" bun:"tags,type:text"` CreatedByUser *types.User `json:"user" bun:"rel:belongs-to,join:created_by=id"` diff --git a/pkg/types/tracefunnel/store.go b/pkg/types/tracefunneltypes/store.go similarity index 62% rename from pkg/types/tracefunnel/store.go rename to pkg/types/tracefunneltypes/store.go index 44dbf7da4e..ba000410e0 100644 --- a/pkg/types/tracefunnel/store.go +++ b/pkg/types/tracefunneltypes/store.go @@ -1,4 +1,4 @@ -package tracefunnel +package tracefunneltypes import ( "context" @@ -8,8 +8,8 @@ import ( type FunnelStore interface { Create(context.Context, *StorableFunnel) error - Get(context.Context, valuer.UUID) (*StorableFunnel, error) + Get(context.Context, valuer.UUID, valuer.UUID) (*StorableFunnel, error) List(context.Context, valuer.UUID) ([]*StorableFunnel, error) Update(context.Context, *StorableFunnel) error - Delete(context.Context, valuer.UUID) error + Delete(context.Context, valuer.UUID, valuer.UUID) error } diff --git a/pkg/types/tracefunnel/tracefunnel.go b/pkg/types/tracefunneltypes/tracefunnel.go similarity index 87% rename from pkg/types/tracefunnel/tracefunnel.go rename to pkg/types/tracefunneltypes/tracefunnel.go index 07c863f3e2..3c6b664922 100644 --- a/pkg/types/tracefunnel/tracefunnel.go +++ b/pkg/types/tracefunneltypes/tracefunnel.go @@ -1,4 +1,4 @@ -package tracefunnel +package tracefunneltypes import ( "github.com/SigNoz/signoz/pkg/errors" @@ -12,20 +12,18 @@ var ( ErrFunnelAlreadyExists = errors.MustNewCode("funnel_already_exists") ) -// BaseMetadata metadata for funnels type BaseMetadata struct { - types.Identifiable // funnel id - types.TimeAuditable - types.UserAuditable - Name string `json:"funnel_name" bun:"name,type:text,notnull"` // funnel name - Description string `json:"description" bun:"description,type:text"` // funnel description - OrgID valuer.UUID `json:"org_id" bun:"org_id,type:varchar,notnull"` } // StorableFunnel Core Data Structure (StorableFunnel and FunnelStep) type StorableFunnel struct { + types.Identifiable + types.TimeAuditable + types.UserAuditable bun.BaseModel `bun:"table:trace_funnel"` - BaseMetadata + Name string `json:"funnel_name" bun:"name,type:text,notnull"` + Description string `json:"description" bun:"description,type:text"` + OrgID valuer.UUID `json:"org_id" bun:"org_id,type:varchar,notnull"` Steps []*FunnelStep `json:"steps" bun:"steps,type:text,notnull"` Tags string `json:"tags" bun:"tags,type:text"` CreatedByUser *types.User `json:"user" bun:"rel:belongs-to,join:created_by=id"` @@ -84,8 +82,8 @@ type TimeRange struct { // StepTransitionRequest represents a request for step transition analytics type StepTransitionRequest struct { TimeRange - StepAOrder int64 `json:"step_start,omitempty"` - StepBOrder int64 `json:"step_end,omitempty"` + StepStart int64 `json:"step_start,omitempty"` + StepEnd int64 `json:"step_end,omitempty"` } // UserInfo represents basic user information diff --git a/pkg/types/tracefunnel/utils.go b/pkg/types/tracefunneltypes/utils.go similarity index 79% rename from pkg/types/tracefunnel/utils.go rename to pkg/types/tracefunneltypes/utils.go index 38cd0c2b7b..2b90720d75 100644 --- a/pkg/types/tracefunnel/utils.go +++ b/pkg/types/tracefunneltypes/utils.go @@ -1,4 +1,4 @@ -package tracefunnel +package tracefunneltypes import ( "fmt" @@ -48,14 +48,33 @@ func ValidateFunnelSteps(steps []*FunnelStep) error { } // NormalizeFunnelSteps normalizes step orders to be sequential starting from 1. -// Returns a new slice with normalized step orders, leaving the input slice unchanged. +// The function takes a slice of pointers to FunnelStep and returns a new slice with normalized step orders. +// The input slice is left unchanged. If the input slice contains nil pointers, they will be filtered out. +// Returns an empty slice if the input is empty or contains only nil pointers. func NormalizeFunnelSteps(steps []*FunnelStep) []*FunnelStep { if len(steps) == 0 { return []*FunnelStep{} } - newSteps := make([]*FunnelStep, len(steps)) - copy(newSteps, steps) + // Filter out nil pointers and create a new slice + validSteps := make([]*FunnelStep, 0, len(steps)) + for _, step := range steps { + if step != nil { + validSteps = append(validSteps, step) + } + } + + if len(validSteps) == 0 { + return []*FunnelStep{} + } + + // Create a defensive copy of the valid steps + newSteps := make([]*FunnelStep, len(validSteps)) + for i, step := range validSteps { + // Create a copy of each step to avoid modifying the original + stepCopy := *step + newSteps[i] = &stepCopy + } sort.Slice(newSteps, func(i, j int) bool { return newSteps[i].Order < newSteps[j].Order diff --git a/pkg/types/tracefunnel/utils_test.go b/pkg/types/tracefunneltypes/utils_test.go similarity index 84% rename from pkg/types/tracefunnel/utils_test.go rename to pkg/types/tracefunneltypes/utils_test.go index 3ce4e691db..8ebc4cd347 100644 --- a/pkg/types/tracefunnel/utils_test.go +++ b/pkg/types/tracefunneltypes/utils_test.go @@ -1,4 +1,4 @@ -package tracefunnel +package tracefunneltypes import ( "net/http" @@ -90,12 +90,12 @@ func TestValidateTimestampIsMilliseconds(t *testing.T) { func TestValidateFunnelSteps(t *testing.T) { tests := []struct { name string - steps []FunnelStep + steps []*FunnelStep expectError bool }{ { name: "valid steps", - steps: []FunnelStep{ + steps: []*FunnelStep{ { ID: valuer.GenerateUUID(), Name: "Step 1", @@ -115,7 +115,7 @@ func TestValidateFunnelSteps(t *testing.T) { }, { name: "too few steps", - steps: []FunnelStep{ + steps: []*FunnelStep{ { ID: valuer.GenerateUUID(), Name: "Step 1", @@ -128,7 +128,7 @@ func TestValidateFunnelSteps(t *testing.T) { }, { name: "missing service name", - steps: []FunnelStep{ + steps: []*FunnelStep{ { ID: valuer.GenerateUUID(), Name: "Step 1", @@ -147,7 +147,7 @@ func TestValidateFunnelSteps(t *testing.T) { }, { name: "missing span name", - steps: []FunnelStep{ + steps: []*FunnelStep{ { ID: valuer.GenerateUUID(), Name: "Step 1", @@ -166,7 +166,7 @@ func TestValidateFunnelSteps(t *testing.T) { }, { name: "negative order", - steps: []FunnelStep{ + steps: []*FunnelStep{ { ID: valuer.GenerateUUID(), Name: "Step 1", @@ -201,12 +201,12 @@ func TestValidateFunnelSteps(t *testing.T) { func TestNormalizeFunnelSteps(t *testing.T) { tests := []struct { name string - steps []FunnelStep - expected []FunnelStep + steps []*FunnelStep + expected []*FunnelStep }{ { name: "already normalized steps", - steps: []FunnelStep{ + steps: []*FunnelStep{ { ID: valuer.GenerateUUID(), Name: "Step 1", @@ -222,7 +222,7 @@ func TestNormalizeFunnelSteps(t *testing.T) { Order: 2, }, }, - expected: []FunnelStep{ + expected: []*FunnelStep{ { Name: "Step 1", ServiceName: "test-service", @@ -239,7 +239,7 @@ func TestNormalizeFunnelSteps(t *testing.T) { }, { name: "unordered steps", - steps: []FunnelStep{ + steps: []*FunnelStep{ { ID: valuer.GenerateUUID(), Name: "Step 2", @@ -255,7 +255,7 @@ func TestNormalizeFunnelSteps(t *testing.T) { Order: 1, }, }, - expected: []FunnelStep{ + expected: []*FunnelStep{ { Name: "Step 1", ServiceName: "test-service", @@ -272,7 +272,7 @@ func TestNormalizeFunnelSteps(t *testing.T) { }, { name: "steps with gaps in order", - steps: []FunnelStep{ + steps: []*FunnelStep{ { ID: valuer.GenerateUUID(), Name: "Step 1", @@ -295,7 +295,7 @@ func TestNormalizeFunnelSteps(t *testing.T) { Order: 2, }, }, - expected: []FunnelStep{ + expected: []*FunnelStep{ { Name: "Step 1", ServiceName: "test-service", @@ -316,17 +316,58 @@ func TestNormalizeFunnelSteps(t *testing.T) { }, }, }, + { + name: "steps with nil pointers", + steps: []*FunnelStep{ + { + ID: valuer.GenerateUUID(), + Name: "Step 1", + ServiceName: "test-service", + SpanName: "test-span", + Order: 1, + }, + nil, + { + ID: valuer.GenerateUUID(), + Name: "Step 2", + ServiceName: "test-service", + SpanName: "test-span-2", + Order: 2, + }, + }, + expected: []*FunnelStep{ + { + Name: "Step 1", + ServiceName: "test-service", + SpanName: "test-span", + Order: 1, + }, + { + Name: "Step 2", + ServiceName: "test-service", + SpanName: "test-span-2", + Order: 2, + }, + }, + }, + { + name: "empty steps", + steps: []*FunnelStep{}, + expected: []*FunnelStep{}, + }, + { + name: "all nil steps", + steps: []*FunnelStep{nil, nil}, + expected: []*FunnelStep{}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Make a copy of the steps to avoid modifying the original - steps := make([]FunnelStep, len(tt.steps)) - copy(steps, tt.steps) - - result := NormalizeFunnelSteps(steps) + result := NormalizeFunnelSteps(tt.steps) // Compare only the relevant fields + assert.Len(t, result, len(tt.expected)) for i := range result { assert.Equal(t, tt.expected[i].Name, result[i].Name) assert.Equal(t, tt.expected[i].ServiceName, result[i].ServiceName) @@ -425,6 +466,7 @@ func TestConstructFunnelResponse(t *testing.T) { now := time.Now() funnelID := valuer.GenerateUUID() orgID := valuer.GenerateUUID() + userID := valuer.GenerateUUID() tests := []struct { name string @@ -435,24 +477,24 @@ func TestConstructFunnelResponse(t *testing.T) { { name: "with user email from funnel", funnel: &StorableFunnel{ - BaseMetadata: BaseMetadata{ - Identifiable: types.Identifiable{ - ID: funnelID, - }, - Name: "test-funnel", - OrgID: orgID, - TimeAuditable: types.TimeAuditable{ - CreatedAt: now, - UpdatedAt: now, - }, - UserAuditable: types.UserAuditable{ - CreatedBy: "user-123", - UpdatedBy: "user-123", - }, + Identifiable: types.Identifiable{ + ID: funnelID, }, + TimeAuditable: types.TimeAuditable{ + CreatedAt: now, + UpdatedAt: now, + }, + UserAuditable: types.UserAuditable{ + CreatedBy: userID.String(), + UpdatedBy: userID.String(), + }, + Name: "test-funnel", + OrgID: orgID, CreatedByUser: &types.User{ - Identifiable: types.Identifiable{ID: valuer.MustNewUUID("user-123")}, - Email: "funnel@example.com", + Identifiable: types.Identifiable{ + ID: userID, + }, + Email: "funnel@example.com", }, Steps: []*FunnelStep{ { @@ -465,7 +507,7 @@ func TestConstructFunnelResponse(t *testing.T) { }, }, claims: &authtypes.Claims{ - UserID: "user-123", + UserID: userID.String(), OrgID: orgID.String(), Email: "claims@example.com", }, @@ -481,9 +523,9 @@ func TestConstructFunnelResponse(t *testing.T) { }, }, CreatedAt: now.UnixNano() / 1000000, - CreatedBy: "user-123", + CreatedBy: userID.String(), UpdatedAt: now.UnixNano() / 1000000, - UpdatedBy: "user-123", + UpdatedBy: userID.String(), OrgID: orgID.String(), UserEmail: "funnel@example.com", }, @@ -491,21 +533,19 @@ func TestConstructFunnelResponse(t *testing.T) { { name: "with user email from claims", funnel: &StorableFunnel{ - BaseMetadata: BaseMetadata{ - Identifiable: types.Identifiable{ - ID: funnelID, - }, - Name: "test-funnel", - OrgID: orgID, - TimeAuditable: types.TimeAuditable{ - CreatedAt: now, - UpdatedAt: now, - }, - UserAuditable: types.UserAuditable{ - CreatedBy: "user-123", - UpdatedBy: "user-123", - }, + Identifiable: types.Identifiable{ + ID: funnelID, }, + TimeAuditable: types.TimeAuditable{ + CreatedAt: now, + UpdatedAt: now, + }, + UserAuditable: types.UserAuditable{ + CreatedBy: userID.String(), + UpdatedBy: userID.String(), + }, + Name: "test-funnel", + OrgID: orgID, Steps: []*FunnelStep{ { ID: valuer.GenerateUUID(), @@ -517,7 +557,7 @@ func TestConstructFunnelResponse(t *testing.T) { }, }, claims: &authtypes.Claims{ - UserID: "user-123", + UserID: userID.String(), OrgID: orgID.String(), Email: "claims@example.com", }, @@ -533,9 +573,9 @@ func TestConstructFunnelResponse(t *testing.T) { }, }, CreatedAt: now.UnixNano() / 1000000, - CreatedBy: "user-123", + CreatedBy: userID.String(), UpdatedAt: now.UnixNano() / 1000000, - UpdatedBy: "user-123", + UpdatedBy: userID.String(), OrgID: orgID.String(), UserEmail: "claims@example.com", }, @@ -572,12 +612,12 @@ func TestConstructFunnelResponse(t *testing.T) { func TestProcessFunnelSteps(t *testing.T) { tests := []struct { name string - steps []FunnelStep + steps []*FunnelStep expectError bool }{ { name: "valid steps with missing IDs", - steps: []FunnelStep{ + steps: []*FunnelStep{ { Name: "Step 1", ServiceName: "test-service", @@ -595,7 +635,7 @@ func TestProcessFunnelSteps(t *testing.T) { }, { name: "invalid steps - missing service name", - steps: []FunnelStep{ + steps: []*FunnelStep{ { Name: "Step 1", SpanName: "test-span", @@ -612,7 +652,7 @@ func TestProcessFunnelSteps(t *testing.T) { }, { name: "invalid steps - negative order", - steps: []FunnelStep{ + steps: []*FunnelStep{ { Name: "Step 1", ServiceName: "test-service",