chore: analytic events added for reactive behaviour change#40917
chore: analytic events added for reactive behaviour change#40917
Conversation
WalkthroughThis update introduces analytics tracking for changes in the "run behaviour" of actions and executables. New constants and event types are added, analytics events are sent when run behaviour changes both on user action and system updates, and relevant data is included in analytics payloads. Constructor dependencies are updated to support the analytics service. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LayoutActionService
participant AnalyticsService
User->>LayoutActionService: setRunBehaviour(actionId, newRunBehaviour)
LayoutActionService->>LayoutActionService: Save oldRunBehaviour
LayoutActionService->>LayoutActionService: Update action with newRunBehaviour
LayoutActionService->>AnalyticsService: sendRunBehaviourChangedAnalytics(action, oldRunBehaviour, userInitiated=true)
AnalyticsService-->>LayoutActionService: Event sent
LayoutActionService-->>User: Return updated action
sequenceDiagram
participant System
participant OnLoadExecutablesUtil
participant AnalyticsService
System->>OnLoadExecutablesUtil: updateExecutablesRunBehaviour(list)
OnLoadExecutablesUtil->>OnLoadExecutablesUtil: Store oldRunBehaviour for each executable
OnLoadExecutablesUtil->>OnLoadExecutablesUtil: Update runBehaviour
OnLoadExecutablesUtil->>AnalyticsService: sendRunBehaviourChangedAnalytics(executable, oldRunBehaviour, userInitiated=false)
AnalyticsService-->>OnLoadExecutablesUtil: Event sent
OnLoadExecutablesUtil-->>System: Return updated executables
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java (1)
346-363: Raw Map type – add generics
Map data = new HashMap<>();loses type safety and triggers unchecked warnings.
UseMap<String, Object>.- Map<String, Object> data = new HashMap<>(); + Map<String, Object> data = new HashMap<>();app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java (1)
313-314: Minor:oldRunBehaviourMapcan be avoidedSince
executableis mutable, capturingexecutable.getRunBehaviour()into a local variable right before the update (and passing it downstream viaTuple2or a POJO) removes the need for an external map and avoids key-mismatch bugs.Not blocking—just food for thought.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/AnalyticsEvents.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/FieldNameCE.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java(6 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilImpl.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java(3 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: server-spotless / spotless-check
- GitHub Check: server-unit-tests / server-unit-tests
🔇 Additional comments (3)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/AnalyticsEvents.java (1)
27-28: Event name spelling & placement look okayThe new constant is consistent with the existing
"*_RUN_*"naming pattern and passes a string value, so the enum won’t default-lower-case it inadvertently.
No action needed.app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/FieldNameCE.java (1)
212-214: Constant added correctlyThe new key fits the existing constant style.
No further remarks.app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilImpl.java (1)
24-34: Constructor update LGTM
AnalyticsServiceis threaded through correctly and forwarded to the superclass.
No issues spotted.
...ith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java
Show resolved
Hide resolved
...appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java
Show resolved
Hide resolved
...th-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java
Show resolved
Hide resolved
...th-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/server/appsmith-server/src/test/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImplTest.java (2)
63-76: Avoid per-test boilerplate – centraliseAnalyticsServicestubbing insetUp()Every test repeats the same
doAnswer(...).when(onLoadExecutablesUtilCE).sendRunBehaviourChangedAnalytics(...)block.
Moving this stub to the commonsetUp()method (or a private helper) keeps individual test bodies focused on their intent and reduces maintenance overhead.@BeforeEach public void setUp() { onLoadExecutablesUtilCE = spy(new OnLoadExecutablesUtilCEImpl( astService, objectMapper, executableOnLoadService, observationRegistry, observationHelper, featureFlagService, analyticsService)); + // Stub once for all tests + doAnswer(inv -> Mono.just(inv.getArgument(0))) + .when(onLoadExecutablesUtilCE) + .sendRunBehaviourChangedAnalytics(any(Executable.class), any(RunBehaviourEnum.class)); ObservationRegistry.ObservationConfig mockObservationConfig = Mockito.mock(ObservationRegistry.ObservationConfig.class); Mockito.when(observationRegistry.observationConfig()).thenReturn(mockObservationConfig); }
108-111: Duplicate stubbing blocks now redundantOnce the common stub is moved to
setUp(), these duplicateddoAnswerblocks can be deleted, trimming ~30 lines and making the individual tests clearer.Also applies to: 152-155, 199-202, 273-276, 313-316, 409-412, 468-471
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java(6 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImplTest.java(11 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: server-unit-tests / server-unit-tests
- GitHub Check: server-spotless / spotless-check
...erver/src/test/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImplTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java (1)
20-44: Constructor now takes 12 services – consider refactor for readability.The ballooning parameter list signals high coupling and hurts testability. A parameter object, builder, or Spring’s
@RequiredArgsConstructorwith dedicated “context” holder would simplify this signature.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java(6 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilImpl.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java(3 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java
- app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilImpl.java
- app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: server-unit-tests / server-unit-tests
- GitHub Check: server-spotless / spotless-check
🔇 Additional comments (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java (1)
20-32:Details
✅ Verification successful
Verify for new circular‐dependency risks after adding
ApplicationService.Injecting
ApplicationServicehere may introduce a Spring circular-reference ifApplicationService(directly or indirectly) depends onLayoutActionServiceImpl. Please confirm start-up still succeeds and there are no proxy-bean fall-backs.Run this quick scan:
🏁 Script executed:
#!/bin/bash # Looks for any constructor/field injection of LayoutActionServiceImpl (or its interface) inside ApplicationService impls. rg -n --context 2 -e 'LayoutActionServiceImpl' -e 'LayoutActionService' $(fd --type f --extension java | grep -i 'applicationservice')Length of output: 2341
No circular‐dependency risk detected after adding
ApplicationService
A quick scan confirmed that noApplicationServiceimplementation (CE or CE-compatible) injectsLayoutActionService/LayoutActionServiceImpl. Spring startup will not hit a circular reference.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/RunBehaviourAnalyticsUtils.java (2)
49-60: Duplicate/ambiguous timestamp fieldsYou’re sending both
createdAt(rawInstant/Dateobject) and the string-formattedactionCreated.
Downstream analytics pipelines usually expect a single canonical field, otherwise they’ll coerce or drop one.Consider removing the unformatted
createdAtor renaming one of them for clarity.
70-95: Extract datasource mapping for readabilityThe verbose
putblock is repeated in several classes. Extracting it into a small helper (within this util orDatasourceMapper) makes the main flow more readable and less error-prone.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/server/appsmith-server/src/main/java/com/appsmith/server/domains/RunBehaviourAnalyticsMetadata.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/enums/RunBehaviourUpdateSource.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/RunBehaviourAnalyticsUtils.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java(6 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilImpl.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java(3 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java(4 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImplTest.java(11 hunks)
✅ Files skipped from review due to trivial changes (2)
- app/server/appsmith-server/src/main/java/com/appsmith/server/enums/RunBehaviourUpdateSource.java
- app/server/appsmith-server/src/main/java/com/appsmith/server/domains/RunBehaviourAnalyticsMetadata.java
🚧 Files skipped from review as they are similar to previous changes (5)
- app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java
- app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilImpl.java
- app/server/appsmith-server/src/test/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImplTest.java
- app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java
- app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: server-unit-tests / server-unit-tests
...er/appsmith-server/src/main/java/com/appsmith/server/helpers/RunBehaviourAnalyticsUtils.java
Show resolved
Hide resolved
...er/appsmith-server/src/main/java/com/appsmith/server/helpers/RunBehaviourAnalyticsUtils.java
Show resolved
Hide resolved
|
Tested the events with reactivity:
This is verified locally by setting up segment. |
|
Not merging till all things in EE pass as well https://github.com/appsmithorg/appsmith-ee/pull/7804 |
2 participants
Description
This PR adds / updates following analytic events for reactivity run behaviour changes either by system or by user:
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Sanity, @tag.Widget"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/15700973404
Commit: b347ce2
Cypress dashboard.
Tags:
@tag.Sanity, @tag.WidgetSpec:
Tue, 17 Jun 2025 07:46:20 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit