-
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
fix: add settings to allow distributed_product_mode for trace panel #6517
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 119d217 in 47 seconds
More details
- Looked at
41
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. pkg/query-service/app/traces/v4/query_builder.go:259
- Draft comment:
Ensure that thedistributed_product_mode='allow'
setting is consistently applied across all relevant queries to avoid potential inconsistencies or errors. This setting is added here as a temporary fix for a specific issue. - Reason this comment was not posted:
Confidence changes required:50%
The addition of the settingdistributed_product_mode='allow'
is a temporary fix for a specific issue. However, it is important to ensure that this setting is applied consistently across all relevant queries to avoid potential inconsistencies or errors.
Workflow ID: wflow_g46GYFmfegDzdtKQ
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.
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.
👍 Looks good to me! Incremental review on 8eab874 in 28 seconds
More details
- Looked at
13
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. pkg/query-service/app/traces/v3/query_builder_test.go:1172
- Draft comment:
TheExpectedQuery
for the test case "Test Noop trace view" should includesettings distributed_product_mode='allow'
to reflect the changes made in the PR. Ensure that all relevant test cases are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_AMedHWBAr5wxaeaz
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Please help me with the complete query log info for one query. Run the prepared query and share all the query_log entries associated with the query_id on all shards. You can use saasmonitor. |
I ran it on shard 2 and the it's present only on shard 2 query log.
|
FYI: You should use |
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.
👍 Looks good to me! Incremental review on 60d663e in 29 seconds
More details
- Looked at
27
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. pkg/query-service/app/traces/v4/query_builder_test.go:534
- Draft comment:
The test case forPanelTypeTrace
includes the new settingmax_memory_usage=10000000000
, but ensure that similar settings are applied to other relevant test cases, such as forPanelTypeList
, if applicable. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_LJp5bHyskzDBi4yd
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Temporary fix for #6515
Important
Adds
settings distributed_product_mode='allow'
to SQL queries inbuildTracesQuery()
to handle distributed product mode errors and updates tests accordingly.settings distributed_product_mode='allow'
to SQL query inbuildTracesQuery()
inquery_builder.go
to prevent distributed product mode errors.Test_buildTracesQuery
inquery_builder_test.go
to includesettings distributed_product_mode='allow'
.This description was created by for 60d663e. It will automatically update as commits are pushed.