Add metadata to Client for MAU tracking #942
Conversation
WalkthroughThis pull request adds a new Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant R as RPCClient
participant S as Server
C->>R: activateClient({ clientKey, metadata })
R->>S: Send activation request with metadata
S-->>R: Activation response
R-->>C: Activation confirmation
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
packages/sdk/src/api/yorkie/v1/yorkie_pb.tsOops! Something went wrong! :( ESLint: 8.19.0 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/packages/sdk".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "packages/sdk/.eslintrc.js » ../../.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/sdk/src/client/client.tsOops! Something went wrong! :( ESLint: 8.19.0 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/packages/sdk".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "packages/sdk/.eslintrc.js » ../../.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
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 @@
## main #942 +/- ##
==========================================
+ Coverage 78.50% 78.54% +0.04%
==========================================
Files 63 63
Lines 5419 5421 +2
Branches 997 998 +1
==========================================
+ Hits 4254 4258 +4
+ Misses 874 873 -1
+ Partials 291 290 -1 ☔ View full report in Codecov by Sentry. |
emplam27
force-pushed
the
client-user
branch
from
b9d8ca2 to
35edee7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/sdk/src/client/client.ts (2)
194-194: Consider adding validation for metadata values.While the metadata field is correctly defined, consider adding validation to ensure that both keys and values in the metadata map are non-empty strings and don't contain sensitive information.
+private validateMetadata(metadata: Record<string, string>): void { + for (const [key, value] of Object.entries(metadata)) { + if (!key || !value) { + throw new YorkieError(Code.ErrInvalidArgument, 'Metadata keys and values must be non-empty strings'); + } + // Add additional validation as needed + } +}
219-219: Consider adding size limit for metadata.To prevent potential abuse, consider adding a size limit for the metadata object.
-this.metadata = opts.metadata || {}; +const metadata = opts.metadata || {}; +const MAX_METADATA_SIZE = 1024; // 1KB limit +const metadataSize = JSON.stringify(metadata).length; +if (metadataSize > MAX_METADATA_SIZE) { + throw new YorkieError(Code.ErrInvalidArgument, `Metadata size exceeds limit of ${MAX_METADATA_SIZE} bytes`); +} +this.metadata = metadata;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/sdk/src/api/yorkie/v1/yorkie.proto(1 hunks)packages/sdk/src/api/yorkie/v1/yorkie_pb.ts(2 hunks)packages/sdk/src/client/client.ts(4 hunks)
🔇 Additional comments (4)
packages/sdk/src/api/yorkie/v1/yorkie_pb.ts (1)
34-37: LGTM! The metadata field is correctly implemented.The metadata field is properly defined as a map of string key-value pairs and correctly initialized as an empty object. The field definition in the static fields list is also accurate.
Also applies to: 48-48
packages/sdk/src/client/client.ts (2)
131-135: LGTM! The metadata property is well documented.The metadata property in ClientOptions is properly documented with a clear description of its purpose.
278-281: LGTM! The metadata is correctly included in the activate request.The metadata is properly included in the activateClient RPC call.
packages/sdk/src/api/yorkie/v1/yorkie.proto (1)
44-44: LGTM! The metadata field is correctly defined in the proto file.The metadata field is properly defined as a map with string keys and values, and correctly numbered as field 2.
hackerwins
changed the title
What this PR does / why we need it?
Add
metadataanduser_idin client info for MAU measurement.metadata: Additional data from users. Stored in yorkie analytic storageuser_id: Identifier of client's user. MAU is measured by the number of users.Any background context you want to provide?
What are the relevant tickets?
Address yorkie-team/yorkie#1130
Checklist
Summary by CodeRabbit