-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: add APIs for third party api feat #7005
base: main
Are you sure you want to change the base?
Conversation
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.
❌ Changes requested. Reviewed everything up to 9b6a900 in 2 minutes and 4 seconds
More details
- Looked at
571
lines of code in7
files - Skipped
0
files when reviewing. - Skipped posting
11
drafted comments based on config settings.
1. pkg/query-service/app/integrations/thirdPartApi/translator.go:3507
- Draft comment:
Spelling error: Consider renaming 'ParseRequstBody' to 'ParseRequestBody' for clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. pkg/query-service/app/server.go:326
- Draft comment:
New third‑party API routes registered successfully. Verify that authentication and rate‑limiting are correctly applied via the auth middleware. - Reason this comment was not posted:
Comment did not seem useful.
3. pkg/query-service/app/traces/v4/query_builder.go:111
- Draft comment:
Review use of aggregation functions in builder queries – ensure that the conversion functions (e.g. quantile, toFloat64, etc.) properly sanitize inputs to prevent SQL injection. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. pkg/query-service/app/traces/v4/query_builder.go:293
- Draft comment:
When handling the COUNT aggregation, avoid applying it on a timestamp column – review the condition that checks for 'timestamp'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. pkg/query-service/app/integrations/thirdPartApi/translator.go:209
- Draft comment:
Consider documenting the purpose and expected behavior of BuildDomainInfo – it is largely similar to BuildDomainList, so clarifying differences will ease maintenance. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
While documentation could help, this comment is primarily asking for explanation rather than pointing out a specific issue that needs fixing. The functions appear to serve different purposes based on their query construction, and their differences seem intentional. The comment doesn't identify a clear problem that needs addressing.
The functions might be confusing to maintain without documentation, and there could be potential for refactoring to reduce duplication.
However, per the rules, we should not make purely informative comments or ask for explanations. The code itself is functional and the differences appear intentional.
This comment should be removed as it primarily asks for explanation rather than identifying a specific issue that needs fixing.
6. pkg/query-service/app/server.go:319
- Draft comment:
New API routes now include ThirdPartyApi routes. Verify that these endpoints enforce proper access control, and consider adding rate‑limiting if needed. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. pkg/query-service/app/integrations/thirdPartApi/translator.go:1
- Draft comment:
Ensure naming consistency between 'thirdPartApi' and 'thirdPartyApis' across modules for clarity. - Reason this comment was not posted:
Marked as duplicate.
8. pkg/query-service/app/traces/v4/query_builder.go:367
- Draft comment:
Review that the conversion of step interval for rate calculations accounts for unit differences (seconds vs minutes) correctly. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. pkg/query-service/app/traces/v4/query_builder.go:245
- Draft comment:
Consider replacing the magic number -1800 (used to adjust bucketStart) with a named constant for clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. pkg/query-service/app/traces/v4/query_builder.go:242
- Draft comment:
The buildTracesQuery function is quite complex with many nested conditions. Consider refactoring it into smaller helper functions to improve maintainability and readability. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. pkg/query-service/app/traces/v4/query_builder.go:104
- Draft comment:
Ensure that converting the FilterOperator to lowercase here does not unintentionally alter valid operator values if they are case‐sensitive. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_UpveAiwGOgkY0ANd
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.
9b6a900
to
34bbd0c
Compare
Signed-off-by: Shivanshu Raj Shrivastava <[email protected]>
34bbd0c
to
192aa6b
Compare
Signed-off-by: Shivanshu Raj Shrivastava <[email protected]>
192aa6b
to
8f6820f
Compare
Signed-off-by: Shivanshu Raj Shrivastava <[email protected]>
Signed-off-by: Shivanshu Raj Shrivastava <[email protected]>
implements API for landing page of https://www.figma.com/design/RWljuyyoI0tQ07RT2O6pzd/Third-party-API-Monitoring?node-id=42-4764&t=9K0edYVyEN1CPDJy-0
fixes: #6815
Important
Adds new API routes and handlers for third-party API integrations, including domain overview and fetcher services.
RegisterThirdPartyApiRoutes
toserver.go
andhttp_handler.go
for handling third-party API routes./api/v1/third-party-apis/overview/list
,/api/v1/third-party-apis/overview/domain
, and/api/v1/third-party-apis/overview/services
.getDomainList
,getDomainInfo
, andgetUrlList
inhttp_handler.go
to handle requests for third-party API data.ParseRequstBody
to parse request bodies for third-party API requests.ThirdPartApis
struct inmodel.go
to represent third-party API request data.BuildDomainList
andBuildDomainInfo
intranslator.go
to construct query parameters for third-party API requests.FilterResponse
to filter out IP addresses from results ifShowIP
is false.This description was created by
for 9b6a900. It will automatically update as commits are pushed.