Update what is being audit logged #11833

FelixMalfait · 2025-05-02T09:19:17Z

No need to audit log workflow runs as it's already a form of audit log.
Add more audit log for other objects
Rename MessagingTelemetry to MessagingMonitoring
Merge Analytics and Audit in one (Audit)

greptile-apps

PR Summary

This PR updates audit logging practices and renames telemetry services for better clarity across the Twenty application.

Removes @WorkspaceIsNotAuditLogged decorator from multiple entities (api-key, blocklist, connected-account, etc.) to enable audit logging for important operations
Renames MessagingTelemetryService to MessagingMonitoringService for better semantic clarity
Changes ClickHouse table name from 'migrations' to '_migration' and 'events' to 'event' for consistent naming
Adds @WorkspaceIsNotAuditLogged to WorkflowRunWorkspaceEntity since runs already serve as audit logs
Modifies event table schema to include userId in ORDER BY clause for better indexing

_{24 file(s) reviewed, 3 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

...ages/twenty-server/src/modules/messaging/monitoring/services/messaging-monitoring.service.ts

greptile-apps · 2025-05-02T09:22:52Z

packages/twenty-server/src/modules/timeline/jobs/create-audit-log-from-internal-event.ts

+
+      await this.auditLogRepository.insert(
+        workspaceEventBatch.name,
+        eventProperties,
        workspaceMemberId,
        workspaceEventBatch.name.split('.')[0],


style: splitting on '.' is fragile - consider using a more robust method to extract object name

greptile-apps · 2025-05-02T09:22:52Z

packages/twenty-server/src/modules/timeline/jobs/create-audit-log-from-internal-event.ts

      if (workspaceEventBatch.name.endsWith('.updated')) {
-        analytics.track(OBJECT_RECORD_UPDATED_EVENT, eventData.properties);
+        analytics.track(OBJECT_RECORD_UPDATED_EVENT, eventProperties);
      } else if (workspaceEventBatch.name.endsWith('.created')) {
-        analytics.track(OBJECT_RECORD_CREATED_EVENT, eventData.properties);
+        analytics.track(OBJECT_RECORD_CREATED_EVENT, eventProperties);
      } else if (workspaceEventBatch.name.endsWith('.deleted')) {
-        analytics.track(OBJECT_RECORD_DELETED_EVENT, eventData.properties);
+        analytics.track(OBJECT_RECORD_DELETED_EVENT, eventProperties);
      }


style: consider using a switch statement or event type enum for better maintainability

github-actions · 2025-05-02T09:30:46Z

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:59364

This environment will automatically shut down when the PR is closed or after 5 hours.

FelixMalfait · 2025-05-03T08:47:12Z

@greptileai review again

Copilot

Pull Request Overview

This PR consolidates audit logging by merging analytics into the audit module, updating ClickHouse-related integrations, and renaming/refining related components.

Removed the analytics module and its corresponding tests in favor of the audit module.
Updated the ClickHouse seeds, migrations, and service to reflect new audit log table names and configurations.
Adjusted front-end components by removing deprecated feature flag checks for analytics.

Reviewed Changes

Copilot reviewed 95 out of 98 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/twenty-server/src/engine/core-modules/analytics/services/clickhouse.service.spec.ts	Removed analytics clickhouse service tests.
packages/twenty-server/src/engine/core-modules/analytics/analytics.module.ts	Removed analytics module as part of consolidation.
packages/twenty-server/src/engine/api/graphql/workspace-query-runner/workspace-query-runner.module.ts	Swapped AnalyticsModule for AuditModule and adjusted import order.
packages/twenty-server/src/engine/api/graphql/workspace-query-runner/listeners/telemetry.listener.ts	Replaced analyticsService with auditService for telemetry logging.
packages/twenty-server/src/database/clickHouse/seeds/run-seeds.ts	Updated target table name from 'events' to 'auditEvent'.
packages/twenty-server/src/database/clickHouse/migrations/run-migrations.ts	Changed migration table name from 'migrations' to '_migration'.
packages/twenty-front/src/pages/settings/security/SettingsSecurity.tsx	Removed conditional UI block based on deprecated feature flags.
Other files	Adjusted import paths and renamed identifiers to support new audit logging approach.

Files not reviewed (3)

packages/twenty-server/project.json: Language not supported
packages/twenty-server/src/database/clickHouse/migrations/001-create-audit-event-table.sql: Language not supported
packages/twenty-server/src/database/clickHouse/migrations/003-create-external-event-table.sql: Language not supported

Comments suppressed due to low confidence (3)

packages/twenty-server/src/engine/api/graphql/workspace-query-runner/listeners/telemetry.listener.ts:23

[nitpick] The method name 'createAnalyticsContext' in the auditService may cause confusion given the module merge; consider renaming it to better reflect its audit logging purpose.

this.auditService.createAnalyticsContext({

packages/twenty-front/src/pages/settings/security/SettingsSecurity.tsx:65

[nitpick] The removal of the conditional rendering based on the 'IsApprovedAccessDomainsEnabled' feature flag should be reviewed to ensure it aligns with the overall business logic and feature flag strategy.

{IsApprovedAccessDomainsEnabled && (

packages/twenty-server/src/database/clickHouse/seeds/run-seeds.ts:21

Verify that the change from 'events' to 'auditEvent' is consistent across the codebase and documentation, ensuring that all references and integrations align with the new audit logging design.

table: 'auditEvent',

packages/twenty-server/src/database/clickHouse/migrations/run-migrations.ts

greptile-apps

PR Summary

(updates since last review)

This PR continues the audit logging system overhaul by consolidating analytics and audit functionality into a unified audit module, with significant architectural changes.

Replaces AnalyticsService with AuditService across the application, moving from traditional database-backed audit logs to ClickHouse for better performance
Removes AuditLogWorkspaceEntity and BehavioralEventWorkspaceEntity from standard objects, centralizing event tracking in the new audit system
Adds new ClickHouse tables (auditEvent, pageview, externalEvent) with optimized schemas for different types of events
Removes feature flags IsAnalyticsV2Enabled, IsEventObjectEnabled, and IsApprovedAccessDomainsEnabled as these features are now permanent
Concerning: MessagingMonitoringService has commented-out tracking logic with a TODO about Prometheus replacement, potentially missing audit logs

_{80 file(s) reviewed, 17 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2025-05-03T08:49:16Z

packages/twenty-server/src/database/clickHouse/clickHouse.service.spec.ts

+      jest
+        .spyOn(service, 'connectToClient')
+        .mockResolvedValueOnce(mockClickHouseClient);


logic: Mocking connectToClient while testing connectToClient creates a circular dependency and may mask real issues. Consider testing the actual implementation instead.

greptile-apps · 2025-05-03T08:49:17Z

packages/twenty-server/src/database/clickHouse/clickHouse.service.spec.ts

+      mockClickHouseClient.query.mockResolvedValueOnce({
+        json: jest.fn().mockResolvedValueOnce([{ id: 1, name: 'test' }]),
+      } as any);


style: Type assertion to 'any' could hide type mismatches. Consider creating a proper mock type that matches ClickHouseClient's QueryResult interface.

packages/twenty-server/src/database/clickHouse/migrations/003-create-external-event-table.sql

packages/twenty-server/src/engine/core-modules/audit/README.md

greptile-apps · 2025-05-03T08:53:34Z

packages/twenty-server/src/engine/core-modules/audit/types/events.type.ts

+  ObjectRecordDeletedTrackEvent,
+} from 'src/engine/core-modules/audit/utils/events/track/object-record/object-record-delete';


style: File name in import path contains 'delete' but the event name uses 'deleted'. Consider renaming file to match event name for consistency

greptile-apps · 2025-05-03T08:56:54Z

.../twenty-server/src/engine/core-modules/domain-manager/services/custom-domain.service.spec.ts

          useValue: {
            createAnalyticsContext: AnalyticsContextMock,
          },


style: Mock should be renamed from createAnalyticsContext to createAuditContext to match service name changes

greptile-apps · 2025-05-03T08:58:00Z

packages/twenty-server/src/engine/core-modules/user/user.resolver.ts

@@ -75,7 +75,7 @@ export class UserResolver {
    private readonly onboardingService: OnboardingService,
    private readonly userVarService: UserVarsService,
    private readonly fileService: FileService,
-    private readonly analyticsService: AnalyticsService,
+    private readonly auditService: AuditService,


style: The auditService is injected but not used anywhere in this resolver. Consider removing if unused.

...ages/twenty-server/src/modules/messaging/monitoring/services/messaging-monitoring.service.ts

…create-external-event-table.sql Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

FelixMalfait added 2 commits May 2, 2025 11:18

Update what is being audit logged

69a9d7d

Disable monitoring

ea88a13

greptile-apps bot reviewed May 2, 2025

View reviewed changes

FelixMalfait added 2 commits May 2, 2025 11:48

Review

a1cf25c

Begin refacto

fded81e

prastoin added -PR: wip -PR: awaiting author labels May 2, 2025

FelixMalfait added 8 commits May 2, 2025 19:40

Refactoring

b342991

Fix casing

8c872c1

Lint

ef0c9f1

Folder casing issue

9829f16

More renaming

4e8a2b8

Renaming

1cdb7ff

Lint

f5acf00

Fix test

069b3a2

FelixMalfait requested a review from Copilot May 3, 2025 08:47

Copilot AI reviewed May 3, 2025

View reviewed changes

packages/twenty-server/src/database/clickHouse/migrations/run-migrations.ts Show resolved Hide resolved

Remove test

e052929

greptile-apps bot reviewed May 3, 2025

View reviewed changes

FelixMalfait and others added 6 commits May 3, 2025 12:07

More renaming

c06d907

Update packages/twenty-server/src/database/clickHouse/migrations/003-…

85577f4

…create-external-event-table.sql Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

Update packages/twenty-server/src/database/clickHouse/migrations/003-…

f28b0d8

…create-external-event-table.sql Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

Update packages/twenty-server/src/engine/core-modules/audit/README.md

1dec4c8

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

Test

756f818

More renaming

3f8b62f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update what is being audit logged #11833

Update what is being audit logged #11833

FelixMalfait commented May 2, 2025 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot May 2, 2025

greptile-apps bot May 2, 2025

github-actions bot commented May 2, 2025 •

edited

Loading

FelixMalfait commented May 3, 2025

Copilot AI left a comment

greptile-apps bot left a comment

greptile-apps bot May 3, 2025

greptile-apps bot May 3, 2025

greptile-apps bot May 3, 2025

greptile-apps bot May 3, 2025

greptile-apps bot May 3, 2025

		ObjectRecordDeletedTrackEvent,
		} from 'src/engine/core-modules/audit/utils/events/track/object-record/object-record-delete';

Update what is being audit logged #11833

Are you sure you want to change the base?

Update what is being audit logged #11833

Conversation

FelixMalfait commented May 2, 2025 • edited Loading

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot May 2, 2025

Choose a reason for hiding this comment

greptile-apps bot May 2, 2025

Choose a reason for hiding this comment

github-actions bot commented May 2, 2025 • edited Loading

FelixMalfait commented May 3, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot May 3, 2025

Choose a reason for hiding this comment

greptile-apps bot May 3, 2025

Choose a reason for hiding this comment

greptile-apps bot May 3, 2025

Choose a reason for hiding this comment

greptile-apps bot May 3, 2025

Choose a reason for hiding this comment

greptile-apps bot May 3, 2025

Choose a reason for hiding this comment

FelixMalfait commented May 2, 2025 •

edited

Loading

github-actions bot commented May 2, 2025 •

edited

Loading