feat(authz): update integration tests

frontend/src/components/ServiceAccountDrawer/OverviewTab.tsx

@@ -16,8 +16,8 @@ interface OverviewTabProps {
 	account: ServiceAccountRow;
 	localName: string;
 	onNameChange: (v: string) => void;
-	localRole: string;
-	onRoleChange: (v: string | undefined) => void;
+	localRoles: string[];
+	onRolesChange: (v: string[]) => void;
 	isDisabled: boolean;
 	availableRoles: AuthtypesRoleDTO[];
 	rolesLoading?: boolean;
@@ -31,8 +31,8 @@ function OverviewTab({
 	account,
 	localName,
 	onNameChange,
-	localRole,
-	onRoleChange,
+	localRoles,
+	onRolesChange,
 	isDisabled,
 	availableRoles,
 	rolesLoading,
@@ -95,10 +95,15 @@ function OverviewTab({
 				{isDisabled ? (
 					<div className="sa-drawer__input-wrapper sa-drawer__input-wrapper--disabled">
 						<div className="sa-drawer__disabled-roles">
-							{localRole ? (
-								<Badge color="vanilla">
-									{availableRoles.find((r) => r.id === localRole)?.name ?? localRole}
-								</Badge>
+							{localRoles.length > 0 ? (
+								localRoles.map((roleId) => {
+									const role = availableRoles.find((r) => r.id === roleId);
+									return (
+										<Badge key={roleId} color="vanilla">
+											{role?.name ?? roleId}
+										</Badge>
+									);
+								})
 							) : (
 								<span className="sa-drawer__input-text">—</span>
 							)}
@@ -108,14 +113,15 @@ function OverviewTab({
 				) : (
 					<RolesSelect
 						id="sa-roles"
+						mode="multiple"
 						roles={availableRoles}
 						loading={rolesLoading}
 						isError={rolesError}
 						error={rolesErrorObj}
 						onRefetch={onRefetchRoles}
-						value={localRole}
-						onChange={onRoleChange}
-						placeholder="Select role"
+						value={localRoles}
+						onChange={onRolesChange}
+						placeholder="Select roles"
 					/>
 				)}
 			</div>

frontend/src/components/ServiceAccountDrawer/ServiceAccountDrawer.tsx

@@ -8,9 +8,7 @@ import { ToggleGroup, ToggleGroupItem } from '@signozhq/ui/toggle-group';
 import { Pagination, Skeleton } from 'antd';
 import { convertToApiError } from 'api/ErrorResponseHandlerForGeneratedAPIs';
 import {
-	getGetServiceAccountRolesQueryKey,
 	getListServiceAccountsQueryKey,
-	useDeleteServiceAccountRole,
 	useGetServiceAccount,
 	useListServiceAccountKeys,
 	useUpdateServiceAccount,
@@ -37,7 +35,7 @@ import {
 	useQueryState,
 } from 'nuqs';
 import APIError from 'types/api/error';
-import { retryOn429, toAPIError } from 'utils/errorUtils';
+import { toAPIError } from 'utils/errorUtils';
 
 import AddKeyModal from './AddKeyModal';
 import DeleteAccountModal from './DeleteAccountModal';
@@ -92,7 +90,7 @@ function ServiceAccountDrawer({
 		parseAsBoolean.withDefault(false),
 	);
 	const [localName, setLocalName] = useState('');
-	const [localRole, setLocalRole] = useState('');
+	const [localRoles, setLocalRoles] = useState<string[]>([]);
 	const [isSaving, setIsSaving] = useState(false);
 	const [saveErrors, setSaveErrors] = useState<SaveError[]>([]);
 
@@ -140,7 +138,7 @@ function ServiceAccountDrawer({
 		if (!account?.id) {
 			roleSessionRef.current = null;
 		} else if (account.id !== roleSessionRef.current && !isRolesLoading) {
-			setLocalRole(currentRoles[0]?.id ?? '');
+			setLocalRoles(currentRoles.map((r) => r.id).filter(Boolean) as string[]);
 			roleSessionRef.current = account.id;
 		}
 	}, [account?.id, currentRoles, isRolesLoading]);
@@ -151,7 +149,13 @@ function ServiceAccountDrawer({
 	const isDirty =
 		account !== null &&
 		(localName !== (account.name ?? '') ||
-			localRole !== (currentRoles[0]?.id ?? ''));
+			JSON.stringify([...localRoles].sort()) !==
+				JSON.stringify(
+					currentRoles
+						.map((r) => r.id)
+						.filter(Boolean)
+						.sort(),
+				));
 
 	const {
 		roles: availableRoles,
@@ -179,27 +183,6 @@ function ServiceAccountDrawer({
 
 	// the retry for this mutation is safe due to the api being idempotent on backend
 	const { mutateAsync: updateMutateAsync } = useUpdateServiceAccount();
-	const { mutateAsync: deleteRole } = useDeleteServiceAccountRole({
-		mutation: {
-			retry: retryOn429,
-		},
-	});
-
-	const executeRolesOperation = useCallback(
-		async (accountId: string): Promise<RoleUpdateFailure[]> => {
-			if (localRole === '' && currentRoles[0]?.id) {
-				await deleteRole({
-					pathParams: { id: accountId, rid: currentRoles[0].id },
-				});
-				await queryClient.invalidateQueries(
-					getGetServiceAccountRolesQueryKey({ id: accountId }),
-				);
-				return [];
-			}
-			return applyDiff([localRole].filter(Boolean), availableRoles);
-		},
-		[localRole, currentRoles, availableRoles, applyDiff, deleteRole, queryClient],
-	);
 
 	const retryNameUpdate = useCallback(async (): Promise<void> => {
 		if (!account) {
@@ -267,7 +250,7 @@ function ServiceAccountDrawer({
 
 	const retryRolesUpdate = useCallback(async (): Promise<void> => {
 		try {
-			const failures = await executeRolesOperation(selectedAccountId ?? '');
+			const failures = await applyDiff([...localRoles], availableRoles);
 			if (failures.length === 0) {
 				setSaveErrors((prev) => prev.filter((e) => e.context !== 'Roles update'));
 			} else {
@@ -283,7 +266,7 @@ function ServiceAccountDrawer({
 				),
 			);
 		}
-	}, [selectedAccountId, executeRolesOperation, failuresToSaveErrors]);
+	}, [localRoles, availableRoles, applyDiff, failuresToSaveErrors]);
 
 	const handleSave = useCallback(async (): Promise<void> => {
 		if (!account || !isDirty) {
@@ -302,7 +285,7 @@ function ServiceAccountDrawer({
 
 			const [nameResult, rolesResult] = await Promise.allSettled([
 				namePromise,
-				executeRolesOperation(account.id),
+				applyDiff([...localRoles], availableRoles),
 			]);
 
 			const errors: SaveError[] = [];
@@ -343,8 +326,10 @@ function ServiceAccountDrawer({
 		account,
 		isDirty,
 		localName,
+		localRoles,
+		availableRoles,
 		updateMutateAsync,
-		executeRolesOperation,
+		applyDiff,
 		refetchAccount,
 		onSuccess,
 		queryClient,
@@ -443,9 +428,9 @@ function ServiceAccountDrawer({
 								account={account}
 								localName={localName}
 								onNameChange={handleNameChange}
-								localRole={localRole}
-								onRoleChange={(role): void => {
-									setLocalRole(role ?? '');
+								localRoles={localRoles}
+								onRolesChange={(roles): void => {
+									setLocalRoles(roles);
 									clearRoleErrors();
 								}}
 								isDisabled={isDeleted}

frontend/src/components/ServiceAccountDrawer/__tests__/ServiceAccountDrawer.test.tsx

@@ -151,7 +151,7 @@ describe('ServiceAccountDrawer', () => {
 		});
 	});
 
-	it('changing roles enables Save; clicking Save sends role add request without delete', async () => {
+	it('adding a role fires POST for the new role and no DELETE for existing roles', async () => {
 		const roleSpy = jest.fn();
 		const deleteSpy = jest.fn();
 		const user = userEvent.setup({ pointerEventsCheck: 0 });
@@ -171,6 +171,7 @@ describe('ServiceAccountDrawer', () => {
 
 		await screen.findByDisplayValue('CI Bot');
 
+		// Add signoz-viewer while keeping signoz-admin selected
 		await user.click(screen.getByLabelText('Roles'));
 		await user.click(await screen.findByTitle('signoz-viewer'));
 
@@ -188,6 +189,43 @@ describe('ServiceAccountDrawer', () => {
 		});
 	});
 
+	it('removing a role fires DELETE for the removed role and no POST', async () => {
+		const roleSpy = jest.fn();
+		const deleteSpy = jest.fn();
+		const user = userEvent.setup({ pointerEventsCheck: 0 });
+
+		server.use(
+			rest.post(SA_ROLES_ENDPOINT, async (req, res, ctx) => {
+				roleSpy(await req.json());
+				return res(ctx.status(200), ctx.json({ status: 'success', data: {} }));
+			}),
+			rest.delete(SA_ROLE_DELETE_ENDPOINT, (_, res, ctx) => {
+				deleteSpy();
+				return res(ctx.status(200), ctx.json({ status: 'success', data: {} }));
+			}),
+		);
+
+		renderDrawer();
+
+		await screen.findByDisplayValue('CI Bot');
+
+		// Remove the signoz-admin tag from the multi-select
+		const adminTag = await screen.findByTitle('signoz-admin');
+		const removeBtn = adminTag.querySelector(
+			'.ant-select-selection-item-remove',
+		) as Element;
+		await user.click(removeBtn);
+
+		const saveBtn = screen.getByRole('button', { name: /Save Changes/i });
+		await waitFor(() => expect(saveBtn).not.toBeDisabled());
+		await user.click(saveBtn);
+
+		await waitFor(() => {
+			expect(deleteSpy).toHaveBeenCalled();
+			expect(roleSpy).not.toHaveBeenCalled();
+		});
+	});
+
 	it('"Delete Service Account" opens confirm dialog; confirming sends delete request', async () => {
 		const deleteSpy = jest.fn();
 		const user = userEvent.setup({ pointerEventsCheck: 0 });

frontend/src/hooks/serviceAccount/useServiceAccountRoleManager.ts

@@ -3,6 +3,7 @@ import { useQueryClient } from 'react-query';
 import {
 	getGetServiceAccountRolesQueryKey,
 	useCreateServiceAccountRole,
+	useDeleteServiceAccountRole,
 	useGetServiceAccountRoles,
 } from 'api/generated/services/serviceaccount';
 import type { AuthtypesRoleDTO } from 'api/generated/services/sigNoz.schemas';
@@ -44,6 +45,9 @@ export function useServiceAccountRoleManager(
 	const { mutateAsync: createRole } = useCreateServiceAccountRole({
 		mutation: { retry: retryOn429 },
 	});
+	const { mutateAsync: deleteRole } = useDeleteServiceAccountRole({
+		mutation: { retry: retryOn429 },
+	});
 
 	const invalidateRoles = useCallback(
 		() =>
@@ -68,14 +72,21 @@ export function useServiceAccountRoleManager(
 			const addedRoles = availableRoles.filter(
 				(r) => r.id && desiredRoleIds.has(r.id) && !currentRoleIds.has(r.id),
 			);
+			const removedRoles = currentRoles.filter(
+				(r) => r.id && !desiredRoleIds.has(r.id),
+			);
 
-			// TODO: re-enable deletes once BE for this is streamlined
 			const allOperations = [
 				...addedRoles.map((role) => ({
 					role,
 					run: (): ReturnType<typeof createRole> =>
 						createRole({ pathParams: { id: accountId }, data: { id: role.id } }),
 				})),
+				...removedRoles.map((role) => ({
+					role,
+					run: (): ReturnType<typeof deleteRole> =>
+						deleteRole({ pathParams: { id: accountId, rid: role.id ?? '' } }),
+				})),
 			];
 
 			const results = await Promise.allSettled(
@@ -106,7 +117,7 @@ export function useServiceAccountRoleManager(
 
 			return failures;
 		},
-		[accountId, currentRoles, createRole, invalidateRoles],
+		[accountId, currentRoles, createRole, deleteRole, invalidateRoles],
 	);
 
 	return {

pkg/modules/serviceaccount/implserviceaccount/module.go

@@ -377,7 +377,7 @@ func (module *module) getOrGetSetIdentity(ctx context.Context, serviceAccountID
 }
 
 func (module *module) setRole(ctx context.Context, orgID valuer.UUID, id valuer.UUID, role *authtypes.Role) error {
-	serviceAccount, err := module.GetWithRoles(ctx, orgID, id)
+	serviceAccount, err := module.Get(ctx, orgID, id)
 	if err != nil {
 		return err
 	}
@@ -387,24 +387,12 @@ func (module *module) setRole(ctx context.Context, orgID valuer.UUID, id valuer.
 		return err
 	}
 
-	err = module.authz.ModifyGrant(ctx, orgID, serviceAccount.RoleNames(), []string{role.Name}, authtypes.MustNewSubject(coretypes.NewResourceServiceAccount(), id.String(), orgID, nil))
+	err = module.authz.Grant(ctx, orgID, []string{role.Name}, authtypes.MustNewSubject(coretypes.NewResourceServiceAccount(), id.String(), orgID, nil))
 	if err != nil {
 		return err
 	}
 
-	err = module.store.RunInTx(ctx, func(ctx context.Context) error {
-		err = module.store.DeleteServiceAccountRoles(ctx, serviceAccount.ID)
-		if err != nil {
-			return err
-		}
-
-		err = module.store.CreateServiceAccountRole(ctx, serviceAccountRole)
-		if err != nil {
-			return err
-		}
-
-		return nil
-	})
+	err = module.store.CreateServiceAccountRole(ctx, serviceAccountRole)
 	if err != nil {
 		return err
 	}

pkg/modules/serviceaccount/implserviceaccount/store.go

@@ -207,21 +207,6 @@ func (store *store) CreateServiceAccountRole(ctx context.Context, serviceAccount
 	return nil
 }
 
-func (store *store) DeleteServiceAccountRoles(ctx context.Context, serviceAccountID valuer.UUID) error {
-	_, err := store.
-		sqlstore.
-		BunDBCtx(ctx).
-		NewDelete().
-		Model(new(serviceaccounttypes.ServiceAccountRole)).
-		Where("service_account_id = ?", serviceAccountID).
-		Exec(ctx)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
 func (store *store) DeleteServiceAccountRole(ctx context.Context, serviceAccountID valuer.UUID, roleID valuer.UUID) error {
 	_, err := store.
 		sqlstore.

pkg/modules/tag/impltag/module.go

@@ -1,57 +0,0 @@
-package impltag
-
-import (
-	"context"
-
-	"github.com/SigNoz/signoz/pkg/modules/tag"
-	"github.com/SigNoz/signoz/pkg/types/tagtypes"
-	"github.com/SigNoz/signoz/pkg/valuer"
-)
-
-type module struct {
-	store tagtypes.Store
-}
-
-func NewModule(store tagtypes.Store) tag.Module {
-	return &module{store: store}
-}
-
-func (m *module) CreateMany(ctx context.Context, orgID valuer.UUID, entityType tagtypes.EntityType, postable []tagtypes.PostableTag, createdBy string) ([]*tagtypes.Tag, error) {
-	if len(postable) == 0 {
-		return []*tagtypes.Tag{}, nil
-	}
-
-	toCreate, matched, err := tagtypes.Resolve(ctx, m.store, orgID, entityType, postable, createdBy)
-	if err != nil {
-		return nil, err
-	}
-
-	created, err := m.store.Create(ctx, toCreate)
-	if err != nil {
-		return nil, err
-	}
-
-	return append(matched, created...), nil
-}
-
-func (m *module) LinkToEntity(ctx context.Context, orgID valuer.UUID, entityType tagtypes.EntityType, entityID valuer.UUID, tagIDs []valuer.UUID) error {
-	if len(tagIDs) == 0 {
-		return nil
-	}
-	return m.store.CreateRelations(ctx, tagtypes.NewTagRelations(orgID, entityType, entityID, tagIDs))
-}
-
-func (m *module) SyncLinksForEntity(ctx context.Context, orgID valuer.UUID, entityType tagtypes.EntityType, entityID valuer.UUID, tagIDs []valuer.UUID) error {
-	if err := m.store.CreateRelations(ctx, tagtypes.NewTagRelations(orgID, entityType, entityID, tagIDs)); err != nil {
-		return err
-	}
-	return m.store.DeleteRelationsExcept(ctx, entityType, entityID, tagIDs)
-}
-
-func (m *module) ListForEntity(ctx context.Context, entityType tagtypes.EntityType, entityID valuer.UUID) ([]*tagtypes.Tag, error) {
-	return m.store.ListByEntity(ctx, entityType, entityID)
-}
-
-func (m *module) ListForEntities(ctx context.Context, entityType tagtypes.EntityType, entityIDs []valuer.UUID) (map[valuer.UUID][]*tagtypes.Tag, error) {
-	return m.store.ListByEntities(ctx, entityType, entityIDs)
-}

pkg/modules/tag/impltag/store.go

@@ -1,132 +0,0 @@
-package impltag
-
-import (
-	"context"
-
-	"github.com/SigNoz/signoz/pkg/sqlstore"
-	"github.com/SigNoz/signoz/pkg/types/tagtypes"
-	"github.com/SigNoz/signoz/pkg/valuer"
-	"github.com/uptrace/bun"
-)
-
-type store struct {
-	sqlstore sqlstore.SQLStore
-}
-
-func NewStore(sqlstore sqlstore.SQLStore) tagtypes.Store {
-	return &store{sqlstore: sqlstore}
-}
-
-func (s *store) List(ctx context.Context, orgID valuer.UUID, entityType tagtypes.EntityType) ([]*tagtypes.Tag, error) {
-	tags := make([]*tagtypes.Tag, 0)
-	err := s.sqlstore.
-		BunDBCtx(ctx).
-		NewSelect().
-		Model(&tags).
-		Where("org_id = ?", orgID).
-		Where("entity_type = ?", entityType).
-		Scan(ctx)
-	if err != nil {
-		return nil, err
-	}
-	return tags, nil
-}
-
-func (s *store) ListByEntity(ctx context.Context, entityType tagtypes.EntityType, entityID valuer.UUID) ([]*tagtypes.Tag, error) {
-	tags := make([]*tagtypes.Tag, 0)
-	err := s.sqlstore.
-		BunDBCtx(ctx).
-		NewSelect().
-		Model(&tags).
-		Join("JOIN tag_relations AS tr ON tr.tag_id = tag.id").
-		Where("tr.entity_type = ?", entityType).
-		Where("tr.entity_id = ?", entityID).
-		Scan(ctx)
-	if err != nil {
-		return nil, err
-	}
-	return tags, nil
-}
-
-func (s *store) ListByEntities(ctx context.Context, entityType tagtypes.EntityType, entityIDs []valuer.UUID) (map[valuer.UUID][]*tagtypes.Tag, error) {
-	if len(entityIDs) == 0 {
-		return map[valuer.UUID][]*tagtypes.Tag{}, nil
-	}
-
-	type joinedRow struct {
-		tagtypes.Tag
-		EntityID valuer.UUID `bun:"entity_id"`
-	}
-
-	rows := make([]*joinedRow, 0)
-	err := s.sqlstore.
-		BunDBCtx(ctx).
-		NewSelect().
-		Model(&rows).
-		ColumnExpr("tag.*, tr.entity_id").
-		Join("JOIN tag_relations AS tr ON tr.tag_id = tag.id").
-		Where("tr.entity_type = ?", entityType).
-		Where("tr.entity_id IN (?)", bun.In(entityIDs)).
-		Scan(ctx)
-	if err != nil {
-		return nil, err
-	}
-
-	out := make(map[valuer.UUID][]*tagtypes.Tag)
-	for _, r := range rows {
-		tag := r.Tag
-		out[r.EntityID] = append(out[r.EntityID], &tag)
-	}
-	return out, nil
-}
-
-func (s *store) Create(ctx context.Context, tags []*tagtypes.Tag) ([]*tagtypes.Tag, error) {
-	if len(tags) == 0 {
-		return tags, nil
-	}
-	// DO UPDATE on a self-set is a deliberate no-op write whose only purpose
-	// is to make RETURNING fire on conflicting rows. Without it, RETURNING is
-	// silent on the conflict path and we'd have to refetch by (key, value) to
-	// learn the existing rows' IDs after a concurrent-insert race. Setting
-	// key = tag.key (the existing row's value) preserves the first writer's
-	// casing on case-only collisions.
-	err := s.sqlstore.
-		BunDBCtx(ctx).
-		NewInsert().
-		Model(&tags).
-		On("CONFLICT (org_id, entity_type, (LOWER(key)), (LOWER(value))) DO UPDATE").
-		Set("key = tag.key").
-		Returning("*").
-		Scan(ctx)
-	if err != nil {
-		return nil, err
-	}
-	return tags, nil
-}
-
-func (s *store) CreateRelations(ctx context.Context, relations []*tagtypes.TagRelation) error {
-	if len(relations) == 0 {
-		return nil
-	}
-	_, err := s.sqlstore.
-		BunDBCtx(ctx).
-		NewInsert().
-		Model(&relations).
-		On("CONFLICT (entity_type, entity_id, tag_id) DO NOTHING").
-		Exec(ctx)
-	return err
-}
-
-func (s *store) DeleteRelationsExcept(ctx context.Context, entityType tagtypes.EntityType, entityID valuer.UUID, keepTagIDs []valuer.UUID) error {
-	q := s.sqlstore.
-		BunDBCtx(ctx).
-		NewDelete().
-		Model((*tagtypes.TagRelation)(nil)).
-		Where("entity_type = ?", entityType).
-		Where("entity_id = ?", entityID)
-	if len(keepTagIDs) > 0 {
-		q = q.Where("tag_id NOT IN (?)", bun.In(keepTagIDs))
-	}
-	_, err := q.Exec(ctx)
-	return err
-}

pkg/modules/tag/impltag/store_test.go

@@ -1,146 +0,0 @@
-package impltag
-
-import (
-	"context"
-	"path/filepath"
-	"strings"
-	"testing"
-	"time"
-
-	"github.com/SigNoz/signoz/pkg/factory/factorytest"
-	"github.com/SigNoz/signoz/pkg/sqlstore"
-	"github.com/SigNoz/signoz/pkg/sqlstore/sqlitesqlstore"
-	"github.com/SigNoz/signoz/pkg/types/tagtypes"
-	"github.com/SigNoz/signoz/pkg/valuer"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"github.com/uptrace/bun"
-)
-
-func newTestStore(t *testing.T) sqlstore.SQLStore {
-	t.Helper()
-	dbPath := filepath.Join(t.TempDir(), "test.db")
-	store, err := sqlitesqlstore.New(context.Background(), factorytest.NewSettings(), sqlstore.Config{
-		Provider: "sqlite",
-		Connection: sqlstore.ConnectionConfig{
-			MaxOpenConns:    1,
-			MaxConnLifetime: 0,
-		},
-		Sqlite: sqlstore.SqliteConfig{
-			Path:            dbPath,
-			Mode:            "wal",
-			BusyTimeout:     5 * time.Second,
-			TransactionMode: "deferred",
-		},
-	})
-	require.NoError(t, err)
-
-	_, err = store.BunDB().NewCreateTable().
-		Model((*tagtypes.Tag)(nil)).
-		IfNotExists().
-		Exec(context.Background())
-	require.NoError(t, err)
-
-	_, err = store.BunDB().Exec(`CREATE UNIQUE INDEX IF NOT EXISTS uq_tag_org_entity_lower_key_lower_value ON tag (org_id, entity_type, LOWER(key), LOWER(value))`)
-	require.NoError(t, err)
-	return store
-}
-
-var dashboardEntityType = tagtypes.MustNewEntityType("dashboard")
-
-func tagsByLowerKeyValue(t *testing.T, db *bun.DB) map[string]*tagtypes.Tag {
-	t.Helper()
-	all := make([]*tagtypes.Tag, 0)
-	require.NoError(t, db.NewSelect().Model(&all).Scan(context.Background()))
-	out := map[string]*tagtypes.Tag{}
-	for _, tag := range all {
-		out[strings.ToLower(tag.Key)+"\x00"+strings.ToLower(tag.Value)] = tag
-	}
-	return out
-}
-
-func TestStore_Create_PopulatesIDsOnFreshInsert(t *testing.T) {
-	ctx := context.Background()
-	sqlstore := newTestStore(t)
-	s := NewStore(sqlstore)
-
-	orgID := valuer.GenerateUUID()
-	tagA := tagtypes.NewTag(orgID, dashboardEntityType, "tag", "Database", "u@signoz.io")
-	tagB := tagtypes.NewTag(orgID, dashboardEntityType, "team", "BLR", "u@signoz.io")
-	preIDA := tagA.ID
-	preIDB := tagB.ID
-
-	got, err := s.Create(ctx, []*tagtypes.Tag{tagA, tagB})
-	require.NoError(t, err)
-	require.Len(t, got, 2)
-
-	// No race → pre-generated IDs stand. The slice is what we passed in,
-	// confirming Scan didn't reallocate.
-	assert.Equal(t, preIDA, got[0].ID)
-	assert.Equal(t, preIDB, got[1].ID)
-
-	// And the rows are in the DB.
-	stored := tagsByLowerKeyValue(t, sqlstore.BunDB())
-	require.Contains(t, stored, "tag\x00database")
-	require.Contains(t, stored, "team\x00blr")
-	assert.Equal(t, preIDA, stored["tag\x00database"].ID)
-	assert.Equal(t, preIDB, stored["team\x00blr"].ID)
-}
-
-func TestStore_Create_ConflictReturnsExistingRowID(t *testing.T) {
-	ctx := context.Background()
-	sqlstore := newTestStore(t)
-	s := NewStore(sqlstore)
-
-	orgID := valuer.GenerateUUID()
-
-	// Simulate a concurrent insert: someone else has already inserted "tag:Database".
-	winner := tagtypes.NewTag(orgID, dashboardEntityType, "tag", "Database", "concurrent")
-	_, err := s.Create(ctx, []*tagtypes.Tag{winner})
-	require.NoError(t, err)
-	winnerID := winner.ID
-
-	// Now our request runs with a different pre-generated ID for the same
-	// (key, value) — case differs but the functional unique index collapses
-	// them. RETURNING should overwrite our stale ID with winner's ID.
-	loser := tagtypes.NewTag(orgID, dashboardEntityType, "TAG", "DATABASE", "u@signoz.io")
-	loserPreID := loser.ID
-	require.NotEqual(t, winnerID, loserPreID, "pre-generated IDs must differ for this test to be meaningful")
-
-	got, err := s.Create(ctx, []*tagtypes.Tag{loser})
-	require.NoError(t, err)
-	require.Len(t, got, 1)
-
-	assert.Equal(t, winnerID, got[0].ID, "returned slice should carry the existing row's ID, not our stale one")
-	assert.Equal(t, winnerID, loser.ID, "input slice element is mutated in place")
-
-	// And the DB still has exactly one row for that (lower(key), lower(value)) — winner's, with winner's casing.
-	stored := tagsByLowerKeyValue(t, sqlstore.BunDB())
-	require.Len(t, stored, 1)
-	assert.Equal(t, winnerID, stored["tag\x00database"].ID)
-	assert.Equal(t, "tag", stored["tag\x00database"].Key, "winner's casing preserved in key")
-	assert.Equal(t, "Database", stored["tag\x00database"].Value, "winner's casing preserved in value")
-}
-
-func TestStore_Create_MixedFreshAndConflict(t *testing.T) {
-	ctx := context.Background()
-	sqlstore := newTestStore(t)
-	s := NewStore(sqlstore)
-
-	orgID := valuer.GenerateUUID()
-	pre := tagtypes.NewTag(orgID, dashboardEntityType, "tag", "Database", "concurrent")
-	_, err := s.Create(ctx, []*tagtypes.Tag{pre})
-	require.NoError(t, err)
-	preExistingID := pre.ID
-
-	conflict := tagtypes.NewTag(orgID, dashboardEntityType, "tag", "Database", "u@signoz.io")
-	fresh := tagtypes.NewTag(orgID, dashboardEntityType, "team", "BLR", "u@signoz.io")
-	freshPreID := fresh.ID
-
-	got, err := s.Create(ctx, []*tagtypes.Tag{conflict, fresh})
-	require.NoError(t, err)
-	require.Len(t, got, 2)
-
-	assert.Equal(t, preExistingID, got[0].ID, "conflicting row's ID overwritten with the existing row's")
-	assert.Equal(t, freshPreID, got[1].ID, "fresh row's pre-generated ID is preserved")
-}

pkg/modules/tag/tag.go

@@ -1,24 +0,0 @@
-package tag
-
-import (
-	"context"
-
-	"github.com/SigNoz/signoz/pkg/types/tagtypes"
-	"github.com/SigNoz/signoz/pkg/valuer"
-)
-
-type Module interface {
-	// Does not link the resolved tags to any entity — call LinkToEntity for that.
-	CreateMany(ctx context.Context, orgID valuer.UUID, entityType tagtypes.EntityType, postable []tagtypes.PostableTag, createdBy string) ([]*tagtypes.Tag, error)
-
-	// Existing rows are left untouched.
-	LinkToEntity(ctx context.Context, orgID valuer.UUID, entityType tagtypes.EntityType, entityID valuer.UUID, tagIDs []valuer.UUID) error
-
-	// missing links are inserted, obsolete ones removed.
-	SyncLinksForEntity(ctx context.Context, orgID valuer.UUID, entityType tagtypes.EntityType, entityID valuer.UUID, tagIDs []valuer.UUID) error
-
-	ListForEntity(ctx context.Context, entityType tagtypes.EntityType, entityID valuer.UUID) ([]*tagtypes.Tag, error)
-
-	// Entities with no tags are absent from the returned map.
-	ListForEntities(ctx context.Context, entityType tagtypes.EntityType, entityIDs []valuer.UUID) (map[valuer.UUID][]*tagtypes.Tag, error)
-}

pkg/signoz/handler_test.go

@@ -16,7 +16,6 @@ import (
 	"github.com/SigNoz/signoz/pkg/instrumentation/instrumentationtest"
 	"github.com/SigNoz/signoz/pkg/modules/dashboard/impldashboard"
 	"github.com/SigNoz/signoz/pkg/modules/organization/implorganization"
-	"github.com/SigNoz/signoz/pkg/modules/tag/impltag"
 	"github.com/SigNoz/signoz/pkg/modules/user/impluser"
 	"github.com/SigNoz/signoz/pkg/querier"
 	"github.com/SigNoz/signoz/pkg/queryparser"
@@ -45,7 +44,6 @@ func TestNewHandlers(t *testing.T) {
 	emailing := emailingtest.New()
 	queryParser := queryparser.New(providerSettings)
 	require.NoError(t, err)
-	tagModule := impltag.NewModule(impltag.NewStore(sqlstore))
 	dashboardModule := impldashboard.NewModule(impldashboard.NewStore(sqlstore), providerSettings, nil, orgGetter, queryParser)
 
 	flagger, err := flagger.New(context.Background(), instrumentationtest.New().ToProviderSettings(), flagger.Config{}, flagger.MustNewRegistry())
@@ -54,7 +52,7 @@ func TestNewHandlers(t *testing.T) {
 	userRoleStore := impluser.NewUserRoleStore(sqlstore, providerSettings)
 
 	userGetter := impluser.NewGetter(impluser.NewStore(sqlstore, providerSettings), userRoleStore, flagger)
-	modules := NewModules(sqlstore, tokenizer, emailing, providerSettings, orgGetter, alertmanager, nil, nil, nil, nil, nil, nil, nil, queryParser, Config{}, dashboardModule, userGetter, userRoleStore, nil, nil, flagger, tagModule)
+	modules := NewModules(sqlstore, tokenizer, emailing, providerSettings, orgGetter, alertmanager, nil, nil, nil, nil, nil, nil, nil, queryParser, Config{}, dashboardModule, userGetter, userRoleStore, nil, nil, flagger)
 
 	querierHandler := querier.NewHandler(providerSettings, nil, nil)
 	registryHandler := factory.NewHandler(nil)

pkg/signoz/module.go

@@ -40,7 +40,6 @@ import (
 	"github.com/SigNoz/signoz/pkg/modules/session/implsession"
 	"github.com/SigNoz/signoz/pkg/modules/spanpercentile"
 	"github.com/SigNoz/signoz/pkg/modules/spanpercentile/implspanpercentile"
-	"github.com/SigNoz/signoz/pkg/modules/tag"
 	"github.com/SigNoz/signoz/pkg/modules/tracedetail"
 	"github.com/SigNoz/signoz/pkg/modules/tracedetail/impltracedetail"
 	"github.com/SigNoz/signoz/pkg/modules/tracefunnel"
@@ -81,7 +80,6 @@ type Modules struct {
 	CloudIntegration cloudintegration.Module
 	RuleStateHistory rulestatehistory.Module
 	TraceDetail      tracedetail.Module
-	Tag              tag.Module
 }
 
 func NewModules(
@@ -106,7 +104,6 @@ func NewModules(
 	serviceAccount serviceaccount.Module,
 	cloudIntegrationModule cloudintegration.Module,
 	fl flagger.Flagger,
-	tagModule tag.Module,
 ) Modules {
 	quickfilter := implquickfilter.NewModule(implquickfilter.NewStore(sqlstore))
 	orgSetter := implorganization.NewSetter(implorganization.NewStore(sqlstore), alertmanager, quickfilter)
@@ -136,6 +133,5 @@ func NewModules(
 		RuleStateHistory: implrulestatehistory.NewModule(implrulestatehistory.NewStore(telemetryStore, telemetryMetadataStore, providerSettings.Logger)),
 		CloudIntegration: cloudIntegrationModule,
 		TraceDetail:      impltracedetail.NewModule(impltracedetail.NewTraceStore(telemetryStore), providerSettings, config.TraceDetail),
-		Tag:              tagModule,
 	}
 }

pkg/signoz/module_test.go

@@ -18,7 +18,6 @@ import (
 	"github.com/SigNoz/signoz/pkg/modules/organization/implorganization"
 	"github.com/SigNoz/signoz/pkg/modules/serviceaccount"
 	"github.com/SigNoz/signoz/pkg/modules/serviceaccount/implserviceaccount"
-	"github.com/SigNoz/signoz/pkg/modules/tag/impltag"
 	"github.com/SigNoz/signoz/pkg/modules/user/impluser"
 	"github.com/SigNoz/signoz/pkg/queryparser"
 	"github.com/SigNoz/signoz/pkg/sharder"
@@ -46,7 +45,6 @@ func TestNewModules(t *testing.T) {
 	emailing := emailingtest.New()
 	queryParser := queryparser.New(providerSettings)
 	require.NoError(t, err)
-	tagModule := impltag.NewModule(impltag.NewStore(sqlstore))
 	dashboardModule := impldashboard.NewModule(impldashboard.NewStore(sqlstore), providerSettings, nil, orgGetter, queryParser)
 
 	flagger, err := flagger.New(context.Background(), instrumentationtest.New().ToProviderSettings(), flagger.Config{}, flagger.MustNewRegistry())
@@ -58,7 +56,7 @@ func TestNewModules(t *testing.T) {
 
 	serviceAccount := implserviceaccount.NewModule(implserviceaccount.NewStore(sqlstore), nil, nil, nil, providerSettings, serviceaccount.Config{})
 
-	modules := NewModules(sqlstore, tokenizer, emailing, providerSettings, orgGetter, alertmanager, nil, nil, nil, nil, nil, nil, nil, queryParser, Config{}, dashboardModule, userGetter, userRoleStore, serviceAccount, implcloudintegration.NewModule(), flagger, tagModule)
+	modules := NewModules(sqlstore, tokenizer, emailing, providerSettings, orgGetter, alertmanager, nil, nil, nil, nil, nil, nil, nil, queryParser, Config{}, dashboardModule, userGetter, userRoleStore, serviceAccount, implcloudintegration.NewModule(), flagger)
 
 	reflectVal := reflect.ValueOf(modules)
 	for i := 0; i < reflectVal.NumField(); i++ {

pkg/signoz/provider.go

@@ -196,7 +196,6 @@ func NewSQLMigrationProviderFactories(
 		sqlmigration.NewDropUserDeletedAtFactory(sqlstore, sqlschema),
 		sqlmigration.NewMigrateAWSAllRegionsFactory(sqlstore),
 		sqlmigration.NewAddServiceAccountManagedRoleTransactionsFactory(sqlstore),
-		sqlmigration.NewAddTagsFactory(sqlstore, sqlschema),
 	)
 }
 

pkg/signoz/signoz.go

@@ -29,7 +29,6 @@ import (
 	"github.com/SigNoz/signoz/pkg/modules/rulestatehistory"
 	"github.com/SigNoz/signoz/pkg/modules/serviceaccount"
 	"github.com/SigNoz/signoz/pkg/modules/serviceaccount/implserviceaccount"
-	"github.com/SigNoz/signoz/pkg/modules/tag/impltag"
 	"github.com/SigNoz/signoz/pkg/modules/user/impluser"
 	"github.com/SigNoz/signoz/pkg/prometheus"
 	"github.com/SigNoz/signoz/pkg/querier"
@@ -326,11 +325,6 @@ func New(
 	// Initialize query parser (needed for dashboard module)
 	queryParser := queryparser.New(providerSettings)
 
-	// Initialize tag module — shared across modules that link entities to tags
-	// (currently dashboard; future: alerts, RBAC). Built once here and injected
-	// where needed.
-	tagModule := impltag.NewModule(impltag.NewStore(sqlstore))
-
 	// Initialize dashboard module
 	dashboard := dashboardModuleCallback(sqlstore, providerSettings, analytics, orgGetter, queryParser, querier, licensing)
 
@@ -447,7 +441,7 @@ func New(
 	}
 
 	// Initialize all modules
-	modules := NewModules(sqlstore, tokenizer, emailing, providerSettings, orgGetter, alertmanager, analytics, querier, telemetrystore, telemetryMetadataStore, authNs, authz, cache, queryParser, config, dashboard, userGetter, userRoleStore, serviceAccount, cloudIntegrationModule, flagger, tagModule)
+	modules := NewModules(sqlstore, tokenizer, emailing, providerSettings, orgGetter, alertmanager, analytics, querier, telemetrystore, telemetryMetadataStore, authNs, authz, cache, queryParser, config, dashboard, userGetter, userRoleStore, serviceAccount, cloudIntegrationModule, flagger)
 
 	// Initialize ruler from the variant-specific provider factories
 	rulerInstance, err := factory.NewProviderFromNamedMap(ctx, providerSettings, config.Ruler, rulerProviderFactories(cache, alertmanager, sqlstore, telemetrystore, telemetryMetadataStore, prometheus, orgGetter, modules.RuleStateHistory, querier, queryParser), "signoz")

pkg/sqlmigration/079_add_tags.go

@@ -1,102 +0,0 @@
-package sqlmigration
-
-import (
-	"context"
-
-	"github.com/SigNoz/signoz/pkg/factory"
-	"github.com/SigNoz/signoz/pkg/sqlschema"
-	"github.com/SigNoz/signoz/pkg/sqlstore"
-	"github.com/uptrace/bun"
-	"github.com/uptrace/bun/migrate"
-)
-
-type addTags struct {
-	sqlstore  sqlstore.SQLStore
-	sqlschema sqlschema.SQLSchema
-}
-
-func NewAddTagsFactory(sqlstore sqlstore.SQLStore, sqlschema sqlschema.SQLSchema) factory.ProviderFactory[SQLMigration, Config] {
-	return factory.NewProviderFactory(factory.MustNewName("add_tags"), func(ctx context.Context, ps factory.ProviderSettings, c Config) (SQLMigration, error) {
-		return &addTags{
-			sqlstore:  sqlstore,
-			sqlschema: sqlschema,
-		}, nil
-	})
-}
-
-func (migration *addTags) Register(migrations *migrate.Migrations) error {
-	return migrations.Register(migration.Up, migration.Down)
-}
-
-func (migration *addTags) Up(ctx context.Context, db *bun.DB) error {
-	tx, err := db.BeginTx(ctx, nil)
-	if err != nil {
-		return err
-	}
-
-	defer func() {
-		_ = tx.Rollback()
-	}()
-
-	sqls := [][]byte{}
-
-	tagTableSQLs := migration.sqlschema.Operator().CreateTable(&sqlschema.Table{
-		Name: "tag",
-		Columns: []*sqlschema.Column{
-			{Name: "id", DataType: sqlschema.DataTypeText, Nullable: false},
-			{Name: "key", DataType: sqlschema.DataTypeText, Nullable: false},
-			{Name: "value", DataType: sqlschema.DataTypeText, Nullable: false},
-			{Name: "org_id", DataType: sqlschema.DataTypeText, Nullable: false},
-			{Name: "entity_type", DataType: sqlschema.DataTypeText, Nullable: false},
-			{Name: "created_at", DataType: sqlschema.DataTypeTimestamp, Nullable: false},
-			{Name: "created_by", DataType: sqlschema.DataTypeText, Nullable: true},
-			{Name: "updated_at", DataType: sqlschema.DataTypeTimestamp, Nullable: false},
-			{Name: "updated_by", DataType: sqlschema.DataTypeText, Nullable: true},
-		},
-		PrimaryKeyConstraint: &sqlschema.PrimaryKeyConstraint{ColumnNames: []sqlschema.ColumnName{"id"}},
-		ForeignKeyConstraints: []*sqlschema.ForeignKeyConstraint{
-			{
-				ReferencingColumnName: sqlschema.ColumnName("org_id"),
-				ReferencedTableName:   sqlschema.TableName("organizations"),
-				ReferencedColumnName:  sqlschema.ColumnName("id"),
-			},
-		},
-	})
-	sqls = append(sqls, tagTableSQLs...)
-
-	// Functional unique index: case-insensitive uniqueness on (org_id, entity_type, key, value).
-	// sqlschema.UniqueIndex doesn't support expressions, so emit raw SQL — both
-	// Postgres and SQLite (modernc 3.50.x) support expression indexes.
-	sqls = append(sqls, []byte(`CREATE UNIQUE INDEX IF NOT EXISTS uq_tag_org_entity_lower_key_lower_value ON tag (org_id, entity_type, LOWER(key), LOWER(value))`))
-
-	tagRelationsTableSQLs := migration.sqlschema.Operator().CreateTable(&sqlschema.Table{
-		Name: "tag_relations",
-		Columns: []*sqlschema.Column{
-			{Name: "entity_type", DataType: sqlschema.DataTypeText, Nullable: false},
-			{Name: "entity_id", DataType: sqlschema.DataTypeText, Nullable: false},
-			{Name: "tag_id", DataType: sqlschema.DataTypeText, Nullable: false},
-			{Name: "org_id", DataType: sqlschema.DataTypeText, Nullable: false},
-		},
-		PrimaryKeyConstraint: &sqlschema.PrimaryKeyConstraint{ColumnNames: []sqlschema.ColumnName{"entity_type", "entity_id", "tag_id"}},
-		ForeignKeyConstraints: []*sqlschema.ForeignKeyConstraint{
-			{
-				ReferencingColumnName: sqlschema.ColumnName("org_id"),
-				ReferencedTableName:   sqlschema.TableName("organizations"),
-				ReferencedColumnName:  sqlschema.ColumnName("id"),
-			},
-		},
-	})
-	sqls = append(sqls, tagRelationsTableSQLs...)
-
-	for _, sql := range sqls {
-		if _, err := tx.ExecContext(ctx, string(sql)); err != nil {
-			return err
-		}
-	}
-
-	return tx.Commit()
-}
-
-func (migration *addTags) Down(_ context.Context, _ *bun.DB) error {
-	return nil
-}

pkg/types/serviceaccounttypes/service_acccount.go

@@ -245,7 +245,6 @@ type Store interface {
 
 	// Service Account Role
 	CreateServiceAccountRole(context.Context, *ServiceAccountRole) error
-	DeleteServiceAccountRoles(context.Context, valuer.UUID) error
 	DeleteServiceAccountRole(context.Context, valuer.UUID, valuer.UUID) error
 
 	// Service Account Factor API Key

pkg/types/tagtypes/store.go

@@ -1,27 +0,0 @@
-package tagtypes
-
-import (
-	"context"
-
-	"github.com/SigNoz/signoz/pkg/valuer"
-)
-
-type Store interface {
-	List(ctx context.Context, orgID valuer.UUID, entityType EntityType) ([]*Tag, error)
-
-	ListByEntity(ctx context.Context, entityType EntityType, entityID valuer.UUID) ([]*Tag, error)
-
-	ListByEntities(ctx context.Context, entityType EntityType, entityIDs []valuer.UUID) (map[valuer.UUID][]*Tag, error)
-
-	// Create upserts the given tags and returns them with authoritative IDs.
-	// On conflict on (org_id, entity_type, LOWER(key), LOWER(value)) — which
-	// happens only when a concurrent insert raced ours, including casing-only
-	// collisions — the returned entry carries the existing row's ID rather
-	// than the pre-generated one in the input.
-	Create(ctx context.Context, tags []*Tag) ([]*Tag, error)
-
-	// CreateRelations inserts tag-entity relations. Conflicts on the composite primary key are ignored.
-	CreateRelations(ctx context.Context, relations []*TagRelation) error
-
-	DeleteRelationsExcept(ctx context.Context, entityType EntityType, entityID valuer.UUID, keepTagIDs []valuer.UUID) error
-}

pkg/types/tagtypes/tag.go

@@ -1,137 +0,0 @@
-package tagtypes
-
-import (
-	"context"
-	"strings"
-	"time"
-
-	"github.com/SigNoz/signoz/pkg/errors"
-	"github.com/SigNoz/signoz/pkg/types"
-	"github.com/SigNoz/signoz/pkg/valuer"
-	"github.com/uptrace/bun"
-)
-
-var (
-	ErrCodeTagInvalidName = errors.MustNewCode("tag_invalid_name")
-	ErrCodeTagNotFound    = errors.MustNewCode("tag_not_found")
-)
-
-type Tag struct {
-	bun.BaseModel `bun:"table:tag,alias:tag"`
-
-	types.Identifiable
-	types.TimeAuditable
-	types.UserAuditable
-	Key        string      `json:"key" required:"true" bun:"key,type:text,notnull"`
-	Value      string      `json:"value" required:"true" bun:"value,type:text,notnull"`
-	OrgID      valuer.UUID `json:"orgId" required:"true" bun:"org_id,type:text,notnull"`
-	EntityType EntityType  `json:"entityType" required:"true" bun:"entity_type,type:text,notnull"`
-}
-
-type PostableTag struct {
-	Key   string `json:"key" required:"true"`
-	Value string `json:"value" required:"true"`
-}
-
-type GettableTag = PostableTag
-
-func NewGettableTagFromTag(tag *Tag) *GettableTag {
-	return &GettableTag{Key: tag.Key, Value: tag.Value}
-}
-
-func NewGettableTagsFromTags(tags []*Tag) []*GettableTag {
-	out := make([]*GettableTag, len(tags))
-	for i, t := range tags {
-		out[i] = NewGettableTagFromTag(t)
-	}
-	return out
-}
-
-func NewTag(orgID valuer.UUID, entityType EntityType, key, value, createdBy string) *Tag {
-	now := time.Now()
-	return &Tag{
-		Identifiable: types.Identifiable{ID: valuer.GenerateUUID()},
-		TimeAuditable: types.TimeAuditable{
-			CreatedAt: now,
-			UpdatedAt: now,
-		},
-		UserAuditable: types.UserAuditable{
-			CreatedBy: createdBy,
-			UpdatedBy: createdBy,
-		},
-		Key:        key,
-		Value:      value,
-		OrgID:      orgID,
-		EntityType: entityType,
-	}
-}
-
-// Resolve canonicalizes a batch of user-supplied (key, value) tag pairs against
-// the existing tags for an org. Lookup is case-insensitive on both key and
-// value (matching the storage uniqueness rule); when an existing row matches,
-// its display casing is reused. Inputs are deduped on (LOWER(key), LOWER(value));
-// the first input's casing wins on collisions. Returns:
-//   - toCreate: new Tag rows the caller should insert (with pre-generated IDs)
-//   - matched: existing rows the caller's input already pointed to. They
-//     already carry authoritative IDs from the store.
-func Resolve(ctx context.Context, store Store, orgID valuer.UUID, entityType EntityType, postable []PostableTag, createdBy string) ([]*Tag, []*Tag, error) {
-	if len(postable) == 0 {
-		return nil, nil, nil
-	}
-
-	existing, err := store.List(ctx, orgID, entityType)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	lowercaseTagsMap := make(map[string]*Tag, len(existing))
-	for _, t := range existing {
-		mapKey := strings.ToLower(t.Key) + "\x00" + strings.ToLower(t.Value)
-		lowercaseTagsMap[mapKey] = t
-	}
-
-	seenInRequestAlready := make(map[string]struct{}, len(postable)) // postable can have the same tag multiple times
-	toCreate := make([]*Tag, 0)
-	matched := make([]*Tag, 0)
-
-	for _, p := range postable {
-		key, value, err := validatePostableTag(p)
-		if err != nil {
-			return nil, nil, err
-		}
-		lookup := strings.ToLower(key) + "\x00" + strings.ToLower(value)
-		if _, dup := seenInRequestAlready[lookup]; dup {
-			continue
-		}
-		seenInRequestAlready[lookup] = struct{}{}
-
-		if existingTag, ok := lowercaseTagsMap[lookup]; ok {
-			matched = append(matched, existingTag)
-			continue
-		}
-		toCreate = append(toCreate, NewTag(orgID, entityType, key, value, createdBy))
-	}
-
-	return toCreate, matched, nil
-}
-
-// Entity-specific reserved-key checks (e.g. dashboard column names that would
-// collide with the list-query DSL) are the caller's responsibility — perform
-// them before calling into the tag module.
-func validatePostableTag(p PostableTag) (string, string, error) {
-	key := strings.TrimSpace(p.Key)
-	value := strings.TrimSpace(p.Value)
-	if key == "" {
-		return "", "", errors.Newf(errors.TypeInvalidInput, ErrCodeTagInvalidName, "tag key cannot be empty")
-	}
-	if value == "" {
-		return "", "", errors.Newf(errors.TypeInvalidInput, ErrCodeTagInvalidName, "tag value cannot be empty")
-	}
-	if strings.ContainsRune(key, '/') {
-		return "", "", errors.Newf(errors.TypeInvalidInput, ErrCodeTagInvalidName, "tag key %q cannot contain '/'", key)
-	}
-	if strings.ContainsRune(value, '/') {
-		return "", "", errors.Newf(errors.TypeInvalidInput, ErrCodeTagInvalidName, "tag value %q cannot contain '/'", value)
-	}
-	return key, value, nil
-}

pkg/types/tagtypes/tag_relation.go

@@ -1,38 +0,0 @@
-package tagtypes
-
-import (
-	"github.com/SigNoz/signoz/pkg/valuer"
-	"github.com/uptrace/bun"
-)
-
-type EntityType struct{ valuer.String }
-
-func MustNewEntityType(name string) EntityType {
-	return EntityType{valuer.NewString(name)}
-}
-
-type TagRelation struct {
-	bun.BaseModel `bun:"table:tag_relations,alias:tag_relations"`
-
-	EntityType EntityType  `json:"entityType" required:"true" bun:"entity_type,type:text,notnull"`
-	EntityID   valuer.UUID `json:"entityId" required:"true" bun:"entity_id,pk,type:text,notnull"`
-	TagID      valuer.UUID `json:"tagId" required:"true" bun:"tag_id,pk,type:text,notnull"`
-	OrgID      valuer.UUID `json:"orgId" required:"true" bun:"org_id,type:text,notnull"`
-}
-
-func NewTagRelation(orgID valuer.UUID, entityType EntityType, entityID valuer.UUID, tagID valuer.UUID) *TagRelation {
-	return &TagRelation{
-		EntityType: entityType,
-		EntityID:   entityID,
-		TagID:      tagID,
-		OrgID:      orgID,
-	}
-}
-
-func NewTagRelations(orgID valuer.UUID, entityType EntityType, entityID valuer.UUID, tagIDs []valuer.UUID) []*TagRelation {
-	relations := make([]*TagRelation, 0, len(tagIDs))
-	for _, tagID := range tagIDs {
-		relations = append(relations, NewTagRelation(orgID, entityType, entityID, tagID))
-	}
-	return relations
-}

pkg/types/tagtypes/tag_test.go

@@ -1,166 +0,0 @@
-package tagtypes
-
-import (
-	"context"
-	"strings"
-	"testing"
-
-	"github.com/SigNoz/signoz/pkg/valuer"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-func TestValidatePostableTag(t *testing.T) {
-	tests := []struct {
-		name      string
-		input     PostableTag
-		wantKey   string
-		wantValue string
-		wantError bool
-	}{
-		{name: "simple pair", input: PostableTag{Key: "team", Value: "pulse"}, wantKey: "team", wantValue: "pulse"},
-		{name: "preserves casing", input: PostableTag{Key: "Team", Value: "Pulse"}, wantKey: "Team", wantValue: "Pulse"},
-		{name: "trims key", input: PostableTag{Key: "  team  ", Value: "pulse"}, wantKey: "team", wantValue: "pulse"},
-		{name: "trims value", input: PostableTag{Key: "team", Value: "  pulse  "}, wantKey: "team", wantValue: "pulse"},
-
-		{name: "empty key rejected", input: PostableTag{Key: "", Value: "pulse"}, wantError: true},
-		{name: "empty value rejected", input: PostableTag{Key: "team", Value: ""}, wantError: true},
-		{name: "whitespace-only key rejected", input: PostableTag{Key: "   ", Value: "pulse"}, wantError: true},
-		{name: "whitespace-only value rejected", input: PostableTag{Key: "team", Value: "   "}, wantError: true},
-
-		{name: "slash in key rejected", input: PostableTag{Key: "team/eng", Value: "pulse"}, wantError: true},
-		{name: "slash in value rejected", input: PostableTag{Key: "team", Value: "pulse/events"}, wantError: true},
-	}
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			gotKey, gotValue, err := validatePostableTag(tc.input)
-			if tc.wantError {
-				require.Error(t, err)
-				return
-			}
-			require.NoError(t, err)
-			assert.Equal(t, tc.wantKey, gotKey)
-			assert.Equal(t, tc.wantValue, gotValue)
-		})
-	}
-}
-
-var testEntityType = MustNewEntityType("dashboard")
-
-type fakeStore struct {
-	tags          []*Tag
-	listCallCount int
-}
-
-func (f *fakeStore) List(_ context.Context, _ valuer.UUID, _ EntityType) ([]*Tag, error) {
-	f.listCallCount++
-	out := make([]*Tag, len(f.tags))
-	copy(out, f.tags)
-	return out, nil
-}
-
-func (f *fakeStore) Create(_ context.Context, tags []*Tag) ([]*Tag, error) {
-	return tags, nil
-}
-
-func (f *fakeStore) CreateRelations(_ context.Context, _ []*TagRelation) error {
-	return nil
-}
-
-func (f *fakeStore) ListByEntity(_ context.Context, _ EntityType, _ valuer.UUID) ([]*Tag, error) {
-	return []*Tag{}, nil
-}
-
-func (f *fakeStore) ListByEntities(_ context.Context, _ EntityType, _ []valuer.UUID) (map[valuer.UUID][]*Tag, error) {
-	return map[valuer.UUID][]*Tag{}, nil
-}
-
-func (f *fakeStore) DeleteRelationsExcept(_ context.Context, _ EntityType, _ valuer.UUID, _ []valuer.UUID) error {
-	return nil
-}
-
-func TestResolve(t *testing.T) {
-	t.Run("empty input does not hit store", func(t *testing.T) {
-		store := &fakeStore{}
-		toCreate, matched, err := Resolve(context.Background(), store, valuer.GenerateUUID(), testEntityType, nil, "u@signoz.io")
-		require.NoError(t, err)
-		assert.Empty(t, toCreate)
-		assert.Empty(t, matched)
-		assert.Zero(t, store.listCallCount, "should not hit store when input is empty")
-	})
-
-	t.Run("creates missing pairs and reuses existing", func(t *testing.T) {
-		orgID := valuer.GenerateUUID()
-		dbTag := NewTag(orgID, testEntityType, "team", "Pulse", "seed")
-		dbTag2 := NewTag(orgID, testEntityType, "Database", "redis", "seed")
-		store := &fakeStore{tags: []*Tag{dbTag, dbTag2}}
-
-		toCreate, matched, err := Resolve(context.Background(), store, orgID, testEntityType, []PostableTag{
-			{Key: "team", Value: "events"},    // new
-			{Key: "DATABASE", Value: "REDIS"}, // case-only conflict
-			{Key: "Brand", Value: "New"},      // new
-		}, "u@signoz.io")
-		require.NoError(t, err)
-
-		createdLowerKVs := []string{}
-		for _, tg := range toCreate {
-			createdLowerKVs = append(createdLowerKVs, strings.ToLower(tg.Key)+"\x00"+strings.ToLower(tg.Value))
-		}
-		assert.ElementsMatch(t, []string{"team\x00events", "brand\x00new"}, createdLowerKVs,
-			"only the two missing pairs should be returned for insertion")
-
-		require.Len(t, matched, 1, "DATABASE:REDIS should hit the existing 'Database:redis' tag")
-		assert.Same(t, dbTag2, matched[0], "matched should return the existing pointer with its authoritative ID")
-	})
-
-	t.Run("dedupes inputs that map to the same lower(key)+lower(value)", func(t *testing.T) {
-		orgID := valuer.GenerateUUID()
-		store := &fakeStore{}
-
-		toCreate, matched, err := Resolve(context.Background(), store, orgID, testEntityType, []PostableTag{
-			{Key: "Foo", Value: "Bar"},
-			{Key: "foo", Value: "bar"},
-			{Key: "FOO", Value: "BAR"},
-		}, "u@signoz.io")
-		require.NoError(t, err)
-
-		require.Empty(t, matched)
-		require.Len(t, toCreate, 1, "duplicate inputs must collapse into a single insert")
-		assert.Equal(t, "Foo", toCreate[0].Key, "first input's casing wins")
-		assert.Equal(t, "Bar", toCreate[0].Value, "first input's casing wins")
-	})
-
-	t.Run("preserves existing casing on case-only match", func(t *testing.T) {
-		orgID := valuer.GenerateUUID()
-		dbTag := NewTag(orgID, testEntityType, "Team", "Pulse", "seed")
-		store := &fakeStore{tags: []*Tag{dbTag}}
-
-		toCreate, matched, err := Resolve(context.Background(), store, orgID, testEntityType, []PostableTag{
-			{Key: "team", Value: "PULSE"},
-		}, "u@signoz.io")
-		require.NoError(t, err)
-
-		assert.Empty(t, toCreate)
-		require.Len(t, matched, 1)
-		assert.Equal(t, "Team", matched[0].Key)
-		assert.Equal(t, "Pulse", matched[0].Value)
-	})
-
-	t.Run("propagates validation error from any input", func(t *testing.T) {
-		store := &fakeStore{}
-		_, _, err := Resolve(context.Background(), store, valuer.GenerateUUID(), testEntityType, []PostableTag{
-			{Key: "team", Value: "pulse"},
-			{Key: "", Value: "x"},
-		}, "u@signoz.io")
-		require.Error(t, err)
-	})
-
-	t.Run("propagates slash validation error", func(t *testing.T) {
-		store := &fakeStore{}
-		_, _, err := Resolve(context.Background(), store, valuer.GenerateUUID(), testEntityType, []PostableTag{
-			{Key: "team/eng", Value: "pulse"},
-		}, "u@signoz.io")
-		require.Error(t, err)
-		assert.True(t, strings.Contains(err.Error(), "/"), "error should reference the disallowed character")
-	})
-}

tests/fixtures/serviceaccount.py

@@ -73,6 +73,30 @@ def get_first_key_id(signoz: types.SigNoz, token: str, service_account_id: str)
     return resp.json()["data"][0]["id"]
 
 
+def create_service_account_with_roles(signoz: types.SigNoz, token: str, name: str, roles: list[str]) -> str:
+    """Create a service account and assign multiple roles."""
+    resp = requests.post(
+        signoz.self.host_configs["8080"].get(SERVICE_ACCOUNT_BASE),
+        json={"name": name},
+        headers={"Authorization": f"Bearer {token}"},
+        timeout=5,
+    )
+    assert resp.status_code == HTTPStatus.CREATED, resp.text
+    service_account_id = resp.json()["data"]["id"]
+
+    for role in roles:
+        role_id = find_role_by_name(signoz, token, role)
+        role_resp = requests.post(
+            signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
+            json={"id": role_id},
+            headers={"Authorization": f"Bearer {token}"},
+            timeout=5,
+        )
+        assert role_resp.status_code == HTTPStatus.NO_CONTENT, role_resp.text
+
+    return service_account_id
+
+
 def find_service_account_by_name(signoz: types.SigNoz, token: str, name: str) -> dict:
     """Find a service account by name from the list endpoint."""
     list_resp = requests.get(

tests/integration/tests/serviceaccount/04_roles.py

@@ -44,13 +44,13 @@ def test_assign_role_to_service_account(
     create_user_admin: types.Operation,  # pylint: disable=unused-argument
     get_token: Callable[[str, str], str],
 ):
-    """POST /{id}/roles replaces existing role, verify via GET."""
+    """POST /{id}/roles adds a role alongside existing ones."""
     token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD)
 
     # create service account with viewer role
     service_account_id = create_service_account(signoz, token, "sa-assign-role", role="signoz-viewer")
 
-    # assign editor role (replaces viewer)
+    # assign editor role (additive — viewer stays)
     editor_role_id = find_role_by_name(signoz, token, "signoz-editor")
     assign_resp = requests.post(
         signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
@@ -60,7 +60,7 @@ def test_assign_role_to_service_account(
     )
     assert assign_resp.status_code == HTTPStatus.NO_CONTENT, assign_resp.text
 
-    # verify only editor role is present (viewer was replaced)
+    # verify both viewer and editor roles are present
     roles_resp = requests.get(
         signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
         headers={"Authorization": f"Bearer {token}"},
@@ -68,9 +68,31 @@ def test_assign_role_to_service_account(
     )
     assert roles_resp.status_code == HTTPStatus.OK, roles_resp.text
     role_names = [r["name"] for r in roles_resp.json()["data"]]
-    assert len(role_names) == 1
+    assert len(role_names) == 2
+    assert "signoz-viewer" in role_names
     assert "signoz-editor" in role_names
-    assert "signoz-viewer" not in role_names
+
+    # assign admin role — all three should be present
+    admin_role_id = find_role_by_name(signoz, token, "signoz-admin")
+    assign_resp = requests.post(
+        signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
+        json={"id": admin_role_id},
+        headers={"Authorization": f"Bearer {token}"},
+        timeout=5,
+    )
+    assert assign_resp.status_code == HTTPStatus.NO_CONTENT, assign_resp.text
+
+    roles_resp = requests.get(
+        signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
+        headers={"Authorization": f"Bearer {token}"},
+        timeout=5,
+    )
+    assert roles_resp.status_code == HTTPStatus.OK, roles_resp.text
+    role_names = [r["name"] for r in roles_resp.json()["data"]]
+    assert len(role_names) == 3
+    assert "signoz-viewer" in role_names
+    assert "signoz-editor" in role_names
+    assert "signoz-admin" in role_names
 
 
 def test_assign_role_idempotent(
@@ -103,16 +125,16 @@ def test_assign_role_idempotent(
     assert role_names.count("signoz-viewer") == 1
 
 
-def test_assign_role_replaces_access(
+def test_assign_role_expands_access(
     signoz: types.SigNoz,
     create_user_admin: types.Operation,  # pylint: disable=unused-argument
     get_token: Callable[[str, str], str],
 ):
-    """After role replacement, SA loses old permissions and gains new ones."""
+    """Adding a higher-privilege role expands the SA's access."""
     token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD)
 
     # create SA with viewer role and an API key
-    service_account_id, api_key = create_service_account_with_key(signoz, token, "sa-role-replace-access", role="signoz-viewer")
+    service_account_id, api_key = create_service_account_with_key(signoz, token, "sa-role-expand-access", role="signoz-viewer")
 
     # viewer should get 403 on admin-only endpoint
     resp = requests.get(
@@ -122,7 +144,7 @@ def test_assign_role_replaces_access(
     )
     assert resp.status_code == HTTPStatus.FORBIDDEN, f"Expected 403 for viewer on admin endpoint, got {resp.status_code}: {resp.text}"
 
-    # assign admin role (replaces viewer)
+    # assign admin role (additive — viewer stays)
     admin_role_id = find_role_by_name(signoz, token, "signoz-admin")
     assign_resp = requests.post(
         signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
@@ -138,9 +160,9 @@ def test_assign_role_replaces_access(
         headers={"SIGNOZ-API-KEY": api_key},
         timeout=5,
     )
-    assert resp.status_code == HTTPStatus.OK, f"Expected 200 for admin on admin endpoint, got {resp.status_code}: {resp.text}"
+    assert resp.status_code == HTTPStatus.OK, f"Expected 200 after adding admin role, got {resp.status_code}: {resp.text}"
 
-    # verify only admin role is present
+    # verify both roles are present
     roles_resp = requests.get(
         signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
         headers={"Authorization": f"Bearer {token}"},
@@ -148,9 +170,9 @@ def test_assign_role_replaces_access(
     )
     assert roles_resp.status_code == HTTPStatus.OK, roles_resp.text
     role_names = [r["name"] for r in roles_resp.json()["data"]]
-    assert len(role_names) == 1
+    assert len(role_names) == 2
     assert "signoz-admin" in role_names
-    assert "signoz-viewer" not in role_names
+    assert "signoz-viewer" in role_names
 
 
 def test_remove_role_from_service_account(
@@ -158,13 +180,22 @@ def test_remove_role_from_service_account(
     create_user_admin: types.Operation,  # pylint: disable=unused-argument
     get_token: Callable[[str, str], str],
 ):
-    """DELETE /{id}/roles/{rid} revokes a role."""
+    """DELETE /{id}/roles/{rid} revokes one role while keeping others."""
     token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD)
     service_account_id = create_service_account(signoz, token, "sa-remove-role", role="signoz-editor")
 
-    editor_role_id = find_role_by_name(signoz, token, "signoz-editor")
+    # add admin role (now has editor + admin)
+    admin_role_id = find_role_by_name(signoz, token, "signoz-admin")
+    assign_resp = requests.post(
+        signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
+        json={"id": admin_role_id},
+        headers={"Authorization": f"Bearer {token}"},
+        timeout=5,
+    )
+    assert assign_resp.status_code == HTTPStatus.NO_CONTENT, assign_resp.text
 
-    # remove the role
+    # remove editor role
+    editor_role_id = find_role_by_name(signoz, token, "signoz-editor")
     resp = requests.delete(
         signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles/{editor_role_id}"),
         headers={"Authorization": f"Bearer {token}"},
@@ -172,7 +203,7 @@ def test_remove_role_from_service_account(
     )
     assert resp.status_code == HTTPStatus.NO_CONTENT, resp.text
 
-    # verify role is gone
+    # verify editor is gone but admin remains
     roles_resp = requests.get(
         signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
         headers={"Authorization": f"Bearer {token}"},
@@ -181,6 +212,7 @@ def test_remove_role_from_service_account(
     assert roles_resp.status_code == HTTPStatus.OK, roles_resp.text
     role_names = [r["name"] for r in roles_resp.json()["data"]]
     assert "signoz-editor" not in role_names
+    assert "signoz-admin" in role_names
 
 
 def test_remove_role_verify_access_lost(