-
Notifications
You must be signed in to change notification settings - Fork 5
feat: add VWO destination integration #4753
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
|
Thank you @zeeshan-vwo for contributing this PR. |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Note
|
| Cohort / File(s) | Summary |
|---|---|
Configuration src/v0/destinations/vwo/config.ts |
Adds BASE_URL, region-specific API_ENDPOINTS (DEFAULT, EU, AS), DESTINATION_NAME, and readonly SUPPORTED_EVENT_TYPES (['track']). Exports constants. |
Types & Validation src/v0/destinations/vwo/types.ts |
Adds VWODestinationConfigSchema (zod) with required accountId and optional region enum (DEFAULT |
Utilities src/v0/destinations/vwo/utils.ts |
Adds buildVWOPayload, validateVWOEvent, getUserIdentifier, and buildEndpoint. Implements payload sanitization and throws InstrumentationError when validation fails. Exports all helpers. |
Transform src/v0/destinations/vwo/transform.ts |
Implements process(event) for single-event transformation and processRouterDest(inputs, reqMetadata) for batched routing. Validates message/config, resolves user identifier, builds payload, selects region endpoint, encodes endpoint URL, and returns standardized POST request objects with headers and JSON body. |
Sequence Diagram(s)
sequenceDiagram
participant Router
participant Transform as transform.ts
participant Utils as utils.ts
participant Config as config.ts
participant VWO as VWO API
Router->>Transform: process(VWORouterRequest)
Transform->>Utils: validateVWOEvent(message)
alt valid
Transform->>Utils: getUserIdentifier(message)
Utils-->>Transform: userId
Transform->>Utils: buildVWOPayload(message, userId, sessionId)
Utils-->>Transform: VWOEventPayload
Transform->>Config: read region / API_ENDPOINTS
Config-->>Transform: selected base endpoint
Transform->>Utils: buildEndpoint(baseEndpoint, accountId, eventName)
Utils-->>Transform: encoded endpoint URL
Transform-->>Router: POST request (headers + JSON body)
Router->>VWO: POST request to endpoint
else invalid
Transform-->>Router: error (InstrumentationError / ConfigurationError)
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
- Pay attention to validation/error types in
utils.ts(InstrumentationError vs ConfigurationError). - Verify payload field mappings and removal of undefined/null values.
- Confirm region-to-endpoint mapping and URL encoding in
buildEndpoint. - Ensure
processRouterDestbatching conforms to router utilities and expected request shape/headers.
Suggested reviewers
- krishna2020
- maheshkutty
- koladilip
- sivashanmukh
Pre-merge checks and finishing touches
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description check | ✅ Passed | The description comprehensively covers changes, objectives, test coverage, and follows the required template structure with all key sections populated. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Title check | ✅ Passed | The title 'feat: add VWO destination integration' directly and clearly summarizes the main change—adding a new VWO destination integration to the codebase. |
Tip
📝 Customizable high-level summaries are now available in beta!
You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
- Provide your own instructions using the
high_level_summary_instructionssetting. - Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
- Use
high_level_summary_in_walkthroughto move the summary from the description to the walkthrough section.
Example instruction:
"Divide the high-level summary into five sections:
- 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
- 📓 References — List relevant issues, discussions, documentation, or related PRs.
- 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
- 📊 Contributor Summary — Include a Markdown table showing contributions:
| Contributor | Lines Added | Lines Removed | Files Changed |- ✔️ Additional Notes — Add any extra reviewer context.
Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.
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.
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/v0/destinations/vwo/transform.ts (1)
80-80: Simplify region handling with validated schema.The region fallback logic here is redundant because the Zod schema in
types.tsalready provides a default value of'DEFAULT'and validates the enum. Theregionfield will always be a valid value ('DEFAULT','EU', or'AS') at runtime.Apply this diff to simplify:
- const apiEndpoint = API_ENDPOINTS[region as keyof typeof API_ENDPOINTS] || API_ENDPOINTS.DEFAULT; + const apiEndpoint = API_ENDPOINTS[region];src/v0/destinations/vwo/utils.ts (1)
71-73: Consider tightening the return type.The function returns an empty string if both
userIdandanonymousIdare missing, butvalidateVWOEventensures at least one exists. The empty string case should never occur in practice.For stricter type safety, you could:
- Add an assertion:
return message.userId || message.anonymousId || throw new Error('Unreachable')- Or document that this function assumes validation has already occurred
- Or use a non-nullable return type if the TypeScript configuration supports it
This is a minor concern since
validateVWOEventis called before this function in the actual flow.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
test/integrations/component.test.tsis excluded by!**/test/**test/integrations/destinations/vwo/processor/data.tsis excluded by!**/test/**test/integrations/destinations/vwo/router/data.tsis excluded by!**/test/**
📒 Files selected for processing (4)
src/v0/destinations/vwo/config.ts(1 hunks)src/v0/destinations/vwo/transform.ts(1 hunks)src/v0/destinations/vwo/types.ts(1 hunks)src/v0/destinations/vwo/utils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ItsSudip
Repo: rudderlabs/rudder-transformer PR: 4497
File: src/v0/destinations/tiktok_ads/transformV2.js:686-691
Timestamp: 2025-07-15T06:27:07.528Z
Learning: For TikTok Ads destination in src/v0/destinations/tiktok_ads/transformV2.js, there are plans for a future enhancement to group events by eventSource when batching to ensure all events in a batch have the same event_source value, as acknowledged by ItsSudip in PR #4497.
📚 Learning: 2025-07-15T06:27:07.528Z
Learnt from: ItsSudip
Repo: rudderlabs/rudder-transformer PR: 4497
File: src/v0/destinations/tiktok_ads/transformV2.js:686-691
Timestamp: 2025-07-15T06:27:07.528Z
Learning: For TikTok Ads destination in src/v0/destinations/tiktok_ads/transformV2.js, there are plans for a future enhancement to group events by eventSource when batching to ensure all events in a batch have the same event_source value, as acknowledged by ItsSudip in PR #4497.
Applied to files:
src/v0/destinations/vwo/transform.tssrc/v0/destinations/vwo/types.ts
📚 Learning: 2025-05-29T13:29:39.436Z
Learnt from: maheshkutty
Repo: rudderlabs/rudder-transformer PR: 4359
File: src/v0/util/index.js:2272-2272
Timestamp: 2025-05-29T13:29:39.436Z
Learning: The `combineBatchRequestsWithSameJobIds` function in `src/v0/util/index.js` is used by both Mixpanel and Google AdWords Offline Conversions destinations in production. The function assumes metadata is always an array in multiple operations (forEach, filter, sort, map) and needs defensive programming to handle non-array metadata cases to prevent runtime errors.
Applied to files:
src/v0/destinations/vwo/transform.ts
🧬 Code graph analysis (3)
src/v0/destinations/vwo/transform.ts (5)
src/v0/destinations/vwo/types.ts (1)
VWORouterRequest(45-45)src/v0/destinations/vwo/utils.ts (4)
validateVWOEvent(52-62)getUserIdentifier(71-73)buildVWOPayload(13-44)buildEndpoint(83-89)src/v0/destinations/vwo/config.ts (1)
API_ENDPOINTS(21-21)src/v0/util/index.js (3)
defaultRequestConfig(461-475)defaultPostRequestConfig(435-438)simpleProcessRouterDest(1815-1834)src/v0/util/constant.js (1)
JSON_MIME_TYPE(11-11)
src/v0/destinations/vwo/utils.ts (3)
src/types/rudderEvents.ts (1)
RudderMessage(43-43)src/v0/destinations/vwo/types.ts (1)
VWOEventPayload(21-38)src/v0/util/index.js (1)
removeUndefinedAndNullValues(55-55)
src/v0/destinations/vwo/types.ts (3)
src/types/controlPlaneConfig.ts (1)
Destination(31-49)src/types/destinationTransformation.ts (1)
RouterTransformationRequestData(95-106)src/types/rudderEvents.ts (1)
RudderMessage(43-43)
🔇 Additional comments (6)
src/v0/destinations/vwo/types.ts (2)
9-14: LGTM! Well-structured validation schema.The Zod schema provides strong runtime validation with appropriate constraints:
- Required
accountIdwith non-empty validation- Region enum with sensible default
- Passthrough for config extensibility
21-38: LGTM! Clear payload structure definition.The interface clearly defines the VWO event payload structure with proper nesting and the flexible
[key: string]: anyindex signature to accommodate custom event properties.src/v0/destinations/vwo/utils.ts (3)
13-44: LGTM! Payload construction follows best practices.The function correctly:
- Generates unique message IDs and session identifiers
- Safely extracts event properties with fallback
- Protects critical metadata fields by spreading
eventPropertiesfirst, then explicitly settingeventTypeandeventNameto prevent user overrides- Sanitizes the payload with
removeUndefinedAndNullValues
52-62: LGTM! Clear validation with appropriate error messages.The validation logic correctly enforces:
- Event name requirement for track events
- User identifier requirement (userId or anonymousId)
Error messages are clear and actionable.
83-89: LGTM! Proper URL encoding for query parameters.The function correctly uses
encodeURIComponentfor botheventNameandaccountIdto prevent injection issues and handle special characters.src/v0/destinations/vwo/config.ts (1)
7-7: The BASE_URL is correct for production use—no changes needed.The official VWO production API endpoint for offline conversion events is indeed
https://dev.visualwebsiteoptimizer.com. The "dev" prefix in the domain is part of VWO's official production URL and should not be modified. The code is correct as-is.
|
@maheshkutty @krishna2020 can you please review this PR. Please let me know if I have to make any other changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/v0/destinations/vwo/utils.ts (1)
71-79: Consider a more descriptive error message for the unreachable case.While line 78 should be unreachable due to prior validation in
validateVWOEvent, using a more descriptive error message would aid debugging if this invariant is ever violated during refactoring or changes to the call flow.Apply this diff to improve the error message:
if (message.anonymousId) { return message.anonymousId; } - throw new Error('Unreachable'); + throw new InstrumentationError( + 'Neither userId nor anonymousId is available. This should have been caught by validateVWOEvent.' + );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
test/integrations/component.test.tsis excluded by!**/test/**
📒 Files selected for processing (2)
src/v0/destinations/vwo/transform.ts(1 hunks)src/v0/destinations/vwo/utils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: maheshkutty
Repo: rudderlabs/rudder-transformer PR: 4380
File: src/v0/destinations/google_adwords_remarketing_lists/config.js:3-5
Timestamp: 2025-06-04T07:24:58.591Z
Learning: User maheshkutty prefers to handle Google Ads API version upgrades for different destinations in separate pull requests rather than bundling multiple destinations together in a single PR.
Learnt from: ItsSudip
Repo: rudderlabs/rudder-transformer PR: 4497
File: src/v0/destinations/tiktok_ads/transformV2.js:686-691
Timestamp: 2025-07-15T06:27:07.528Z
Learning: For TikTok Ads destination in src/v0/destinations/tiktok_ads/transformV2.js, there are plans for a future enhancement to group events by eventSource when batching to ensure all events in a batch have the same event_source value, as acknowledged by ItsSudip in PR #4497.
📚 Learning: 2025-07-15T06:27:07.528Z
Learnt from: ItsSudip
Repo: rudderlabs/rudder-transformer PR: 4497
File: src/v0/destinations/tiktok_ads/transformV2.js:686-691
Timestamp: 2025-07-15T06:27:07.528Z
Learning: For TikTok Ads destination in src/v0/destinations/tiktok_ads/transformV2.js, there are plans for a future enhancement to group events by eventSource when batching to ensure all events in a batch have the same event_source value, as acknowledged by ItsSudip in PR #4497.
Applied to files:
src/v0/destinations/vwo/transform.ts
📚 Learning: 2025-05-29T13:29:39.436Z
Learnt from: maheshkutty
Repo: rudderlabs/rudder-transformer PR: 4359
File: src/v0/util/index.js:2272-2272
Timestamp: 2025-05-29T13:29:39.436Z
Learning: The `combineBatchRequestsWithSameJobIds` function in `src/v0/util/index.js` is used by both Mixpanel and Google AdWords Offline Conversions destinations in production. The function assumes metadata is always an array in multiple operations (forEach, filter, sort, map) and needs defensive programming to handle non-array metadata cases to prevent runtime errors.
Applied to files:
src/v0/destinations/vwo/transform.ts
🧬 Code graph analysis (2)
src/v0/destinations/vwo/transform.ts (5)
src/v0/destinations/vwo/types.ts (1)
VWORouterRequest(45-45)src/v0/destinations/vwo/utils.ts (4)
validateVWOEvent(52-62)getUserIdentifier(71-79)buildVWOPayload(13-44)buildEndpoint(89-95)src/v0/destinations/vwo/config.ts (1)
API_ENDPOINTS(21-21)src/v0/util/index.js (3)
defaultRequestConfig(461-475)defaultPostRequestConfig(435-438)simpleProcessRouterDest(1815-1834)src/v0/util/constant.js (1)
JSON_MIME_TYPE(11-11)
src/v0/destinations/vwo/utils.ts (3)
src/types/rudderEvents.ts (1)
RudderMessage(43-43)src/v0/destinations/vwo/types.ts (1)
VWOEventPayload(21-38)src/v0/util/index.js (1)
removeUndefinedAndNullValues(55-55)
🔇 Additional comments (6)
src/v0/destinations/vwo/transform.ts (3)
1-35: LGTM! Clear documentation and well-organized imports.The file header provides excellent context about VWO integration features and supported operations. The imports are properly structured and include all necessary dependencies.
110-112: LGTM! Clean wrapper function.The
processfunction provides a clean public API by delegating toprocessEvent. This separation is appropriate for the transformer pattern.
124-130: LGTM! Proper batch processing implementation.The
processRouterDestfunction correctly uses thesimpleProcessRouterDestutility for batch event processing. The emptyprocessParamsobject is appropriate since no additional processing parameters are needed for VWO.src/v0/destinations/vwo/utils.ts (3)
1-4: LGTM! Appropriate imports.All imports are necessary for the utility functions and properly typed.
52-62: LGTM! Comprehensive validation logic.The validation function properly checks required fields for track events and ensures either
userIdoranonymousIdis present. Error messages are clear and actionable.
89-95: LGTM! Proper query parameter encoding.The function correctly uses
encodeURIComponentfor both the event name and account ID query parameters, preventing potential URL injection issues. Implementation is clean and secure.
|
@krishna2020 @maheshkutty, can you please review this? Our customers are blocked because of this. We will be very grateful if you can review it as soon as possible. |
Hey, @zeeshan-vwo , I work on the product team at rudderstack. Would you be open to a quick 30-minute call to walk through the integration and answer questions? This would really accelerate our understanding and review process. On our end, we're setting up a VWO test account to explore the platform (would love if this could be free with no trial period so we can continue to test) We're aiming to fast-track integrations that provide clear customer value, so your insights will be incredibly helpful in moving this forward efficiently. |
|
@Drewdodds Sure, we can have a quick call over this. Please feel free to block any of the suitable time from the link below. |
Schedule for Friday at 9:00 EST. Look forward to talking. I will add an engineer from my side as well. Can you [email protected] to the invite? |
|
@Drewdodds Sure, I’ve added [email protected] to the invite, but I saw a warning while adding it—could you please confirm if the invite went through? |
|
This PR is considered to be stale. It has been open for 20 days with no further activity thus it is going to be closed in 7 days. To avoid such a case please consider removing the stale label manually or add a comment to the PR. |
We are still working on this integration. This PR may not be merged but we are working on a PR that will solve for the use cases here as well as others. |
What are the changes introduced in this PR?
This PR introduces a new VWO destination integration for audience sync for targetting. The implementation follows the modern TypeScript-first approach with Zod schemas for runtime validation.
What is the related Linear task?
N/A
Please explain the objectives of your changes below
Overview
VWO is an A/B testing and conversion optimization platform. This integration enables syncing of audience/cohorts from RudderStack to VWO.
Key Features
Files Added
Source Code:
src/v0/destinations/vwo/transform.ts- Main transformation logic with processor and router functionssrc/v0/destinations/vwo/types.ts- TypeScript type definitions and Zod schemassrc/v0/destinations/vwo/utils.ts- Utility functions for payload building and validationsrc/v0/destinations/vwo/config.ts- API endpoints and configuration constantsTests:
test/integrations/destinations/vwo/processor/data.ts- 7 processor test casestest/integrations/destinations/vwo/router/data.ts- 2 router test casesModified:
test/integrations/component.test.ts- Added 'vwo' to INTEGRATIONS_WITH_UPDATED_TEST_STRUCTUREAny changes to existing capabilities/behaviour, mention the reason & what are the changes ?
N/A - This is a new destination integration with no impact on existing destinations.
Any new dependencies introduced with this change?
N/A - Uses existing dependencies (Zod is already in package.json)
Any new generic utility introduced or modified. Please explain the changes.
N/A - Uses existing utility functions from
src/v0/util:defaultRequestConfig()defaultPostRequestConfigsimpleProcessRouterDest()removeUndefinedAndNullValues()Any technical or performance related pointers to consider with the change?
N/A
Test Coverage