feat: add tags for sdk source types #1841
Conversation
saikumarrs
requested review from
lvrach,
cisse21,
debanjan97,
am6010,
ssbeefeater,
ruchiramoitra,
ItsSudip,
shrouti1507,
sandeepdsvs,
krishna2020 and
1abhishekpandey
as code owners
WalkthroughThe pull request introduces updates to multiple JSON configuration files for various sources, including AMP, Android, Cordova, DotNet, Flutter, Go, iOS, Java, JavaScript, Node, PHP, Python, React Native, Ruby, Rust, and Unity. Each configuration file now contains an Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1841 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 53 53
Branches 7 7
=========================================
Hits 53 53 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
src/configurations/sources/php/db-config.json (1)
4-7: Consider updating "type" field for consistencyThe
"type": "cloud"seems inconsistent with the new tagging system. Consider either:
- Updating it to
"type": "server"to align with the new SDK type classification, or- Document the relationship between "cloud" type and "server-sdk" tag
src/configurations/sources/ruby/db-config.json (1)
4-7: Consider documenting the tagging systemSince this is a new feature that adds SDK type classification, consider:
- Adding documentation about the tagging system and its purpose
- Creating a schema to validate the allowed tag values
- Documenting the relationship between the "type" field and "tag" field
src/configurations/sources/rust/db-config.json (1)
4-7: LGTM! Consider documenting the SDK classification criteria.The "server-sdk" tag is appropriate for Rust. To maintain consistency as the project grows:
Consider adding documentation that outlines:
- Criteria for classifying SDKs as client-side vs server-side
- List of supported SDKs and their classifications
- Process for adding new SDK configurations
src/configurations/sources/javascript/db-config.json (1)
1-3: Consider standardizing JavaScript spellingThe name and displayName fields use "Javascript" spelling. Consider using the standard "JavaScript" spelling for consistency with official branding.
{ - "name": "Javascript", - "displayName": "Javascript", + "name": "JavaScript", + "displayName": "JavaScript",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/configurations/sources/amp/db-config.json(1 hunks)src/configurations/sources/android/db-config.json(1 hunks)src/configurations/sources/cordova/db-config.json(1 hunks)src/configurations/sources/dotnet/db-config.json(1 hunks)src/configurations/sources/flutter/db-config.json(1 hunks)src/configurations/sources/go/db-config.json(1 hunks)src/configurations/sources/ios/db-config.json(1 hunks)src/configurations/sources/java/db-config.json(1 hunks)src/configurations/sources/javascript/db-config.json(1 hunks)src/configurations/sources/node/db-config.json(1 hunks)src/configurations/sources/php/db-config.json(1 hunks)src/configurations/sources/python/db-config.json(1 hunks)src/configurations/sources/react_native/db-config.json(1 hunks)src/configurations/sources/ruby/db-config.json(1 hunks)src/configurations/sources/rust/db-config.json(1 hunks)src/configurations/sources/unity/db-config.json(1 hunks)
🔇 Additional comments (13)
src/configurations/sources/amp/db-config.json (1)
4-7: LGTM! Correctly tagged as client-side SDK.
The addition of the "client-sdk" tag is appropriate for AMP, as it operates in the client's browser.
Let's verify the consistency of client-side SDK tagging across related configurations:
✅ Verification successful
Verified: Client-side SDK tagging is consistent across all relevant sources
The verification shows that the "client-sdk" tag is correctly applied to all expected client-side SDKs:
- amp
- android
- cordova
- flutter
- ios
- javascript
- react_native
- unity
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent tagging of client-side SDKs
# Expected: All client-side SDKs (amp, android, cordova, flutter, ios, javascript, react_native, unity) should have "client-sdk" tag
rg -l '"tag":\s*"client-sdk"' src/configurations/sources/*/db-config.json
Length of output: 6334
src/configurations/sources/go/db-config.json (1)
4-7: Consider updating the type field for consistency.
While the "server-sdk" tag is correctly assigned for the Go SDK, the type field still uses "cloud". Consider updating it to maintain consistency with the new SDK type terminology.
Let's verify the consistency of server-side SDK configurations:
src/configurations/sources/ios/db-config.json (1)
4-7: LGTM! Correctly tagged as client-side SDK.
The addition of the "client-sdk" tag is appropriate for iOS, as it operates on mobile devices. The configuration maintains consistency with other mobile platform SDKs.
src/configurations/sources/php/db-config.json (1)
5-7: LGTM: Correct SDK type classification
The addition of the "tag": "server-sdk" is appropriate for PHP, which is indeed a server-side SDK.
src/configurations/sources/java/db-config.json (1)
4-7: Same observations as PHP configuration
The changes mirror the PHP configuration. The same comments apply:
- The server-side SDK classification is correct for Java
- Consider addressing the "type"/"tag" naming inconsistency
src/configurations/sources/ruby/db-config.json (1)
4-7: Verify consistent tagging across all source configurations
The changes follow the same pattern as PHP and Java configurations. While the server-side classification is correct for Ruby, let's ensure consistency across all source configurations.
✅ Verification successful
SDK tagging is correctly implemented for all SDKs, but missing for integrations
The verification shows that all SDK configurations (both client and server) have consistent tagging:
- Client SDKs (client-sdk): AMP, Android, Cordova, Flutter, iOS, JavaScript, React Native, Unity
- Server SDKs (server-sdk): DotNet, Go, Java, Node, PHP, Python, Ruby, Rust
The missing tags are only in integration sources (like Adjust, Salesforce, etc.) and data warehouses (like BigQuery, Snowflake, etc.), which is expected as they are not SDKs.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent tagging across all source configurations
# Expected results:
# 1. All SDKs should have options.tag
# 2. Client SDKs: AMP, Android, Cordova, Flutter, iOS, JavaScript, React Native, Unity
# 3. Server SDKs: DotNet, Go, Java, Node, PHP, Python, Ruby, Rust
# Check if all source configs have the options.tag field
echo "Checking for missing tags..."
find src/configurations/sources -name "db-config.json" -exec sh -c '
if ! jq -e ".options.tag" "{}" >/dev/null; then
echo "Missing tag in: {}"
fi
' \;
# List all SDK types for verification
echo -e "\nVerifying SDK type classifications..."
find src/configurations/sources -name "db-config.json" -exec sh -c '
echo "$(dirname {} | xargs basename): $(jq -r ".options.tag" {})"
' \; | sort
# Check for any inconsistencies between type and tag
echo -e "\nChecking type/tag consistency..."
find src/configurations/sources -name "db-config.json" -exec sh -c '
sdk=$(dirname {} | xargs basename)
type=$(jq -r ".type" {})
tag=$(jq -r ".options.tag" {})
echo "$sdk: type=$type, tag=$tag"
' \; | sort
Length of output: 13567
src/configurations/sources/node/db-config.json (1)
4-7: LGTM! Correctly tagged as server-side SDK.
The addition of the "server-sdk" tag is appropriate for Node.js, which is primarily used for server-side development.
Let's verify consistent tagging across all server-side SDKs:
✅ Verification successful
Let me verify if the configuration files exist for these SDKs before concluding.
Verified: Server-side SDK tagging is consistent across all SDKs
After checking all configuration files, I can confirm that all server-side SDKs (Node.js, Python, Ruby, PHP, Go, Rust, Java, .NET) consistently use the "server-sdk" tag, while client-side SDKs (iOS, Android, JavaScript, React Native, etc.) use "client-sdk" tag.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check tag consistency across server-side SDK configurations
# Expected: All server-side SDKs (Node, Python, Ruby, PHP, etc.) should have tag: "server-sdk"
echo "Checking server SDK tags..."
for sdk in node python ruby php dotnet go rust java; do
jq -r --arg sdk "$sdk" \
'if .options.tag != "server-sdk" then "\($sdk): Incorrect tag: \(.options.tag)" else empty end' \
"src/configurations/sources/$sdk/db-config.json"
done
Length of output: 1826
Script:
#!/bin/bash
# Check which SDK configuration files exist
echo "Existing SDK configurations:"
fd "db-config.json" src/configurations/sources/
# For existing configs, let's see their content
echo -e "\nConfiguration contents:"
for file in $(fd "db-config.json" src/configurations/sources/); do
echo -e "\n=== $file ==="
cat "$file"
done
Length of output: 75971
src/configurations/sources/dotnet/db-config.json (1)
4-7: Verify .NET SDK classification and consider name consistency.
While the structure is correct, please verify:
- Does this specifically represent server-side .NET SDK? .NET can be used for both client and server development.
- The display name uses ".Net" while the name uses "DotNet". Consider standardizing the naming.
✅ Verification successful
Classification is correct and naming is consistent with other SDKs
The verification shows that:
- The "server-sdk" tag is correctly used for .NET SDK, consistent with other server-side SDKs like Java, Python, Ruby, Go, Node, etc.
- The naming convention (.Net/DotNet) follows the pattern used by other SDKs where displayName is user-friendly (e.g., ".Net") and name is code-friendly (e.g., "DotNet").
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are separate configurations for different .NET variants
# Expected: Confirm if we need separate configs for client-side .NET
echo "Checking .NET configurations..."
fd -t f "db-config.json" src/configurations/sources/ -x grep -l "\.Net\|DotNet" {}
Length of output: 206
Script:
#!/bin/bash
# Let's check the content of the .NET configuration and related files
echo "=== Content of .NET configuration ==="
cat src/configurations/sources/dotnet/db-config.json
echo -e "\n=== Searching for .NET related patterns in configurations ==="
rg -i "dotnet|\.net" src/configurations/sources/ -C 2
echo -e "\n=== Checking if there are any client/server distinctions in other SDKs ==="
fd -t f "db-config.json" src/configurations/sources/ -x cat {}
Length of output: 41804
src/configurations/sources/unity/db-config.json (1)
4-7: LGTM! Appropriate tag for Unity SDK
The "client-sdk" tag is correctly applied as Unity applications run on client devices.
src/configurations/sources/android/db-config.json (1)
4-7: LGTM! Appropriate tag for Android SDK
The "client-sdk" tag is correctly applied as Android applications run on client devices.
src/configurations/sources/flutter/db-config.json (1)
4-7: LGTM! Appropriate tag for Flutter SDK
The addition of the "options" object with "tag": "client-sdk" is correct as Flutter is a client-side mobile framework.
src/configurations/sources/cordova/db-config.json (1)
4-7: LGTM! Appropriate tag for Cordova SDK
The addition of the "options" object with "tag": "client-sdk" is correct as Cordova is a client-side mobile framework.
src/configurations/sources/javascript/db-config.json (1)
5-7: LGTM! Appropriate tag for JavaScript web SDK
The addition of the "options" object with "tag": "client-sdk" is correct as this is for the web/browser JavaScript SDK.
@arpl, maybe we should use |
Yes, I think this is better |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/schemas/sources/db-config-schema.json (1)
150-154: LGTM! Consider adding conditional validation.The implementation of
sdkExecutionEnvironmentis clean and well-documented. The enum values appropriately differentiate between client-side and server-side SDKs.Consider adding JSON Schema validation to ensure this property is:
- Required when
typeis an SDK type (amp, android, ios, etc.)- Not allowed when
typeis a non-SDK type (cloud, warehouse, etc.)Here's a suggested schema enhancement using
if/then/else:{ "properties": { "type": { "type": "string", "enum": ["cloud", "cloudSource", "amp", "android", ...] }, "options": { "type": "object", + "allOf": [ + { + "if": { + "properties": { + "type": { + "enum": ["amp", "android", "cordova", "flutter", "ios", "web", "reactnative", "unity"] + } + } + }, + "then": { + "required": ["sdkExecutionEnvironment"] + }, + "else": { + "not": { + "required": ["sdkExecutionEnvironment"] + } + } + } + ], "properties": { "sdkExecutionEnvironment": { "type": "string", "enum": ["server", "client"] } } } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/configurations/sources/amp/db-config.json(1 hunks)src/configurations/sources/android/db-config.json(1 hunks)src/configurations/sources/cordova/db-config.json(1 hunks)src/configurations/sources/dotnet/db-config.json(1 hunks)src/configurations/sources/flutter/db-config.json(1 hunks)src/configurations/sources/go/db-config.json(1 hunks)src/configurations/sources/ios/db-config.json(1 hunks)src/configurations/sources/java/db-config.json(1 hunks)src/configurations/sources/javascript/db-config.json(1 hunks)src/configurations/sources/node/db-config.json(1 hunks)src/configurations/sources/php/db-config.json(1 hunks)src/configurations/sources/python/db-config.json(1 hunks)src/configurations/sources/react_native/db-config.json(1 hunks)src/configurations/sources/ruby/db-config.json(1 hunks)src/configurations/sources/rust/db-config.json(1 hunks)src/configurations/sources/unity/db-config.json(1 hunks)src/schemas/sources/db-config-schema.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- src/configurations/sources/unity/db-config.json
- src/configurations/sources/go/db-config.json
- src/configurations/sources/dotnet/db-config.json
- src/configurations/sources/node/db-config.json
- src/configurations/sources/python/db-config.json
- src/configurations/sources/ruby/db-config.json
- src/configurations/sources/rust/db-config.json
- src/configurations/sources/cordova/db-config.json
- src/configurations/sources/ios/db-config.json
- src/configurations/sources/amp/db-config.json
- src/configurations/sources/javascript/db-config.json
- src/configurations/sources/java/db-config.json
- src/configurations/sources/flutter/db-config.json
- src/configurations/sources/php/db-config.json
- src/configurations/sources/android/db-config.json
- src/configurations/sources/react_native/db-config.json
🔇 Additional comments (1)
src/schemas/sources/db-config-schema.json (1)
150-154: Verify consistent implementation across source configurations
Let's ensure the changes are consistently applied across all source configurations.
✅ Verification successful
Implementation verified across SDK configurations
The verification results show that:
- All client-side SDKs (amp, android, cordova, flutter, ios, javascript, react_native, unity) consistently have
sdkExecutionEnvironmentset to "client" - All server-side SDKs (dotnet, go, java, node, php, python, ruby, rust) consistently have
sdkExecutionEnvironmentset to "server" - Non-SDK sources do not have the
sdkExecutionEnvironmentproperty, as expected
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify sdkExecutionEnvironment implementation across source configs
echo "Checking SDK sources for sdkExecutionEnvironment property..."
# Client-side SDKs
for sdk in amp android cordova flutter ios javascript react_native unity; do
echo "Checking $sdk..."
cat "src/configurations/sources/$sdk/db-config.json" | jq -r '.options.sdkExecutionEnvironment // "MISSING"'
done
echo -e "\nChecking server-side SDKs..."
# Server-side SDKs
for sdk in dotnet go java node php python ruby rust; do
echo "Checking $sdk..."
cat "src/configurations/sources/$sdk/db-config.json" | jq -r '.options.sdkExecutionEnvironment // "MISSING"'
done
echo -e "\nChecking non-SDK sources (should not have the property)..."
# Non-SDK sources
for source in $(find src/configurations/sources -name db-config.json | grep -vE 'amp|android|cordova|flutter|ios|javascript|react_native|unity|dotnet|go|java|node|php|python|ruby|rust'); do
echo "Checking $(dirname $source | xargs basename)..."
if jq -e '.options.sdkExecutionEnvironment' "$source" >/dev/null 2>&1; then
echo "WARNING: Non-SDK source has sdkExecutionEnvironment property"
cat "$source" | jq '.options.sdkExecutionEnvironment'
fi
done
Length of output: 39334
What are the changes introduced in this PR?
I've added tags for SDK source types to indicate client vs server side SDKs.
What is the related Linear task?
https://linear.app/rudderstack/issue/SDK-2767/rename-cloud-to-server-side-sources-or-anything-appropriate-in-the
Please explain the objectives of your changes below
We want to maintain the source of truth in the source definitions indicating the SDK type.
Any changes to existing capabilities/behaviour, mention the reason & what are the changes ?
N/A
Any new dependencies introduced with this change?
N/A
Any new checks got introduced or modified in test suites. Please explain the changes.
N/A
Developer checklist
My code follows the style guidelines of this project
No breaking changes are being introduced.
All related docs linked with the PR?
All changes manually tested?
Any documentation changes needed with this change?
I have executed schemaGenerator tests and updated schema if needed
Are sensitive fields marked as secret in definition config?
My test cases and placeholders use only masked/sample values for sensitive fields
Is the PR limited to 10 file changes & one task?
Reviewer checklist
Is the type of change in the PR title appropriate as per the changes?
Verified that there are no credentials or confidential data exposed with the changes.
Summary by CodeRabbit
New Features
Changes by Source
"options": { "sdkExecutionEnvironment": "client" }"options": { "sdkExecutionEnvironment": "server" }