feat: consume the new licenses v3 structure. #6341
Conversation
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
|
contributes to - https://github.com/SigNoz/platform-pod/issues/304 |
vikrantgupta25
changed the title
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
|
contributes to - https://github.com/SigNoz/engineering-pod/issues/1961 ( v3 deprecation of activations from query service ) |
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 7882010 in 1 minute and 13 seconds
More details
- Looked at
661lines of code in10files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. ee/query-service/integrations/signozio/signozio.go:134
- Draft comment:
Movedefer response.Body.Close()immediately after checking for errors to ensure the body is closed in all cases. - Reason this comment was not posted:
Confidence changes required:50%
Thedefer response.Body.Close()should be placed immediately after checking for errors to ensure the body is closed in all cases.
2. ee/query-service/license/db.go:285
- Draft comment:
The error message should be updated to reflect the correct operation:
return fmt.Errorf("update license failed: license marshal error")
- Reason this comment was not posted:
Marked as duplicate.
3. ee/query-service/app/api/api.go:173
- Draft comment:
Avoid using thecomponent/index.tsxfile structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. - Reason this comment was not posted:
Comment was on unchanged code.
4. ee/query-service/app/api/license.go:95
-
Draft comment:
applyLicenseV3is a duplicate ofapplyLicense. Consider refactoring to handle both versions in a single function. -
function
applyLicense(license.go) -
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The functions are similar in structure but handle different types and methods, which suggests they are not duplicates. The comment may not be valid as the functions serve different purposes despite their structural similarity.
The functions could potentially be refactored to share common logic, but the comment's suggestion of duplication might be misleading due to the different types and methods involved.
While the functions are not duplicates, there might be room for refactoring shared logic. However, the comment's suggestion of duplication is not accurate.
The comment should be deleted as it incorrectly identifies the functions as duplicates. They handle different types and methods, indicating distinct purposes.
Workflow ID: wflow_4mGfkKPlUAG29pra
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 15d10a0 in 36 seconds
More details
- Looked at
98lines of code in3files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. ee/query-service/license/manager.go:345
- Draft comment:
Do not add non-ClickHouse related functions to the ClickHouseReader interface. Use the DAO in the telemetry instance to access such information instead. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_pOrMMV4C438f5iCV
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 4535e15 in 31 seconds
More details
- Looked at
15lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. ee/query-service/license/db.go:298
- Draft comment:
The error message should reflect the update operation:
zap.L().Error("error in updating license data: ", zap.Error(err))
- Reason this comment was not posted:
Comment looked like it was already resolved.
2. ee/query-service/license/db.go:298
- Draft comment:
The error message should be consistent with the operation being performed. Consider changing it to reflect the update operation. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_gNGa4hb60xifQxml
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
❌ Changes requested. Incremental review on ee510e6 in 44 seconds
More details
- Looked at
105lines of code in4files - Skipped
0files when reviewing. - Skipped posting
5drafted comments based on config settings.
1. ee/query-service/license/db.go:67
- Draft comment:
Consider adding error handling for thejson.Unmarshalcall to prevent potential panics due to malformed JSON. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The code already includes error handling forjson.Unmarshalby returning an error if unmarshalling fails. The comment seems to suggest something that is already implemented, making it redundant.
I might be missing a more nuanced form of error handling that the comment is suggesting, but the current implementation does handle errors by returning them.
The existing error handling seems sufficient for the context, as it prevents further execution with malformed data.
The comment is redundant because the code already handles errors fromjson.Unmarshal.
2. ee/query-service/app/api/license.go:95
- Draft comment:
Consider logging the error before callingRespondErrorto aid in debugging. - Reason this comment was not posted:
Confidence changes required:50%
ThelistLicensesV3function inapi/license.godoes not log the error whenRespondErroris called. Logging the error would help in debugging issues.
3. ee/query-service/app/api/license.go:91
- Draft comment:
Avoid using inline styles in React components. Instead, use external stylesheets, CSS classes, or styled components. This is applicable in other parts of the code as well. - Reason this comment was not posted:
Comment was on unchanged code.
4. ee/query-service/license/db.go:52
- Draft comment:
Avoid using inline styles in React components. Instead, use external stylesheets, CSS classes, or styled components. This is applicable in other parts of the code as well. - Reason this comment was not posted:
Marked as duplicate.
5. ee/query-service/license/manager.go:222
- Draft comment:
Avoid using inline styles in React components. Instead, use external stylesheets, CSS classes, or styled components. This is applicable in other parts of the code as well. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_jqCsBEXJzrHzHMe7
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
vikrantgupta25
changed the title
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
1 similar comment
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
| ah.Respond(w, l) | ||
| } | ||
|
|
||
| func (ah *APIHandler) refreshLicensesV3(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
In the refreshLicenses call, you will only get the license key. Nothing else will be given.
There was a problem hiding this comment.
do we need a license key here ? what if the key here doesn't correspond to the active license key currently ? then do set that as active? Since by design each deployment will have only once license and query service is aware of the active license we do not specifically need the license key here
|
The important thing to keep in mind is:
|
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 508c8a5 in 1 minute and 12 seconds
More details
- Looked at
429lines of code in5files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. ee/query-service/license/db.go:295
- Draft comment:
TheUpdateLicenseV3function should marshal only theDatafield of the license, not the entire license object. This issue is also present in theUpdateLicenseV3function. - Reason this comment was not posted:
Comment was on unchanged code.
2. ee/query-service/integrations/signozio/signozio.go:121
- Draft comment:
Consider returning the map directly instead of a pointer to a map inValidateLicenseV3. This simplifies the function signature and usage. - Reason this comment was not posted:
Confidence changes required:50%
TheValidateLicenseV3function returns a pointer to a map, which is unconventional and can lead to unnecessary complexity. It would be better to return the map directly.
3. ee/query-service/model/license.go:123
- Draft comment:
Use type assertions with a check to avoid runtime panics inParseFeaturesV3. This issue is also present in theParseFeaturesV3method. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_1F0uPK0mjj5XZtRA
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 9214f7c in 37 seconds
More details
- Looked at
19lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. ee/query-service/license/db.go:120
- Draft comment:
Do not add non-ClickHouse related functions likeCreateFeatureto the ClickHouseReader interface. Use the DAO in the telemetry instance instead. This also applies toGetFeature,GetAllFeatures,UpdateFeature, andInitFeatures. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_zNXXb8z2KP1iQWAE
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
❌ Changes requested. Incremental review on cc645c5 in 51 seconds
More details
- Looked at
89lines of code in3files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. ee/query-service/license/db.go:324
- Draft comment:
Error message should mention 'update' instead of 'insert'. - Reason this comment was not posted:
Comment was on unchanged code.
2. ee/query-service/license/manager.go:360
- Draft comment:
Consider implementing a comparison between old and new licenses as suggested by the comment. - Reason this comment was not posted:
Confidence changes required:50%
The comment in the RefreshLicense function suggests a comparison check is needed between old and new licenses. This is a potential area for improvement or a reminder for future work.
3. ee/query-service/license/db.go:322
- Draft comment:
Avoid using inline styles in React components. Use external stylesheets, CSS classes, or styled components instead. This is also applicable in other parts of the code where inline styles are used. - Reason this comment was not posted:
Comment was on unchanged code.
4. ee/query-service/license/manager.go:362
- Draft comment:
Avoid using inline styles in React components. Use external stylesheets, CSS classes, or styled components instead. This is also applicable in other parts of the code where inline styles are used. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_O4MYn7bTc146pBt5
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Summary
zeusand refreshing the license fromzeuspost any update to the plan of the license or validity etc.Related Issues / PR's
contributes to - https://github.com/SigNoz/platform-pod/issues/304
Screenshots
NA
Affected Areas and Manually Tested Areas
Important
Introduce and integrate a new license v3 structure with new endpoints, database schema, and management logic.
/api/v3/licensesand/api/v3/licenses/refreshendpoints inapi.gofor applying and refreshing licenses.applyLicenseV3,refreshLicensesV3, andlistLicensesV3inlicense.go.ActivateV3,RefreshLicense, andValidateV3methods inmanager.go.StartManagerinmanager.goto supportuseLicensesV3flag.licenses_v3table insqlite/init.go.InsertLicenseV3andUpdateLicenseV3indb.go.LicenseV3struct inlicense.go.ParseFeaturesV3method inlicense.go.ServerOptionsinserver.goto includeUseLicensesV3flag.ValidateLicenseV3function insignozio.go.This description was created by
for cc645c5. It will automatically update as commits are pushed.